Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Michael Catanzaro
Depends on: 140068
Blocks: 110014
  Show dependency treegraph
Reported: 2015-01-04 20:10 PST by Michael Catanzaro
Modified: 2015-07-15 13:35 PDT (History)
5 users (show)

See Also:

Patch (2.82 KB, patch)
2015-01-04 20:18 PST, Michael Catanzaro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Catanzaro 2015-01-04 20:10:03 PST
The path that's currently stored in LIBEXECDIR is one level lower than the real LIBEXECDIR, which we are going to need. Introduce PKGLIBEXECDIR so that we can use LIBEXECDIR for the real LIBEXECDIR in the future.
Comment 1 Michael Catanzaro 2015-01-04 20:18:54 PST
Created attachment 243958 [details]
Comment 2 Carlos Garcia Campos 2015-01-04 23:21:55 PST
Comment on attachment 243958 [details]

View in context: https://bugs.webkit.org/attachment.cgi?id=243958&action=review

> Source/WebKit2/Shared/gtk/ProcessExecutablePathGtk.cpp:62
> -    return pathByAppendingComponent(filenameToString(LIBEXECDIR), processName);
> +    return pathByAppendingComponent(filenameToString(PKGLIBEXECDIR), processName);

If it's one level lower, how can this work? I'm confused.
Comment 3 Michael Catanzaro 2015-01-05 10:09:15 PST
(In reply to comment #2)
> If it's one level lower, how can this work? I'm confused.

It works because the patch only renames that compile-time constant. Now, as for why we should do it:

$LIBEXEC_INSTALL_DIR is set in OptionsGTK.cmake to $(libexecdir)/webkitgtk-4.0 (that's how we'd write it using Autotools). This is confusing, but it's the result of different naming conventions. The platform-independent WebKit2/CMakeLists.txt, which I did not want to change, installs our helper binaries to $LIBEXEC_INSTALL_DIR, which makes sense if you consider that $LIBEXEC_INSTALL_DIR should be a subdirectory of GNU $(libexecdir) -- we don't install anything directly in $(libexecdir), so this is a reasonable definition for $LIBEXEC_INSTALL_DIR, which doesn't need to be changed.

But $LIBEXEC_INSTALL_DIR is not $(libexecdir), which is how we treat it in our GTK+ specific code, by setting $LIBEXECDIR = $LIBEXEC_INSTALL_DIR. This works because our code currently uses $LIBEXECDIR only for launching our helper binaries in $(libexecdir)/webkitgtk-4.0, but I need to use it to refer to the actual $(libexecdir) and I don't want to type LIBEXEDIR + "../" in order to get to $(libexecdir), which would be very confusing. Now, note that in Autotools, $(libexecdir)/webkitgtk-4.0 would be equivalent to $(pkglibexecdir), so the least-confusing way around this problem is to just set $LIBEXECDIR to what would in Autotools be $(libexecdir) and $PKGLIBEXECDIR to what would in Autotools be $(pkglibexecdir) and what in our build system is $LIBEXEC_INSTALL_DIR.
Comment 4 Michael Catanzaro 2015-07-15 08:09:31 PDT
Comment on attachment 243958 [details]

(More easy patches!)
Comment 5 Martin Robinson 2015-07-15 09:13:34 PDT
(In reply to comment #4)
> Comment on attachment 243958 [details]
> Patch
> (More easy patches!)

Comment 6 Michael Catanzaro 2015-07-15 11:02:01 PDT
I'd rather rename PACKAGE_LOCALE_DIR to LOCALEDIR, for consistency with the familiar GNU directory names; what do you think? To me, the PACKAGE_ prefix is reminiscent of the PKGWHATEVER prefix used in Autotools, which means "a directory named for the project underneath the WHATEVER directory," but that's not what PACKAGE_LOCALE_DIR is.
Comment 7 Michael Catanzaro 2015-07-15 12:04:50 PDT
Martin agreed on IRC to use PKGLIBEXECDIR. (And presumably he wants LOCALEDIR too, for consistency? I will do that in a separate patch after this one lands.)
Comment 8 WebKit Commit Bot 2015-07-15 13:35:30 PDT
Comment on attachment 243958 [details]

Clearing flags on attachment: 243958

Committed r186852: <http://trac.webkit.org/changeset/186852>
Comment 9 WebKit Commit Bot 2015-07-15 13:35:34 PDT
All reviewed patches have been landed.  Closing bug.