Bug 206061 – [GTK][WPE] EWS should not wipe the JHBuild in the unapply patch step

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Carlos Alberto Lopez Perez]

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.

[Carlos Alberto Lopez Perez]

Created attachment 387336 [details]
Patch

[Carlos Alberto Lopez Perez]

Created attachment 387338 [details]
Patch

typo fix

[Aakash Jain]

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]

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

Created attachment 387355 [details]
Patch

Add missing return shell.ShellCommand.start(self)

[Carlos Alberto Lopez Perez]

Comment on attachment 387355 [details]
Patch

Clearing flags on attachment: 387355

Committed r254359: <https://trac.webkit.org/changeset/254359>

[Carlos Alberto Lopez Perez]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/58485543>