Bug 75857 - GTK+ EWS needs to run update-webkitgtk-libs after applying a patch
Summary: GTK+ EWS needs to run update-webkitgtk-libs after applying a patch
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gustavo Noronha (kov)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-09 07:35 PST by Gustavo Noronha (kov)
Modified: 2012-01-17 11:19 PST (History)
5 users (show)

See Also:


Attachments
Patch (3.42 KB, patch)
2012-01-09 07:41 PST, Gustavo Noronha (kov)
no flags Details | Formatted Diff | Diff
Patch (3.32 KB, patch)
2012-01-16 12:22 PST, Gustavo Noronha (kov)
abarth: review+
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-09 07:35:00 PST
GTK+ EWS needs to run update-webkitgtk-libs after applying a patch
Comment 1 Gustavo Noronha (kov) 2012-01-09 07:41:01 PST
Created attachment 121672 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2012-01-09 07:54:43 PST
We need something like this for our EWS. I don't know the python tools code well, so I made this patch to get feedback on how we should go about implementing this from you EWS masters =)
Comment 3 Adam Barth 2012-01-09 09:30:16 PST
Comment on attachment 121672 [details]
Patch

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

> Tools/Scripts/webkitpy/tool/bot/earlywarningsystemtask.py:79
> +class GtkEarlyWarningSystemTask(EarlyWarningSystemTask):
> +    def _buid(self):
> +        path = os.path.join(os.dirname(self._tool.path), 'update-webkitgtk-libs')
> +        self._tool.executive.run_command([path])
> +        EarlyWarningSystemTask._build()

This isn't the right approach.  Port-specific logic should be in the Port classes:

http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/ports.py

Really, you should consider making this part of "update-webkit --gtk", maybe with some extra command line flag.
Comment 4 Gustavo Noronha (kov) 2012-01-11 05:33:50 PST
> http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/ports.py
> 
> Really, you should consider making this part of "update-webkit --gtk", maybe with some extra command line flag.

Is that run after the patch has been applied? What we'd like to have is running that command after applying the patch and before build. Would making it part of update-webkit --gtk give us that? If not, do you have another approach to recommend? Thanks!
Comment 5 Adam Barth 2012-01-11 10:19:41 PST
Ah, I understand.  The Chromium port has a similar issue.  If you look at:

http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/ports.py#L239

you see that we pass --update-chromium to build-webkit.  That causes build-webkit to make sure everything is updated properly.  You might want to add a --update-gtk flag that's similar.
Comment 6 Gustavo Noronha (kov) 2012-01-16 05:28:47 PST
(In reply to comment #5)
> Ah, I understand.  The Chromium port has a similar issue.  If you look at:
> 
> http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/ports.py#L239
> 
> you see that we pass --update-chromium to build-webkit.  That causes build-webkit to make sure everything is updated properly.  You might want to add a --update-gtk flag that's similar.

Aha, thanks Adam, that's a great idea! I'll get an updated patch up later today =)
Comment 7 Gustavo Noronha (kov) 2012-01-16 12:22:08 PST
Created attachment 122673 [details]
Patch
Comment 8 Adam Barth 2012-01-17 01:14:47 PST
Comment on attachment 122673 [details]
Patch

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

> Tools/Scripts/webkitdirs.pm:1843
> +        system "rm", "-rf", "$dir";

That's scary!
Comment 9 Gustavo Noronha (kov) 2012-01-17 04:29:54 PST
Committed r105142: <http://trac.webkit.org/changeset/105142>
Comment 10 Csaba Osztrogonác 2012-01-17 06:41:52 PST
It broke a python unittest:

FAIL: test_gtk_port (webkitpy.common.config.ports_unittest.WebKitPortTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/ramdisk/qt-linux-release/build/Tools/Scripts/webkitpy/common/config/ports_unittest.py", line 54, in test_gtk_port
    self.assertEquals(GtkPort.build_webkit_command(), WebKitPort.script_shell_command("build-webkit") + ["--gtk", WebKitPort.makeArgs()])
AssertionError: ['Tools/Scripts/build-webkit', '--gtk', '--update-gtk', '--makeargs="-j16"'] != ['Tools/Scripts/build-webkit', '--gtk', '--makeargs="-j16"']
Comment 11 Gustavo Noronha (kov) 2012-01-17 11:19:58 PST
Oops, I pushed a fix for this already, sorry for the noise, I forgot to check for unit tests =(