[webkit-reviews] review granted: [Bug 76161] [GTK] jhbuild cloning is not following WEBKITOUTPUTDIR. : [Attachment 138178] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has granted Gustavo Noronha (kov)
<gns at gnome.org>'s request for review:
Bug 76161: [GTK] jhbuild cloning is not following WEBKITOUTPUTDIR.
https://bugs.webkit.org/show_bug.cgi?id=76161

Attachment 138178: Patch
https://bugs.webkit.org/attachment.cgi?id=138178&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=138178&action=review


Great! There are just a few minor change suggestions below.

> Tools/jhbuild/jhbuild-wrapper:48
> +if os.environ.has_key('WEBKITOUTPUTDIR'):
> +    installation_prefix =
os.path.abspath(os.path.join(os.environ['WEBKITOUTPUTDIR'], 'Dependencies',
'Root'))
> +    source_path =
os.path.abspath(os.path.join(os.environ['WEBKITOUTPUTDIR'], 'Dependencies',
'Source'))
> +    jhbuild_source_path = os.path.join(source_path, 'jhbuild')
> +    jhbuild_path = os.path.join(installation_prefix, 'bin', 'jhbuild')
> +else:
> +    installation_prefix = os.path.abspath(top_level_path('WebKitBuild',
'Dependencies', 'Root'))
> +    source_path = os.path.abspath(top_level_path('WebKitBuild',
'Dependencies', 'Source'))
> +    jhbuild_source_path = os.path.join(source_path, 'jhbuild')
> +    jhbuild_path = top_level_path('WebKitBuild', 'Dependencies', 'Root',
'bin', 'jhbuild')
> +

I think in this case something like this might be better:

if os.environ.has_key('WEBKITOUTPUTDIR'):
    dependencies_path = os.path.abspath(top_level_path('WebKitBuild',
'Dependencies'))
else:
    dependencies_path = top_level_path('WebKitBuild', 'Dependencies')

And then you can make all paths relative to those. I think a change like this
makes sense for the previous two code blocks too.