bugGNU Wget - Bugs: bug #50223, wget 1.19 will not build on MacOS...

 
 

bug #50223: wget 1.19 will not build on MacOS 10.12.3

Submitter:  Charles <chdiza>
Submitted:  Fri 03 Feb 2017 04:14:45 PM UTC
   
 
Category:  Build/Install Severity:  3 - Normal
Priority:  5 - Normal Status:  Fixed
Privacy:  Public Assigned to:  None
Originator Name:  chdiza Open/Closed:  Closed
Release:  1.19 Operating System:  Mac OS
Reproducibility:  Every Time Fixed Release:  None
Planned Release:  1.19.1 Regression:  None
Work Required:  None Patch Included:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 07 Feb 2017 02:04:45 PM UTC, comment #21: 

I going to make a 1.19.1 release in a few days... still waiting for more reports ;-)

Tim Ruehsen <rockdaboot>
Group administrator
Tue 07 Feb 2017 12:34:03 AM UTC, comment #20: 

Mind publishing a wget 1.19.1 or 1.20 release tarball?

ilovezfs <ilovezfs>
Mon 06 Feb 2017 10:02:55 PM UTC, comment #19: 

As you say, Test-iri-disabled and 'invalid multibyte sequence' do not belong here.

I pushed the changes in configure.ac and tests/WgetTests.pm.

Thanks everybody for info & helping.

Tim Ruehsen <rockdaboot>
Group administrator
Sun 05 Feb 2017 11:43:32 PM UTC, comment #18: 

I just ran make check with Wget 1.18 and a whopping 9 tests failed on macOS 10.12.3, including Test-iri-disabled, so this test failure isn't a regression and doesn't really belong to this thread.

Zhiming Wang <zmwang>
Sun 05 Feb 2017 11:30:00 PM UTC, comment #17: 

I also added another log where wget is run with --debug, in case it is more useful. Also, I've seen this


Incomplete or invalid multibyte sequence encountered


error message with IRIs with older versions of Wget before (compiled with IRI support).

Zhiming Wang <zmwang>
Sun 05 Feb 2017 10:28:15 PM UTC, comment #16: 

Thanks, removing the test "X$iri" = "Xno" branch does fix the problem.

Now that compilation succeeds, I ran make check to make sure everything's working, and insterestingly, whether I configure with --disable-iri or not, there's one single test failure: Test-iri-disabled.px. I'm attaching the log for this test. Looks like wget insists on percent encoding the filenames, despite the "blah blah saved" showing the raw unencoded filenames.

By the way, I also noticed a small problem in tests/WgetTests.pm's error message. Here's a patch to fix it


diff --git a/tests/WgetTests.pm b/tests/WgetTests.pm
index 34253b1..4d32813 100644
--- a/tests/WgetTests.pm
+++ b/tests/WgetTests.pm
@@ -359,8 +359,8 @@ sub _verify_download
               );
     if (@unexpected_downloads)
     {
-        return 'Test failed: unexpected downloaded files [' . join ', ',
-          @unexpected_downloads . "]\n";
+        return 'Test failed: unexpected downloaded files [' .
+          (join ', ', @unexpected_downloads) . "]\n";
     }

     return q{};


As you can see, without the patch (which puts parentheses around the join), @unexpected_downloads is treated in a scalar context and join ', ' does nothing, resulting in messages like


Test failed: unexpected downloaded files [3]


instead of the presumably intended


Test failed: unexpected downloaded files [p1_fran%E7ais.html, p2_%E9%E9n.html, p3_%A4%A4%A4.html]


Zhiming Wang <zmwang>
Sun 05 Feb 2017 07:06:24 PM UTC, comment #15: 

I guess I found the prolem in configure.ac:

if test "X$iri" = "Xno"; then
  # we don't need libunistring - clear settings from gnulib module
  LIBUNISTRING=""
  LTLIBUNISTRING=""
fi

That is old code and it hits you when you don't build with IRI support (your gist shows IRI: no).

Please test again without that code (remove it from configure.ac and autoreconf && ./configure && make).

If that works for you, I'll make a commit without it.

Tim Ruehsen <rockdaboot>
Group administrator
Sun 05 Feb 2017 06:41:56 PM UTC, comment #14: 

$ ls -la m4/libunistring*.m4
-rw-r--r-- 1 tim tim 6320 Feb  4 15:22 m4/libunistring-base.m4
-rw-r--r-- 1 tim tim 6348 Feb  4 15:22 m4/libunistring.m4
-rw-r--r-- 1 tim tim  961 Feb  4 15:22 m4/libunistring-optional.m4

What you could do to debug the emptiness of (LT)LIBUNISTRING:
Edit m4/libunistring.m4, put some 'echo' lines printing $LIBUNISTRING. The *.m4 files are translated into shell code (the ./configure script). You will see your echoes in the output of ./configure. That is what I would do (and did a few times when developing m4 macros).

There are not so many places where (LT)LIBUNISTRING is set:
```
m4/libunistring.m4:          HAVE_LIBUNISTRING=no
m4/libunistring.m4:        LIBUNISTRING=
m4/libunistring.m4:        LTLIBUNISTRING=
m4/libunistring.m4:          LIBUNISTRING=
m4/libunistring.m4:          LTLIBUNISTRING=
m4/libunistring.m4:        LIBUNISTRING="$LIBUNISTRING $LIBICONV"
m4/libunistring.m4:        LTLIBUNISTRING="$LTLIBUNISTRING $LTLIBICONV"
```

Tim Ruehsen <rockdaboot>
Group administrator
Sun 05 Feb 2017 12:35:14 AM UTC, comment #13: 

I expanded the set of modules to all of those included in bootstrap.conf, but couldn't reproduce the empty LIBUNISTRING and LTLIBUNISTRING. In case someone wants to take a look, here are the files for my dummy project: https://gist.github.com/642a4acc94da8f487d7e3cacd2b12f98.

Wget's current gnulib submodule is 16 commits behind master; fast forwarding to master doesn't seem to change anything.

Zhiming Wang <zmwang>
Sat 04 Feb 2017 09:43:08 PM UTC, comment #12: 

Thanks for putting so work into this issue !

It might be a gnulib issue, but we need a work-around anyways.

Could you run your dummy project with all the gnulib modules from wget (bootstrap.conf) ? Just to see if the problem occurs or not.

If the problem occurs, maybe you could track it down to a few modules (libunistring-optional and unistr/u8-strlen, unicase/u8-tolower might interfere in some ways).

Too bad that I can't reproduce it...

BTW, in the wget directory, you could test with the latest gnulib: 'cd gnulib; git checkout master; git pull; cd ..; git commit -m "gnulib: update" gnulib; ./bootstrap; ./configure; make clean; make'

Tim Ruehsen <rockdaboot>
Group administrator
Sat 04 Feb 2017 08:45:17 PM UTC, comment #11: 

The commit to add wcwidth to bootstrap.conf did not help me, but upon further investigation, I realized that it was because I have libunistring installed (which, if linked, provides uc_width); if I move libunistring out of the way, I have no problem with compilation. Therefore, this may not be a problem for most users.

That said, the libunistring problem is a weird one. configure does check for libunistring:


./configure | grep libunistring
checking whether included libunistring is requested... no
checking for libunistring... yes
checking how to link with libunistring... -L/usr/local/lib -lunistring
checking for libunistring version... 0.9.7
checking whether to use the included libunistring... no


but it doesn't set the proper ld flags:


$ grep LIBUNISTRING= config.log
HAVE_LIBUNISTRING='yes'
LIBUNISTRING=''
LTLIBUNISTRING=''


(the above should instead be:


$ grep LIBUNISTRING= config.log
HAVE_LIBUNISTRING='yes'
LIBUNISTRING='-L/usr/local/lib -lunistring'
LTLIBUNISTRING='-L/usr/local/lib -lunistring -R/usr/local/lib'


)

Interestingly, I created a dummy project where I ran


gnulib/gnulib-tool --import libunistring-optional


then with boilerplate configure.ac and automake.am, the generated configure would set LIBUNISTRING and LTLIBUNISTRING to the correct values. Not sure where the problem is.

Zhiming Wang <zmwang>
Sat 04 Feb 2017 06:34:17 PM UTC, comment #10: 

Ah, OK.
'wcwidth' gnulib module was missing in bootstrap.conf.
Please try again the latest git master.

Tim Ruehsen <rockdaboot>
Group administrator
Sat 04 Feb 2017 05:43:13 PM UTC, comment #9: 

Issues other than uc_width have been fixed by master.

Sorry for not being specific enough about uc_width. It is used by rpl_wcwidth:


  CCLD     wget
Undefined symbols for architecture x86_64:
  "_uc_width", referenced from:
      _rpl_wcwidth in libgnu.a(wcwidth.o)
ld: symbol(s) not found for architecture x86_64


wcwidth replacement is used because


checking whether wcwidth works reasonably in UTF-8 locales... no


Logs can be found at https://gist.github.com/51e9245975f3081bba345184c5be6b08.

Zhiming Wang <zmwang>
Sat 04 Feb 2017 05:13:01 PM UTC, comment #8: 

Could anyone test latest 'master' from git repo ? It needs a ./bootstrap && ./configure && make clean && make && make check

I tried to fix all three issues mentioned here, but I am unsure about uc_width (likely used by u8_strlen()).

@zmwang There is the gnu-info mailing list for announcements. But only non-alpha release should be announced there. Maybe there is a way to change the policy...

Tim Ruehsen <rockdaboot>
Group administrator
Fri 03 Feb 2017 11:42:49 PM UTC, comment #7: 

By the way, may I suggest that an announcement mailing list (say, wget-announce), be created to notify people about releases, including alphas? I assume many of us would be willing to test alpha releases but may not want to subscribe to bug-wget.

(I just put together a cron job to monitor https://alpha.gnu.org/gnu/wget/ so I will be notified of future alphas, but subscribing to an announcement list would be so much easier.)

Zhiming Wang <zmwang>
Fri 03 Feb 2017 11:00:44 PM UTC, comment #6: 

Including unistd.h before wget.h (provided that "config.h" is included before <unistd.h>) does fix the issue. However, I uncovered two more bugs while testing this fix.

First, in src/xattr.h, the fsetxattr macro is declared with a space between the macro name and the argument list parenthesis, which is apparently wrong. Trivial to fix.

Secondly, for some reason, lib/uniwidth/width.c isn't added to the source list, leading to undefined symbol error for uc_width. Since autotools make my head spin... I just uncommented the "if LIBUNISTRING_COMPILE_UNIWIDTH_WIDTH" conditional guarding "libgnu_a_SOURCES += uniwidth/width.c" in lib/Makefile.am, and the build proceeded to a success. Apparently this is not a proper fix, but at least it tells us how many bugs there are in total...

I have attached my "sketchy patch", if you just want to get your hands on a copy of wget 1.19 on macOS as soon as possible and don't care for proper fixes.

Zhiming Wang <zmwang>
Fri 03 Feb 2017 10:54:42 PM UTC, comment #5: 

And adding


#include "config.h"


first before


#include <unistd.h>


leads back to the originally reported error.

Ryan Carsten Schmidt <ryandesign>
Fri 03 Feb 2017 10:47:11 PM UTC, comment #4: 

Making the suggested change leads to:


In file included from ftp.c:32:
../lib/unistd.h:139:3: error: "Please include config.h first."
 #error "Please include config.h first."
  ^
../lib/unistd.h:141:1: error: unknown type name '_GL_INLINE_HEADER_BEGIN'
_GL_INLINE_HEADER_BEGIN
^
../lib/unistd.h:669:1: error: expected identifier or '('
_GL_CXXALIAS_SYS (close, int, (int fd));
^
../lib/time.h:231:5: note: expanded from macro '_GL_CXXALIAS_SYS'
    _GL_EXTERN_C int _gl_cxxalias_dummy
    ^
../lib/time.h:123:23: note: expanded from macro '_GL_EXTERN_C'
# define _GL_EXTERN_C extern
                      ^
In file included from ftp.c:32:
../lib/unistd.h:1943:1: error: unknown type name '_GL_INLINE_HEADER_END'
_GL_INLINE_HEADER_END
^
In file included from ftp.c:34:
In file included from ./wget.h:47:
In file included from ./sysdep.h:42:
In file included from ../lib/sys/stat.h:47:
/usr/include/sys/stat.h:110:1: error: expected identifier or '('
struct ostat {
^
5 errors generated.


Ryan Carsten Schmidt <ryandesign>
Fri 03 Feb 2017 09:48:12 PM UTC, comment #3: 

Thanks for the additional info.
It looks like unlink() doesn't work as expected on MacOSX, so that gnulib add wrapper code, doing it in <unistd.h> with

#define unlink rpl_unlink

Now, in ftp.c we include "wget.h" before <unistd.h>.
"wget.h" includes "options.h" which declares 'struct options' with a member called 'unlink'. Thereafter <unistd.h> makes the above define and thus all references to member 'unlink' are rewritten to 'rpl_unlink' by the preprocessor => syntax error.

Could anyone test to include unistd.h before wget.h in ftp.c ?
Maybe other sources are affected similarly... but it would be good to know if ftp.c compiles with such a change.

I guess the best fix would be to rename member 'unlink' to somethink else, e.g. 'do_unlink'.

Tim Ruehsen <rockdaboot>
Group administrator
Fri 03 Feb 2017 09:30:56 PM UTC, comment #2: 

I have attached config.log. Other log files can be found in https://gist.github.com/14bb04872e454f7769c661c4e0ff680f.

The problem is not limited to macOS 10.12.3. We see the same error on 10.10 and 10.11.

Note that macOS does have unlink(2), declared in unistd.h. The difference between wget 1.18 and wget 1.19 is that configure performs additional checks on unlink and unlinkat which fail:


checking whether unlink honors trailing slashes... no
checking whether unlink of a parent directory fails as it should... no


Zhiming Wang <zmwang>
Fri 03 Feb 2017 04:40:48 PM UTC, comment #1: 

That's why we had an alpha release... so everyone could test and report before the release... ;-)

We could easily solve that problem by renaming the field...

But is there really no unlink() function available on MacOS 10.12.3 ? If there is, the gnulib detection of unlink() doesn't seem to work correctly and we have to talk with the gnulib people.

Could you upload config.log here ?

Tim Ruehsen <rockdaboot>
Group administrator
Fri 03 Feb 2017 04:14:45 PM UTC, original submission:  

I never had any problems with wget 1.18, but trying to build 1.19 with the same ./configure flags as before will fail to build.  My flags are:
  --with-ssl=openssl
  --with-libssl-prefix=/usr/local/ssl
  --diasble-pcre
  --disable-iri

The `make` step fails with this:

------------------8<------------------------------------
/Applications/Xcode.app/Contents/Developer/usr/bin/make  all-am
  CC       connect.o
  CC       convert.o
  CC       cookies.o
  CC       ftp.o
ftp.c:1466:19: error: no member named 'rpl_unlink' in 'struct options'
          if (opt.unlink && file_exists_p (con->target))
              ~~~ ^
../lib/unistd.h:1851:19: note: expanded from macro 'unlink'
#   define unlink rpl_unlink
                  ^
1 error generated.
make[3]: * [ftp.o] Error 1
make[3]: * Waiting for unfinished jobs....
make[2]: * [all] Error 2
make[1]: * [all-recursive] Error 1
make: * [all] Error 2
----------------------------8<-----------------------------


Charles <chdiza>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #39677:  Test-iri-disabled-with-debug.log added by zmwang (7KiB - application/octet-stream - Test-iri-disabled.log, where wget is run with --debug)
file #39675:  Test-iri-disabled.log added by zmwang (2KiB - application/octet-stream - Log of failed test tests/Test-iri-disabled.px. Filenames are percent encoded when they shouldn't be.)
file #39657:  macOS-sketchy-patch.diff added by zmwang (2KiB - application/octet-stream - sketchy patch that fixes compilation on OS X / macOS; not proper fix)
file #39656:  config.log added by zmwang (977KiB - application/octet-stream - config.log wget 1.19 macOS 10.12.3)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by ilovezfs (Posted a comment)
  • -email is unavailable- added by ryandesign (Posted a comment)
  • -email is unavailable- added by zmwang (Updated the item)
  • -email is unavailable- added by rockdaboot (Posted a comment)
  • -email is unavailable- added by chdiza (Submitted the item)
  •  

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-02-06 rockdaboot StatusNone Fixed
        Open/ClosedOpen Closed
        Planned ReleaseNone 1.19.1
    2017-02-05 zmwang Attached File- Added Test-iri-disabled-with-debug.log, #39677
    2017-02-05 zmwang Attached File- Added Test-iri-disabled.log, #39675
    2017-02-03 zmwang Attached File- Added macOS-sketchy-patch.diff, #39657
    2017-02-03 zmwang Attached File- Added config.log, #39656

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code