Re: Space separator missing in gnulib when building wget2

2019-11-29 Thread Tim Rühsen
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

2019-11-29 Thread Dagobert Michelsen
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

2019-11-29 Thread Tim Rühsen
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