On Wed, Nov 20, 2019 at 02:13:37PM +0100, Ján Tomko wrote:
> On Mon, Nov 11, 2019 at 02:38:17PM +0000, Daniel P. Berrangé wrote:
> > As part of an goal to eliminate Perl from libvirt build tools,
> > rewrite the pdwtags processing script in Python.
> >
> > The original inline shell and perl code was completely
> > unintelligible. The new python code is a manual conversion
> > that attempts todo basically the same thing.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
> > ---
> > Makefile.am | 1 +
> > build-aux/syntax-check.mk | 3 +-
> > scripts/check-remote-protocol.py | 136 +++++++++++++++++++++++++++++++
> > src/Makefile.am | 98 ++++------------------
> > 4 files changed, 155 insertions(+), 83 deletions(-)
> > create mode 100644 scripts/check-remote-protocol.py
> >
> > +if n < 1:
> > + print("WARNING: No structs/enums matched. Your", file=sys.stderr)
> > + print("WARNING: pdwtags program is probably too old", file=sys.stderr)
> > + print("WARNING: skipping the remote protocol test", file=sys.stderr)
> > + print("WARNING: install dwarves-1.3 or newer", file=sys.stderr)
> > + sys.exit(8)
> > +
> > +diff = subprocess.Popen(["diff", "-u", expected, "-"], stdin=subprocess.PIPE)
> > +actualstr = "\n".join(actual) + "\n"
> > +diff.communicate(input=actualstr.encode("utf-8"))
> > +
> > +sys.exit(diff.returncode)
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index bb63e2486c..c40e61e7ee 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -196,83 +196,6 @@ DRIVER_SOURCES += \
> >
> >
> >
> > -# Ensure that we don't change the struct or member names or member ordering
> > -# in remote_protocol.x The embedded perl below needs a few comments, and
> > -# presumes you know what pdwtags output looks like:
> > -# * use -0777 -n to slurp the entire file into $_.
> > -# * the "split" splits on the /* DD */ comments, so that $p iterates
> > -# through the struct definitions.
> > -# * process only "struct remote_..." entries
> > -# * remove comments and preceding TAB throughout
> > -# * remove empty lines throughout
> > -# * remove white space at end of buffer
> > -
> > -# With pdwtags 1.8, --verbose output includes separators like these:
> > -# /* 93 */
> > -# /* <0> (null):0 */
> > -# with the second line omitted for intrinsic types.
> > -# Whereas with pdwtags 1.3, they look like this:
> > -# /* <2d2> /usr/include/libio.h:180 */
> > -# The alternation of the following regexps matches both cases.
> > -r1 = /\* \d+ \*/
> > -r2 = /\* <[[:xdigit:]]+> \S+:\d+ \*/
> > -libs_prefix = remote_|qemu_|lxc_|admin_
> > -other_prefix = keepalive|vir(Net|LockSpace|LXCMonitor)
> > -struct_prefix = ($(libs_prefix)|$(other_prefix))
> > -
> > -# Depending on configure options, libtool creates one or both of
> > -# remote/{,.libs/}libvirt_driver_remote_la-remote_protocol.o. We want
> > -# the newest of the two, in case configure options changed and a stale
> > -# file is left around from an earlier build.
>
> So I assume the configure option that gives you the path without .libs
> is gone?
I honestly don't know what configure option it might have been
referring to in the first place.
My guess was perhaps related to --disable-shared to force a static
library build. Libvirt fails to link when doing this and we never
test it, so I considerd static build out of scope, and in any case
when i try it a .libs dir still appears
In absence of a clear need I figured just ignore this comment and
lets see if anyone reports breakage that our CI systems don't
catch.
> > -# The pdwtags output is completely different when building with clang
> > -# which causes the comparison against expected output to fail, so skip
> > -# if using clang as CC.
> > -PDWTAGS = \
> > - $(AM_V_GEN)if $(CC) -v 2>&1 | grep -q clang; then \
> > - echo 'WARNING: skipping pdwtags test with Clang' >&2; \
> > - exit 0; \
> > - fi; \
> > - if (pdwtags --help) > /dev/null 2>&1; then \
> > - o=`ls -t $(<:.lo=.$(OBJEXT)) \
> > - $(subst /,/.libs/,$(<:.lo=.$(OBJEXT))) \
> > - 2>/dev/null | sed -n 1p`; \
>
> And that OBJEXT is always .o
Same note as above.
>
> > - test -f "$$o" || { echo ".o for $< not found" >&2; exit 1; }; \
> > - pdwtags --verbose $$o > $(@F)-t1 2> $(@F)-t2; \
> > - if test ! -s $(@F)-t1 && test -s $(@F)-t2; then \
> > - rm -rf $(@F)-t?; \
> > - echo 'WARNING: pdwtags appears broken; skipping the $@ test' >&2;\
> > - else \
> > - $(PERL) -0777 -n \
> > - -e 'foreach my $$p (split m!\n*(?:$(r1)|$(r2))\n!) {' \
> > - -e ' if ($$p =~ /^(struct|enum) $(struct_prefix)/) {' \
> > - -e ' $$p =~ s!\t*/\*.*?\*/!!sg;' \
> > - -e ' $$p =~ s!\s+\n!\n!sg;' \
> > - -e ' $$p =~ s!\s+$$!!;' \
> > - -e ' $$p =~ s!\t! !g;' \
> > - -e ' print "$$p\n";' \
> > - -e ' $$n++;' \
> > - -e ' }' \
> > - -e '}' \
> > - -e 'BEGIN {' \
> > - -e ' print "/* -*- c -*- */\n";' \
> > - -e '}' \
> > - -e 'END {' \
> > - -e ' if ($$n < 1) {' \
> > - -e ' warn "WARNING: your pdwtags program is too old\n";' \
> > - -e ' warn "WARNING: skipping the $@ test\n";' \
> > - -e ' warn "WARNING: install dwarves-1.3 or newer\n";' \
> > - -e ' exit 8;' \
> > - -e ' }' \
> > - -e '}' \
> > - < $(@F)-t1 > $(@F)-t3; \
> > - case $$? in 8) rm -f $(@F)-t?; exit 0;; 0) ;; *) exit 1;; esac;\
> > - diff -u $(@)s $(@F)-t3; st=$$?; rm -f $(@F)-t?; exit $$st; \
> > - fi; \
> > - else \
> > - echo 'WARNING: you lack pdwtags; skipping the $@ test' >&2; \
> > - echo 'WARNING: install the dwarves package to get pdwtags' >&2; \
> > - fi
> > -
> > # .libs/libvirt.so is built by libtool as a side-effect of the Makefile
> > # rule for libvirt.la. However, checking symbols relies on Linux ELF layout
> > if WITH_LINUX
> > @@ -301,27 +224,38 @@ PROTOCOL_STRUCTS = \
> > if WITH_REMOTE
> > check-protocol: $(PROTOCOL_STRUCTS) $(PROTOCOL_STRUCTS:structs=struct)
> >
> > +# Ensure that we don't change the struct or member names or member ordering
> > +# in remote_protocol.x The process-pdwtags.py post-processes output to
>
> Here you refer to process-pdwtags.py but the script is called check-remote-protocol.py
>
> > +# extract the bits we want.
> > +
> > +CHECK_REMOTE_PROTOCOL = $(top_srcdir)/scripts/check-remote-protocol.py
> > +
>
> With the comment fixed or the script renamed:
>
> Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx>
>
> Jano
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
