Bug 126381

Summary: REGRESSION(r160304): [GTK] Disable libtool fast install
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, commit-queue, dbates, gustavo, mrobinson, pnormand, zan
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
mrobinson: review+
Patch gustavo: review+

Description Carlos Garcia Campos 2014-01-02 05:21:14 PST
After r160304 we are currently building some of our binaries that are installed with the -no-fast-install ld flag. This makes that the binaries are installed with the source code path hardcoded in binary RPATH:

$ objdump -p /home/cgarcia/gnome/libexec/WebKitWebProcess | grep RPATH
  RPATH                /home/cgarcia/src/git/gnome/WebKit/WebKitBuild/Release/.libs:/home/cgarcia/gnome/lib64

The problem is that fast install is not disabled when installing and the binaries are not relinked.
Comment 1 Carlos Garcia Campos 2014-01-02 05:24:43 PST
Created attachment 220220 [details]
Patch
Comment 2 Carlos Garcia Campos 2014-01-02 09:51:13 PST
Martin made a good point, if we disable fast-install globally, we don't use to use the ld flag for every binary. I'll update the patch.
Comment 3 Carlos Garcia Campos 2014-01-02 10:01:18 PST
Created attachment 220231 [details]
Patch
Comment 4 WebKit Commit Bot 2014-01-02 10:02:20 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 5 Carlos Garcia Campos 2014-01-02 23:53:17 PST
Committed r161255: <http://trac.webkit.org/changeset/161255>
Comment 6 Alberto Garcia 2014-01-03 11:32:17 PST
I'm afraid this change has brought some side effects.

If you 'make install' webkit in a temporary directory using the
DESTDIR variable (which is what most packaging systems do) you get
errors like this:

libtool: install: /usr/bin/install -c Programs/WebKitPluginProcess /tmp/destdir/usr/lib/x86_64-linux-gnu/webkit2gtk-3.0/libexec/WebKitPluginProcess
libtool: install: warning: `libjavascriptcoregtk-3.0.la' has not been installed in `/usr/lib/x86_64-linux-gnu'
libtool: install: warning: `libwebkit2gtk-3.0.la' has not been installed in `/usr/lib/x86_64-linux-gnu'
libtool: install: warning: cannot relink `Programs/WebKitWebProcess'

Then the resulting WebKitWebProcess is not the relinked binary but the
libtool script.
Comment 7 Alberto Garcia 2014-01-03 13:12:29 PST
(In reply to comment #6)
> Then the resulting WebKitWebProcess is not the relinked binary but the libtool script.

Ok, this only seems to happen with the old GNU linker and not with gold.

I don't quite understand the internals here, but one difference is
that the generated libtool script seems to be configured with
`fast_install=no' in the ld.bfd case, and `fast_install=needless' in
the gold case.
Comment 8 Alberto Garcia 2014-01-07 07:09:12 PST
I'm reopening this. If you don't use the gold linker you end up with
the libtool script installed when you do 'make install':

$ make DESTDIR=/tmp/dest install
  [...]
$ file /tmp/dest/usr/local/libexec/WebKit*
/tmp/dest/usr/local/libexec/WebKitNetworkProcess: Bourne-Again shell script, ASCII text executable, with very long lines
/tmp/dest/usr/local/libexec/WebKitPluginProcess:  Bourne-Again shell script, ASCII text executable, with very long lines
/tmp/dest/usr/local/libexec/WebKitWebProcess:     Bourne-Again shell script, ASCII text executable, with very long lines

If the only problem is the rpath, we can probably strip it using
chrpath or something similar, which is what we're doing at the moment
in Debian btw.
Comment 9 Alberto Garcia 2014-01-07 07:35:15 PST
This seems to be a known libtool bug:

http://savannah.gnu.org/support/?107416
Comment 10 Carlos Garcia Campos 2014-01-07 08:41:36 PST
(In reply to comment #8)
> I'm reopening this. If you don't use the gold linker you end up with
> the libtool script installed when you do 'make install':
>

I'm not using gold and my binaries are correctly relinked before being installed.

> $ make DESTDIR=/tmp/dest install
>   [...]
> $ file /tmp/dest/usr/local/libexec/WebKit*
> /tmp/dest/usr/local/libexec/WebKitNetworkProcess: Bourne-Again shell script, ASCII text executable, with very long lines
> /tmp/dest/usr/local/libexec/WebKitPluginProcess:  Bourne-Again shell script, ASCII text executable, with very long lines
> /tmp/dest/usr/local/libexec/WebKitWebProcess:     Bourne-Again shell script, ASCII text executable, with very long lines
> 
> If the only problem is the rpath, we can probably strip it using
> chrpath or something similar, which is what we're doing at the moment
> in Debian btw.
Comment 11 Alberto Garcia 2014-01-07 09:02:25 PST
(In reply to comment #10)
> I'm not using gold and my binaries are correctly relinked before
> being installed.

Do you pass DESTDIR to make install? Are the .la files being installed
as well?
Comment 12 Carlos Garcia Campos 2014-01-07 09:07:15 PST
(In reply to comment #11)
> (In reply to comment #10)
> > I'm not using gold and my binaries are correctly relinked before
> > being installed.
> 
> Do you pass DESTDIR to make install?

No, I don't

> Are the .la files being installed
> as well?

Yes, it seem so.
Comment 13 Alberto Garcia 2014-01-07 09:43:00 PST
(In reply to comment #12)
> > Do you pass DESTDIR to make install?
>
> No, I don't

libtool doesn't seem to be able to find the .la files that it needs to
relink the binaries if they are installed in a temporary dir using
DESTDIR, which is the method that distributors use to package the
sofware.

I think I anyway found the solution, if we pass --enable-fast-install
to configure all binaries will be correctly linked for installation
and without rpath, so that should be enough.

Gustavo, what do you think?
Comment 14 Carlos Garcia Campos 2014-01-07 09:56:15 PST
(In reply to comment #13)
> (In reply to comment #12)
> > > Do you pass DESTDIR to make install?
> >
> > No, I don't
> 
> libtool doesn't seem to be able to find the .la files that it needs to
> relink the binaries if they are installed in a temporary dir using
> DESTDIR, which is the method that distributors use to package the
> sofware.
> 
> I think I anyway found the solution, if we pass --enable-fast-install
> to configure all binaries will be correctly linked for installation
> and without rpath, so that should be enough.

I expected --enable-fast-install to be the same than removing the AC_DISABLE_FAST_INSTALL macro.

Have you tried it with ld as well?

> Gustavo, what do you think?
Comment 15 Alberto Garcia 2014-01-07 10:03:01 PST
(In reply to comment #14)
> I expected --enable-fast-install to be the same than removing the
> AC_DISABLE_FAST_INSTALL macro.

I think so, yes, but then you would have to disable it explicitly when
developing in order to prevent bug 125436.

> Have you tried it with ld as well?

Yes, because with gold seems to work fine in all situations.
Comment 16 Gustavo Noronha (kov) 2014-01-08 04:02:40 PST
We could enable fast install when developer mode is off (so in the same situations in which we would use the linker script to narrow down the symbols that are exported), how does that sound?
Comment 17 Alberto Garcia 2014-01-08 04:25:37 PST
(In reply to comment #16)
> We could enable fast install when developer mode is off (so in the
> same situations in which we would use the linker script to narrow
> down the symbols that are exported), how does that sound?

That actually sounds like the best solution.
Comment 18 Alberto Garcia 2014-01-08 06:09:00 PST
Created attachment 220626 [details]
Patch

After discussing it on IRC, I think the simplest solution is to
disable the fast-install mode when building with build-webkit (which
is what most developers use) and leave it enabled otherwise, which
should make life easier to everyone else.

I wrote a short summary of the whole issue in the ChangeLog file,
hopefully it's clear enough.
Comment 19 Gustavo Noronha (kov) 2014-01-08 06:15:14 PST
Comment on attachment 220626 [details]
Patch

♥ libtool
Comment 20 Alberto Garcia 2014-01-08 06:31:23 PST
Committed r161496: <http://trac.webkit.org/changeset/161496>