Bug 76161 - [GTK] jhbuild cloning is not following WEBKITOUTPUTDIR.
Summary: [GTK] jhbuild cloning is not following WEBKITOUTPUTDIR.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Major
Assignee: Gustavo Noronha (kov)
URL:
Keywords:
: 80698 81475 (view as bug list)
Depends on:
Blocks: 79480
  Show dependency treegraph
 
Reported: 2012-01-12 02:56 PST by Alexis Menard (darktears)
Modified: 2012-05-09 06:41 PDT (History)
7 users (show)

See Also:


Attachments
Patch (5.97 KB, patch)
2012-04-20 15:35 PDT, Gustavo Noronha (kov)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexis Menard (darktears) 2012-01-12 02:56:15 PST
When calling the build (totally clean build) like this :

WEBKITOUTPUTDIR=/home/darktears/dev/troll/webkit-gtk build-webkit --gtk --release

it keeps looping infinitely to detect jhbuild.

http://pastebin.com/aMASEh4v shows the infinite loop.


It's a annoying regression for people who want to test multiple port with the same source directory.
Comment 1 Martin Robinson 2012-01-12 08:48:15 PST
I cannot reproduce this one. Do you mind deleting WebKitBuild/Dependencies and trying again?
Comment 2 Alexis Menard (darktears) 2012-01-12 09:04:33 PST
(In reply to comment #1)
> I cannot reproduce this one. Do you mind deleting WebKitBuild/Dependencies and trying again?

Did it clones again jhbuild and keep looping for ever until the death of my machine.
Comment 3 Alexis Menard (darktears) 2012-01-12 09:13:37 PST
It clones in WebKitBuild in my source dir, maybe it shouldn't "pollute" the source dir.
Comment 4 Martin Robinson 2012-01-12 09:26:28 PST
(In reply to comment #3)
> It clones in WebKitBuild in my source dir, maybe it shouldn't "pollute" the source dir.

That's true, but this is unrelated to the bug. Do you mind trying to install the gnome-common package (or whatever brings in /usr/bin/gnome-autogen.sh) and trying again?
Comment 5 Alexis Menard (darktears) 2012-01-12 09:36:45 PST
Same but I found suspicious this line :

Don't forget to create ~/.jhbuildrc
install -m755 install-check /home/darktears/dev/troll//qt-50-debug/qtbase/bin/install-check

why it would put stuff in my Qt installation??? Just for the record it is my first entry in PATH.
Comment 6 Martin Robinson 2012-01-12 09:46:58 PST
(In reply to comment #5)

> why it would put stuff in my Qt installation??? Just for the record it is my first entry in PATH.

Perhaps this is a bug in the fallback jhbuild build script. It shouldn't be installing anything outside the prefix.
Comment 7 Balazs Kelemen 2012-03-09 08:52:00 PST
*** Bug 80698 has been marked as a duplicate of this bug. ***
Comment 8 Alexis Menard (darktears) 2012-04-19 15:59:26 PDT
(In reply to comment #7)
> *** Bug 80698 has been marked as a duplicate of this bug. ***

/home/darktears/dev/troll/webkit/WebKitBuild/Dependencies/Source/jhbuild/jhbuild is where it is cloned.
Comment 9 Alexis Menard (darktears) 2012-04-19 16:01:16 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > *** Bug 80698 has been marked as a duplicate of this bug. ***
> 
> /home/darktears/dev/troll/webkit/WebKitBuild/Dependencies/Source/jhbuild/jhbuild is where it is cloned.

Also gnome-common seems to be a hard dependency, the fallback script of jhbuild without autotool is proper broken.
Comment 10 Gustavo Noronha (kov) 2012-04-20 15:35:30 PDT
Created attachment 138178 [details]
Patch
Comment 11 Gustavo Noronha (kov) 2012-04-20 15:36:41 PDT
Should we add gnome-common to jhbuild, or do you think it would be better as a dependency we expect the system to provide?
Comment 12 Gustavo Noronha (kov) 2012-04-20 16:05:44 PDT
I added gnome-common as a dependency on the wiki.
Comment 13 Martin Robinson 2012-04-20 16:07:52 PDT
Comment on attachment 138178 [details]
Patch

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.
Comment 14 Raphael Kubo da Costa (:rakuco) 2012-04-24 16:16:43 PDT
*** Bug 81475 has been marked as a duplicate of this bug. ***
Comment 15 Gustavo Noronha (kov) 2012-04-27 22:51:24 PDT
Committed r115532: <http://trac.webkit.org/changeset/115532>
Comment 16 Philippe Normand 2012-05-03 07:24:50 PDT
(In reply to comment #15)
> Committed r115532: <http://trac.webkit.org/changeset/115532>

So this patch restored the behavior to store the jhbuild*.md5sum files in the build directory instead of Dependencies. Is there any reason why?
Comment 17 Philippe Normand 2012-05-03 07:29:16 PDT
(In reply to comment #16)
> (In reply to comment #15)
> > Committed r115532: <http://trac.webkit.org/changeset/115532>
> 
> So this patch restored the behavior to store the jhbuild*.md5sum files in the build directory instead of Dependencies. Is there any reason why?

Hum doesn't seem to be this one. Anyway, something broke :( Will try to track this regression down again.
Comment 18 Philippe Normand 2012-05-03 07:47:19 PDT
Comment on attachment 138178 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138178&action=review

> Tools/Scripts/webkitdirs.pm:1846
> +    return join(baseProductDir(), "Dependencies");

Hrm. the first argument to join() should be the separator no??

> Tools/Scripts/webkitdirs.pm:1991
> +        my $destination = join(getJhbuildPath(), $file);

Ditto.
Comment 19 Philippe Normand 2012-05-03 09:53:13 PDT
Comment on attachment 138178 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138178&action=review

>> Tools/jhbuild/jhbuild-wrapper:48
>> +
> 
> 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.

Looks like this suggestion wasn't correctly applied :( Current code:


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

so we end up with installation_prefix ending with Root/Root ... Will send another patch.
Comment 20 Philippe Normand 2012-05-03 09:59:03 PDT
Oh this was fixored in r115904... My bad for not updating before blaming :)
Comment 21 Gustavo Noronha (kov) 2012-05-09 06:41:53 PDT
Wow, and I thought I had tested it properly =) thanks for your investigations, hopefully it's all fixed now.