Bug 75857 – GTK+ EWS needs to run update-webkitgtk-libs after applying a patch

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Gustavo Noronha (kov)]

GTK+ EWS needs to run update-webkitgtk-libs after applying a patch

[Gustavo Noronha (kov)]

Created attachment 121672 [details]
Patch

[Gustavo Noronha (kov)]

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

[Adam Barth]

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.

[Gustavo Noronha (kov)]

> 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!

[Adam Barth]

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.

[Gustavo Noronha (kov)]

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

[Gustavo Noronha (kov)]

Created attachment 122673 [details]
Patch

[Adam Barth]

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!

[Gustavo Noronha (kov)]

Committed r105142: <http://trac.webkit.org/changeset/105142>

[Csaba Osztrogonác]

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"']

[Gustavo Noronha (kov)]

Oops, I pushed a fix for this already, sorry for the noise, I forgot to check for unit tests =(