Bug 75857

Summary: GTK+ EWS needs to run update-webkitgtk-libs after applying a patch
Product: WebKit Reporter: Gustavo Noronha (kov) <gustavo>
Component: New BugsAssignee: Gustavo Noronha (kov) <gustavo>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, ojan, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch abarth: review+

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