Bug 128596 - 'ar T' is not portable and breaks the build on FreeBSD
Summary: 'ar T' is not portable and breaks the build on FreeBSD
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks: 128598
  Show dependency treegraph
 
Reported: 2014-02-11 05:29 PST by Allison Lortie (desrt)
Modified: 2014-02-17 12:23 PST (History)
6 users (show)

See Also:


Attachments
SetupLibtool.m4: use 'ar T' only on GNU ar (907 bytes, patch)
2014-02-11 05:58 PST, Allison Lortie (desrt)
no flags Details | Formatted Diff | Diff
Patch (1022 bytes, patch)
2014-02-11 06:16 PST, Alberto Garcia
gns: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Allison Lortie (desrt) 2014-02-11 05:29:18 PST
Source/autotools/SetupLibtool.m4 has:

if test -z "$AR_FLAGS"; then
    AR_FLAGS="cruT"
fi
AC_SUBST([AR_FLAGS])


GNU ar documents the 'T' flag like so:

       T   Make the specified archive a thin archive.  If it already exists and is a regular archive, the existing
           members must be present in the same directory as archive.

FreeBSD ar documents the 'T' flag like so:

     -T      Use only the first fifteen characters of the archive member name
             or command line file name argument when naming archive members.


This fragment of Source/WebKit2/GNUmakefile.am:

libWebCoreLayerGtk2.a: $(webcore_layer_gtk2_deps)
        $(AM_V_GEN)
        $(AM_V_at)$(shell rm -f $@)
        $(AM_V_at)$(shell find . -name "*.o" > objects_list)
        $(AM_V_at)$(foreach archive, $(webcore_layer_gtk2_archives), $(shell $(AR) t $(archive) | xargs -n1 basename | xargs -I obj_file grep -F obj_file objects_list | xargs -n50 $(AR) $(AR_FLAGS) $@))
        $(AM_V_at)$(shell rm -f objects_list)


results in a bunch of arguments like './Source/WebCore/platform/graphics/libPlatform_la-FloatRect.o' being passed to 'ar'.

Of course, most of those arguments will share the same first 15 characters, and as a result, a large number of files are not included in the archive.



Webkit should not use 'ar T' on systems with a non-GNU toolchain, or it should explicitly require a GNU toolchain and make sure it uses that.
Comment 1 Allison Lortie (desrt) 2014-02-11 05:58:58 PST
Created attachment 223846 [details]
SetupLibtool.m4: use 'ar T' only on GNU ar
Comment 2 Alberto Garcia 2014-02-11 06:16:38 PST
Created attachment 223848 [details]
Patch

The patch looks fine, here's the updated one with the changelog entry.

Thanks!
Comment 3 Alberto Garcia 2014-02-12 04:12:48 PST
Committed r163954: <http://trac.webkit.org/changeset/163954>
Comment 4 Landry Breuil 2014-02-17 11:46:42 PST
I know this has been fixed/commited, but as it was already sort-of fixed in https://bugs.webkit.org/show_bug.cgi?id=118732, couldnt you set AR_FLAGS=cru in the build environment to avoid having to patch autohell this way ?
Comment 5 Allison Lortie (desrt) 2014-02-17 11:52:07 PST
I don't think it should be expected to have to set an environment variable to prevent it from containing non-POSIX values...

If anything, it should be the other way around.

I think having this check can make everyone happy.
Comment 6 Landry Breuil 2014-02-17 12:00:23 PST
(In reply to comment #5)
> I don't think it should be expected to have to set an environment variable to prevent it from containing non-POSIX values...
> 
> If anything, it should be the other way around.
> 
> I think having this check can make everyone happy.

I totally agree (and i'm happy with the check for OpenBSD too!), was just wondering about the reasoning - usually it's not so easy to get non-linux fixes in :)
Comment 7 Alberto Garcia 2014-02-17 12:23:15 PST
(In reply to comment #6)
> I totally agree (and i'm happy with the check for OpenBSD too!), was
> just wondering about the reasoning - usually it's not so easy to get
> non-linux fixes in :)

The solution proposed by Ryan looked just fine to me. And I've been
pushing non-linux fixes for a long time btw :)