Bug 76585 – [GTK] ensure the jhbuild used by webkit is as up-to-date as needed

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Gustavo Noronha (kov)]

[GTK] ensure the jhbuild used by webkit is as up-to-date as needed

[Gustavo Noronha (kov)]

Created attachment 123029 [details]
Patch

[Martin Robinson]

Comment on attachment 123029 [details]
Patch

Won't this break if we bump jhbuild_revision after someone has already called update-webkitgtk-libs?

[Gustavo Noronha (kov)]

Comment on attachment 123029 [details]
Patch

Attachment 123029 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11196981

[Gustavo Noronha (kov)]

Created attachment 123094 [details]
Patch

[Gustavo Noronha (kov)]

Somehow a bad revision hash (it's actually from soup!) sneaked into the patch, I wonder how. This new one should work =)

[Gustavo Noronha (kov)]

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 =)

[Martin Robinson]

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.

[Gustavo Noronha (kov)]

Created attachment 123155 [details]
Patch

[Martin Robinson]

Comment on attachment 123155 [details]
Patch

Thanks!

[Gustavo Noronha (kov)]

Comment on attachment 123155 [details]
Patch

Clearing flags on attachment: 123155

Committed r105472: <http://trac.webkit.org/changeset/105472>

[Gustavo Noronha (kov)]

All reviewed patches have been landed.  Closing bug.