Bug 117275 – [GTK][EFL] Use function jhbuildWrapperPrefixIfNeeded to run launcher

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Diego Pino]

The code that runs the jhbuild-wrapper in './Tools/Scripts/run-launcher' composes the path and parameters by itself. There's a function, jhbuildWrapperPrefixIfNeeded(), in the Webkitdirs module that does that.

[Diego Pino]

Created attachment 203879 [details]
Patch

[Martin Robinson]

Comment on attachment 203879 [details]
Patch

Nice.

[Carlos Garcia Campos]

*** Bug 116095 has been marked as a duplicate of this bug. ***

[Carlos Garcia Campos]

Comment on attachment 203879 [details]
Patch

This patch doesn't work when not using jhbuild, see:

$ Tools/Scripts/run-launcher --gtk
Starting webkit launcher.
Use of uninitialized value $launcherPath in exec at Tools/Scripts/run-launcher line 87.
Can't exec "": No existe el archivo o el directorio at Tools/Scripts/run-launcher line 87.
Died at Tools/Scripts/run-launcher line 87.

[Diego Pino]

OK, I think the reason is because the function jhbuildWrapperPrefixIfNeeded() tests first if the jhbuildPath exists. If not, returns an empty path.

What I don't understand is that running the script using jhbuildWrapperPrefixIfNeeded() and not using it builds the same path:

$ ./Tools/Scripts/run-launcher --gtk
without using jhbuildWrapperPrefixIfNeeded: /home/dpino/workspace/WebKit/Tools/jhbuild/jhbuild-wrapper
using jhbuildWrapperPrefixIfNeeded: /home/dpino/workspace/WebKit/Tools/jhbuild/jhbuild-wrapper

I could remove the checking of "jhbuildPath exists" in jhbuildWrapperPrefixIfNeeded() but that seems wrong.

[Carlos Garcia Campos]

(In reply to comment #5)
> OK, I think the reason is because the function jhbuildWrapperPrefixIfNeeded() tests first if the jhbuildPath exists. If not, returns an empty path.

In case of empty path we need to use the launcher path directly.

[Diego Pino]

(In reply to comment #6)
> (In reply to comment #5)
> > OK, I think the reason is because the function jhbuildWrapperPrefixIfNeeded() tests first if the jhbuildPath exists. If not, returns an empty path.
> 
> In case of empty path we need to use the launcher path directly.

OK, so regardless the path is empty or not, it seems the launcher path is always the same. If that's the case what I could do is to add a buildLauncherPath() method to webkitdirs.pm and make the function jhbuildWrapperPrefixIfNeeded() to use buildLauncherPath() if the path is not empty. The patch proposed will use buildLauncherPath() instead of jhbuildWrapperPrefixIfNeeded(). 

What do you think?

[Carlos Garcia Campos]

(In reply to comment #7)
> (In reply to comment #6)
> > (In reply to comment #5)
> > > OK, I think the reason is because the function jhbuildWrapperPrefixIfNeeded() tests first if the jhbuildPath exists. If not, returns an empty path.
> > 
> > In case of empty path we need to use the launcher path directly.
> 
> OK, so regardless the path is empty or not, it seems the launcher path is always the same. If that's the case what I could do is to add a buildLauncherPath() method to webkitdirs.pm and make the function jhbuildWrapperPrefixIfNeeded() to use buildLauncherPath() if the path is not empty. The patch proposed will use buildLauncherPath() instead of jhbuildWrapperPrefixIfNeeded(). 
> 
> What do you think?

I don't think so, jhbuildWrapperPrefixIfNeeded() return the jhbuild prefix if needed. So we need to build the launcher path, Programs/GtkLauncher, for example, and if there's a jhbuild prefix, prepend it so what you run in the end is jhbuild run Programs/GtkLauncher and if there's not jhbuild prefix you run Programs/GtkLauncher directly.

[Alberto Garcia]

Created attachment 220510 [details]
Patch

(In reply to comment #5)
> OK, I think the reason is because the function
> jhbuildWrapperPrefixIfNeeded() tests first if the jhbuildPath
> exists. If not, returns an empty path.

That function returns an empty list, so when do you

($launcherPath, my @args) = jhbuildWrapperPrefixIfNeeded();

you're turning $launcherPath into an undefined variable, which you
then are trying to exec.

I reworked the script a bit, now there's a scalar variable for the
launcher command, an array for the jhbuild wrapper and its options and
@ARGV, with the command-line parameters set by the user, which I
decided to leave untouched.

Hope it's a bit clearer now.

[Alberto Garcia]

Created attachment 220511 [details]
Patch

There was actually a duplicate variable in that script, I updated the
patch to remove it as well.

[Alberto Garcia]

Created attachment 220512 [details]
Patch

Today is not my day. This is the correct one.

[Carlos Garcia Campos]

Comment on attachment 220512 [details]
Patch

Thank you very much!

[Alberto Garcia]

Committed r161419: <http://trac.webkit.org/changeset/161419>