I think this is only useful for internal tools and tests, but never when installed, since we don't install the processes in the bin dir but in the libexec dir. We already have an env var to cover the cases of the internal tools and tests, so I think it would be better to use that variable only.
Created attachment 236271 [details] Patch
This will mean we need to set WEBKIT_EXEC_PATH for running MiniBrowser directly, right?
(In reply to comment #2) > This will mean we need to set WEBKIT_EXEC_PATH for running MiniBrowser directly, right? Yes, that's what the patch does. And I don't like it, because downstream need to patch it to remove that. I think we could pass DEVELOPER_MODE to the compiler and only set the envs for developer builds #ifdef DEVELOPER_MODE g_setenv("WEBKIT_EXEC_PATH", WEBKIT_EXEC_PATH, FALSE); g_setenv("WEBKIT_INJECTED_BUNDLE_PATH", WEBKIT_INJECTED_BUNDLE_PATH, FALSE); #endif Or only pass WEBKIT_EXEC_PATH and WEBKIT_INJECTED_BUNDLE_PATH for developer builds and check those directly.
(In reply to comment #3) > (In reply to comment #2) > > This will mean we need to set WEBKIT_EXEC_PATH for running MiniBrowser directly, right? > > Yes, that's what the patch does. And I don't like it, because downstream need to patch it to remove that. I think we could pass DEVELOPER_MODE to the compiler and only set the envs for developer builds > > #ifdef DEVELOPER_MODE > g_setenv("WEBKIT_EXEC_PATH", WEBKIT_EXEC_PATH, FALSE); > g_setenv("WEBKIT_INJECTED_BUNDLE_PATH", WEBKIT_INJECTED_BUNDLE_PATH, FALSE); > #endif > > Or only pass WEBKIT_EXEC_PATH and WEBKIT_INJECTED_BUNDLE_PATH for developer builds and check those directly. hmm, wait, but Tools (including MB) are not build on non developer builds, or does debian also patch that? Berto?
(In reply to comment #4) > hmm, wait, but Tools (including MB) are not build on non developer > builds, or does debian also patch that? Berto? In Debian I'm patching CMakeLists.txt to build the MiniBrowser, the rest of the tools we don't need. I also remove the g_setenv("WEBKIT_INJECTED_BUNDLE_PATH", ...) call from main.c.
(In reply to comment #5) > (In reply to comment #4) > > hmm, wait, but Tools (including MB) are not build on non developer > > builds, or does debian also patch that? Berto? > > In Debian I'm patching CMakeLists.txt to build the MiniBrowser, the > rest of the tools we don't need. > > I also remove the g_setenv("WEBKIT_INJECTED_BUNDLE_PATH", ...) call > from main.c. Ok, so you will have to keep patching it anyway, so we either leave it as it is, or build MB unconditionally. MB doesn't use any internal symbol, so it shouldn't be a problem.
(In reply to comment #6) > Ok, so you will have to keep patching it anyway Yeah, that's not a problem, it's a trivial patch anyway. The only alternative would be that we want to install MiniBrowser by default upstream, which I don't think it's the case?
(In reply to comment #7) > (In reply to comment #6) > > Ok, so you will have to keep patching it anyway > > Yeah, that's not a problem, it's a trivial patch anyway. > > The only alternative would be that we want to install MiniBrowser by > default upstream, which I don't think it's the case? I don't see why not. It's like gtk3-demo, for example, or poppler-glib-demo. We can add a specific compile option to disable it (not the installation, but the whole MB), and enable it always for developer builds, since it's required/expected by the layout tests.
(In reply to comment #8) > > The only alternative would be that we want to install MiniBrowser > > by default upstream, which I don't think it's the case? > I don't see why not. It's like gtk3-demo, for example, or > poppler-glib-demo. Ok, why are not we installing it then? Anyway, if we add an option to install it I think we should disable the hardcoded WEBKIT_INJECTED_BUNDLE_PATH somehow.
Created attachment 248327 [details] New patch This patch introduces DEVELOPMENT_BUILD, and only when defined we look for child processes in UI process bin dir, or even in WEBKIT_EXEC_PATH. And the same for the WEBKIT_INJECTED_BUNDLE_PATH env var.
Committed r181392: <http://trac.webkit.org/changeset/181392>