Summary: | GTK+ EWS needs to run update-webkitgtk-libs after applying a patch | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> | ||||||
Component: | New Bugs | Assignee: | 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
Gustavo Noronha (kov)
2012-01-09 07:35:00 PST
Created attachment 121672 [details]
Patch
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 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. > 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!
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. (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 =) Created attachment 122673 [details]
Patch
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! Committed r105142: <http://trac.webkit.org/changeset/105142> 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"'] Oops, I pushed a fix for this already, sorry for the noise, I forgot to check for unit tests =( |