Bug 76585 - [GTK] ensure the jhbuild used by webkit is as up-to-date as needed
Summary: [GTK] ensure the jhbuild used by webkit is as up-to-date as needed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gustavo Noronha (kov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-18 15:57 PST by Gustavo Noronha (kov)
Modified: 2012-01-19 18:45 PST (History)
3 users (show)

See Also:


Attachments
Patch (3.42 KB, patch)
2012-01-18 15:59 PST, Gustavo Noronha (kov)
no flags Details | Formatted Diff | Diff
Patch (3.43 KB, patch)
2012-01-19 03:21 PST, Gustavo Noronha (kov)
no flags Details | Formatted Diff | Diff
Patch (5.84 KB, patch)
2012-01-19 11:22 PST, 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 Gustavo Noronha (kov) 2012-01-18 15:57:09 PST
[GTK] ensure the jhbuild used by webkit is as up-to-date as needed
Comment 1 Gustavo Noronha (kov) 2012-01-18 15:59:14 PST
Created attachment 123029 [details]
Patch
Comment 2 Martin Robinson 2012-01-18 17:51:34 PST
Comment on attachment 123029 [details]
Patch

Won't this break if we bump jhbuild_revision after someone has already called update-webkitgtk-libs?
Comment 3 Gustavo Noronha (kov) 2012-01-18 20:42:03 PST
Comment on attachment 123029 [details]
Patch

Attachment 123029 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11196981
Comment 4 Gustavo Noronha (kov) 2012-01-19 03:21:53 PST
Created attachment 123094 [details]
Patch
Comment 5 Gustavo Noronha (kov) 2012-01-19 03:29:47 PST
Somehow a bad revision hash (it's actually from soup!) sneaked into the patch, I wonder how. This new one should work =)
Comment 6 Gustavo Noronha (kov) 2012-01-19 03:51:48 PST
Comment on attachment 123094 [details]
Patch

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

> Tools/gtk/run-with-jhbuild:23
> +jhbuild_revision = '1eedc423f75c605224b430579e4c303292199507'

(In reply to comment #2)
> (From update of attachment 123029 [details])
> Won't this break if we bump jhbuild_revision after someone has already called update-webkitgtk-libs?

It won't, my main concern when writing this patch was actually to bring existing jhbuild checkouts up-to-date, for cases like the one we had recently, where we required a bug fix, so we don't need to manually update it. Here's how it works:

> Tools/gtk/run-with-jhbuild:83
> +    process = subprocess.Popen(['git', 'rev-list', 'HEAD^..'], cwd=jhbuild_source_path,
> +                               stdout=subprocess.PIPE, stderr=subprocess.PIPE)

This code is here to check that we have the desired revision - this is the code path we'll hit when jhbuild is already cloned/installed. Then it calls install_and_run_jhbuild(), where this change makes it do the right thing:

> Tools/gtk/run-with-jhbuild:44
> +    else:
> +        process = subprocess.Popen(['git', 'remote', 'update', 'origin'], cwd=source_path)
> +        process.wait()
> +        if process.returncode != 0:
> +            raise Exception('jhbuild remote update origin failed with return code: %i' % process.returncode)

This else block is the code path we'll hit in this case as well - it will run git remote update origin to fetch the new data from the repository, = so that the checkout that follows is able to checkout the revision =)
Comment 7 Martin Robinson 2012-01-19 09:17:26 PST
Comment on attachment 123094 [details]
Patch

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

>> Tools/gtk/run-with-jhbuild:83
>> +                               stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> 
> This code is here to check that we have the desired revision - this is the code path we'll hit when jhbuild is already cloned/installed. Then it calls install_and_run_jhbuild(), where this change makes it do the right thing:

Ah, I get it now!

I would suggest a small refactor. Make a helper function called install_or_update_jhbuild that does the outer check and then move this code and the code above to a helper function called "update_jhbuild." This patch has the unintended consequence of updating jhbuild modules even when the user didn't request it.
Comment 8 Gustavo Noronha (kov) 2012-01-19 11:22:51 PST
Created attachment 123155 [details]
Patch
Comment 9 Martin Robinson 2012-01-19 11:27:51 PST
Comment on attachment 123155 [details]
Patch

Thanks!
Comment 10 Gustavo Noronha (kov) 2012-01-19 18:45:27 PST
Comment on attachment 123155 [details]
Patch

Clearing flags on attachment: 123155

Committed r105472: <http://trac.webkit.org/changeset/105472>
Comment 11 Gustavo Noronha (kov) 2012-01-19 18:45:35 PST
All reviewed patches have been landed.  Closing bug.