Bug 140070

Summary: [GTK] Rename LIBEXECDIR to PKGLIBEXECDIR
Product: WebKit Reporter: Michael Catanzaro <mcatanzaro>
Component: WebKitGTKAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: cgarcia, commit-queue, mcatanzaro, mrobinson, zan
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 140068    
Bug Blocks: 110014    
Attachments:
Description Flags
Patch none

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]
Patch
Comment 2 Carlos Garcia Campos 2015-01-04 23:21:55 PST
Comment on attachment 243958 [details]
Patch

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]
Patch

(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!)

Perhaps PACKAGE_LIBEXECDIR or LIBEXEC_INSTALL_DIR for consistency?
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]
Patch

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.