Re: Space separator missing in gnulib when building wget2
On 11/28/19 11:11 PM, Bruno Haible wrote: > Hi Dagobert, > > lib/gnulib.mk there is a suspicious addition to libgnu_la_LDFLAGS without > space: >> >> libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL) >> ... >> libgnu_la_LDFLAGS += $(LTLIBICONV) >> libgnu_la_LDFLAGS += $(LTLIBINTL) >> libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD) >> ``` >> >> The file `lib/gnulib.mk` is generated during bootstrap. >> >> What is actually executed during bootstrap is >> >> gnulib/gnulib-tool.py --no-changelog --aux-dir=build-aux --doc-base=doc >> --lib=libgnu --m4-base=m4/ --source-base=lib/ --tests-base=lib/tests >> --local-dir=gl --makefile-name=gnulib.mk --libtool --import accept access >> arpa_inet atoll bind c-strcase c-strcasestr c-ctype calloc-posix >> canonicalize-lgpl clock-time close closedir cond connect crypto/md2 >> crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dirname dup dup2 errno >> fclose fcntl fdopen fflush flock fnmatch-gnu fopen freopen fstat fsync >> ftruncate futimens getaddrinfo getpass getsockname gettext-h gettime >> gitlog-to-changelog glob iconv inet_pton inline inttypes ioctl isatty >> lib-symbol-visibility limits-h link listen lock maintainer-makefile >> malloc-posix memchr mkdir mkstemp msvc-nothrow nanosleep netdb netinet_in >> nl_langinfo open opendir pclose pipe-posix progname popen poll posix_spawn >> pwrite qsort_r random_r read readdir realloc-posix recv recvfrom regex >> rename safe-read safe-write select send sendto servent setlocale setsockopt >> socket sockets socklen spawn-pipe stdarg stdbool stddef stdint stat strcase >> strchrnul strdup-posix strerror strndup strpbrk strstr strtoll sys_file >> sys_socket sys_stat sys_time sys_types sys_uio thread time_r unistd unlink >> update-copyright warnings wcwidth write xgethostname >> >> and this command reproduces the error in `lib/gnulib.mk` > > Thanks for the test case. When I run this command > - once with gnulib-tool (shell script), > - once with gnulib-tool.py, > I see these differences in the generated lib/gnulib.mk: > > @@ -168,7 +35,7 @@ > EXTRA_libgnu_la_SOURCES = > libgnu_la_LDFLAGS = $(AM_LDFLAGS) > libgnu_la_LDFLAGS += -no-undefined > -libgnu_la_LDFLAGS += $(GETADDRINFO_LIB) > +libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL) > libgnu_la_LDFLAGS += $(GETHOSTNAME_LIB) > libgnu_la_LDFLAGS += $(HOSTENT_LIB) > libgnu_la_LDFLAGS += $(INET_NTOP_LIB) > @@ -187,7 +54,7 @@ > libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD) > libgnu_la_LDFLAGS += $(LTLIBTHREAD) > libgnu_la_LDFLAGS += $(SERVENT_LIB) > -libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@ > +libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@$(LTLIBTHREAD) > > ## begin gnulib module absolute-header > > Lines > libgnu_la_LDFLAGS += $(LTLIBINTL) > and > libgnu_la_LDFLAGS += $(LTLIBTHREAD) > are already part of the output. Therefore the right fix is not to insert a > space, but to have gnulib-tool.py generate the same lines as the shell script. > > FYI, in the shell script it is the line gnulib-tool:3856. Attached is a diff for review that fixes the issue. Indeed the 'space fix' above only worked for Link: $(LIB1) $(LIB2) optional comment The diff fixes this to also allow (even if not found in modules yet) Link: $(LIB1) optional comment 1 $(LIB2) optional comment 2 Then, the double $([LT]LIB...) entries are removed and sorted as gnulib-tool does it by 'sort -u'. Please review the patch - surely some of my python constructs could be done more elegant. Regards, Tim diff --git a/pygnulib/GLEmiter.py b/pygnulib/GLEmiter.py index a350ba753..f0746f827 100644 --- a/pygnulib/GLEmiter.py +++ b/pygnulib/GLEmiter.py @@ -853,17 +853,18 @@ AC_DEFUN([%V1%_LIBSOURCES], [ emit += '%s_%s_LDFLAGS += -no-undefined\n' % (libname, libext) # Synthesize an ${libname}_${libext}_LDFLAGS augmentation by combining # the link dependencies of all modules. -listing = list() links = [module.getLink() for module in modules if not module.isTests()] +ulinks = list() for link in links: -link = constants.nlremove(link) -position = link.find(' when linking with libtool') -if position != -1: -link = link[:position] -listing += [link] -listing = sorted(set([link for link in listing if link != ''])) -for link in listing: +for lib in link: +lib = constants.nlremove(lib) +position = lib.find(' when linking with libtool') +if position != -1: +lib = lib[:position] +ulinks += [lib] +ulinks = sorted(set(ulinks)) +for link in ulinks: emit += '%s_%s_LDFLAGS += %s\n' % (libname, libext, link) emit += '\n' if pobase: diff --git a/pygnulib/GLImport.py b/pygnulib/GLImport.py index 19ade5e8d..27a01be43 100644 ---
Re: Space separator missing in gnulib when building wget2
Hi Tim, Am 29.11.2019 um 10:16 schrieb Tim Rühsen : > But first let's get the algorithm straight. > In modules/getaddrinfo (and some other modules) are 2 'Link' libraries > given: > > Link: > $(GETADDRINFO_LIB) > $(LTLIBINTL) when linking with libtool, $(LIBINTL) otherwise > > So "libgnu_la_LDFLAGS += $(GETADDRINFO_LIB) $(LTLIBINTL)" would be right > in the first place. But we don't want to add any of those libraries in > case libgnu_la_LDFLAGS already contain them (avoiding duplicates) !? > > Is that right ? > > And what about the order of libraries ? AFAIR, gcc/ld 9+ rely on the > correct order of libraries while older gcc/ld version doesn't. From what I see the shellscript uses 'sort -u‘ on the flag additions. To match the shell behaviour there should be only variable per line or the '-u‘ for uniqueness of addition does not work. This is currently done with a set in Python which should do it, but getLinks currently returns a concatenated string instead of a list which leads to the observed error. Best regards — Dago -- "You don't become great by trying to be great, you become great by wanting to do something, and then doing it so hard that you become great in the process." - xkcd #896 signature.asc Description: Message signed with OpenPGP
Re: Space separator missing in gnulib when building wget2
On 11/28/19 11:11 PM, Bruno Haible wrote: > Hi Dagobert, > > lib/gnulib.mk there is a suspicious addition to libgnu_la_LDFLAGS without > space: >> >> libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL) >> ... >> libgnu_la_LDFLAGS += $(LTLIBICONV) >> libgnu_la_LDFLAGS += $(LTLIBINTL) >> libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD) >> ``` >> >> The file `lib/gnulib.mk` is generated during bootstrap. >> >> What is actually executed during bootstrap is >> >> gnulib/gnulib-tool.py --no-changelog --aux-dir=build-aux --doc-base=doc >> --lib=libgnu --m4-base=m4/ --source-base=lib/ --tests-base=lib/tests >> --local-dir=gl --makefile-name=gnulib.mk --libtool --import accept access >> arpa_inet atoll bind c-strcase c-strcasestr c-ctype calloc-posix >> canonicalize-lgpl clock-time close closedir cond connect crypto/md2 >> crypto/md5 crypto/sha1 crypto/sha256 crypto/sha512 dirname dup dup2 errno >> fclose fcntl fdopen fflush flock fnmatch-gnu fopen freopen fstat fsync >> ftruncate futimens getaddrinfo getpass getsockname gettext-h gettime >> gitlog-to-changelog glob iconv inet_pton inline inttypes ioctl isatty >> lib-symbol-visibility limits-h link listen lock maintainer-makefile >> malloc-posix memchr mkdir mkstemp msvc-nothrow nanosleep netdb netinet_in >> nl_langinfo open opendir pclose pipe-posix progname popen poll posix_spawn >> pwrite qsort_r random_r read readdir realloc-posix recv recvfrom regex >> rename safe-read safe-write select send sendto servent setlocale setsockopt >> socket sockets socklen spawn-pipe stdarg stdbool stddef stdint stat strcase >> strchrnul strdup-posix strerror strndup strpbrk strstr strtoll sys_file >> sys_socket sys_stat sys_time sys_types sys_uio thread time_r unistd unlink >> update-copyright warnings wcwidth write xgethostname >> >> and this command reproduces the error in `lib/gnulib.mk` > > Thanks for the test case. When I run this command > - once with gnulib-tool (shell script), > - once with gnulib-tool.py, > I see these differences in the generated lib/gnulib.mk: > > @@ -168,7 +35,7 @@ > EXTRA_libgnu_la_SOURCES = > libgnu_la_LDFLAGS = $(AM_LDFLAGS) > libgnu_la_LDFLAGS += -no-undefined > -libgnu_la_LDFLAGS += $(GETADDRINFO_LIB) > +libgnu_la_LDFLAGS += $(GETADDRINFO_LIB)$(LTLIBINTL) > libgnu_la_LDFLAGS += $(GETHOSTNAME_LIB) > libgnu_la_LDFLAGS += $(HOSTENT_LIB) > libgnu_la_LDFLAGS += $(INET_NTOP_LIB) > @@ -187,7 +54,7 @@ > libgnu_la_LDFLAGS += $(LTLIBMULTITHREAD) > libgnu_la_LDFLAGS += $(LTLIBTHREAD) > libgnu_la_LDFLAGS += $(SERVENT_LIB) > -libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@ > +libgnu_la_LDFLAGS += @INTL_MACOSX_LIBS@$(LTLIBTHREAD) > > ## begin gnulib module absolute-header > > Lines > libgnu_la_LDFLAGS += $(LTLIBINTL) > and > libgnu_la_LDFLAGS += $(LTLIBTHREAD) > are already part of the output. Therefore the right fix is not to insert a > space, but to have gnulib-tool.py generate the same lines as the shell script. > > FYI, in the shell script it is the line gnulib-tool:3856. > > Any volunteer for doing this? As far as it keeps calm today, let me give it a try. Need to polish my almost zero python experience ;-) But first let's get the algorithm straight. In modules/getaddrinfo (and some other modules) are 2 'Link' libraries given: Link: $(GETADDRINFO_LIB) $(LTLIBINTL) when linking with libtool, $(LIBINTL) otherwise So "libgnu_la_LDFLAGS += $(GETADDRINFO_LIB) $(LTLIBINTL)" would be right in the first place. But we don't want to add any of those libraries in case libgnu_la_LDFLAGS already contain them (avoiding duplicates) !? Is that right ? And what about the order of libraries ? AFAIR, gcc/ld 9+ rely on the correct order of libraries while older gcc/ld version doesn't. Regards, Tim signature.asc Description: OpenPGP digital signature