Bug 140070

Created attachment 243958 [details] Patch

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.

(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 on attachment 243958 [details] Patch (More easy patches!)

(In reply to comment #4) > Comment on attachment 243958 [details] > Patch > > (More easy patches!) Perhaps PACKAGE_LIBEXECDIR or LIBEXEC_INSTALL_DIR for consistency?

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.

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 on attachment 243958 [details] Patch Clearing flags on attachment: 243958 Committed r186852: <http://trac.webkit.org/changeset/186852>

All reviewed patches have been landed. Closing bug.