Bug 206061

Summary: [GTK][WPE] EWS should not wipe the JHBuild in the unapply patch step
Product: WebKit Reporter: Carlos Alberto Lopez Perez <clopez>
Component: Tools / TestsAssignee: Carlos Alberto Lopez Perez <clopez>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, bugs-noreply, dbates, dpino, ews-watchlist, glenn, jbedard, psaavedra, slewis, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=202361
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Carlos Alberto Lopez Perez
Reported 2020-01-10 04:53:01 PST
When a patch fails to build or pass tests, the EWS unapplies the patch and tries to run the build or tests again without the patch. The issue is that to unapply the patch, the EWS its currently calling Tools/Scripts/clean-webkit, which wipes everything. That means that the JHBuild (step InstallGtkDependencies) has to be rebuilt from scratch and that its expensive (around 30 minutes even with ccache). The JHBuild scripts already have logic to detect when they should wipe the JHBuild (if a patch touches the moduleset). Ideally we should not be wiping the JHBuild for GTK and WPE to avoid having to rebuild it after unapplying the patch.
Attachments
Patch (5.95 KB, patch)
2020-01-10 08:05 PST, Carlos Alberto Lopez Perez 		no flags
DetailsFormatted DiffDiff
Patch (5.95 KB, patch)
2020-01-10 08:13 PST, Carlos Alberto Lopez Perez 		no flags
DetailsFormatted DiffDiff
Patch (6.00 KB, patch)
2020-01-10 11:04 PST, Carlos Alberto Lopez Perez 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Carlos Alberto Lopez Perez
Comment 1 2020-01-10 08:05:04 PST
Created attachment 387336 [details] Patch
Carlos Alberto Lopez Perez
Comment 2 2020-01-10 08:13:36 PST
Created attachment 387338 [details] Patch typo fix
Aakash Jain
Comment 3 2020-01-10 10:26:20 PST
Comment on attachment 387338 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387338&action=review > Tools/BuildSlaveSupport/ews-build/steps.py:174 > + self.setCommand(self.command + ['--keep-jhbuild-directory']) Please add following line at the end of this start method: return shell.ShellCommand.start(self) This will also fix the unit-tests. > Tools/Scripts/clean-webkit:48 > + if fs.isdir("WebKitBuild"): Would this path work if the script is invoked from another directory?
Carlos Alberto Lopez Perez
Comment 4 2020-01-10 11:03:06 PST
(In reply to Aakash Jain from comment #3) > Comment on attachment 387338 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=387338&action=review > > > Tools/BuildSlaveSupport/ews-build/steps.py:174 > > + self.setCommand(self.command + ['--keep-jhbuild-directory']) > > Please add following line at the end of this start method: > return shell.ShellCommand.start(self) > > This will also fix the unit-tests. > ups, sure. > > Tools/Scripts/clean-webkit:48 > > + if fs.isdir("WebKitBuild"): > > Would this path work if the script is invoked from another directory? Yes and no... The current clean-webkit script its not prepared to be ran outside of the main directory. For example, if you run: cd Tools/Scripts && ./clean-webkit Then the script only cleans the files inside the dir Tools/Scripts/ And if you run from another directory (for example from /tmp) it fails completely (python backtrace) So the clean-webkit script doesn't seem at all prepared to be ran from other directory than the main one, and this patch doesn't change that.
Carlos Alberto Lopez Perez
Comment 5 2020-01-10 11:04:46 PST
Created attachment 387355 [details] Patch Add missing return shell.ShellCommand.start(self)
Carlos Alberto Lopez Perez
Comment 6 2020-01-10 11:36:31 PST
Comment on attachment 387355 [details] Patch Clearing flags on attachment: 387355 Committed r254359: <https://trac.webkit.org/changeset/254359>
Carlos Alberto Lopez Perez
Comment 7 2020-01-10 11:36:36 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 8 2020-01-10 11:37:12 PST
<rdar://problem/58485543>
Note You need to log in before you can comment on or make changes to this bug.