Bug 216610 – EWS should have a way to force clean build with patch

• Status: ASSIGNED
• Component: Tools / Tests

[Aakash Jain]

Sometimes webkit build with patch applied fails even though the patch is good, due to build being incremental. In those cases, a clean build with patch is required. 

EWS does a clean build without patch (using ToT), but it doesn't have any option to do clean build with patch. We should add some kind of mechanism to force a clean build with patch when required.

[Radar WebKit Bug Importer]

<rdar://problem/68992714>

[Aakash Jain]

Created attachment 409151 [details]
Patch

[Aakash Jain]

Test runs:

https://ews-build.webkit-uat.org/#/builders/35/builds/6271 (with patch name containing 'clean') - step #3 'clean-webkit' was executed

https://ews-build.webkit-uat.org/#/builders/35/builds/6270 (regular patch) - step #3 (clean-webkit) was skipped

[Aakash Jain]

(In reply to Aakash Jain from comment #2)
> Created attachment 409151 [details]
This patch adds a naive way to force a clean-build-with-patch on EWS, by having the patch name containing the word 'clean'. (Note that patch name can be modified through Bugzilla UI even after uploading it)

We can build a better UX (like a button in UI somewhere), but that would be a much larger task. Also, the requirement to run clean-build-with-patch is occasional. This is a quick attempt at adding atleast some way of achieving this functionality.

[Antoine Quint]

Very nice! Can't really review but am looking forward to using this feature.

[Alexey Proskuryakov]

Comment on attachment 409151 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409151&action=review

> Tools/ChangeLog:9
> +        patch name contains the word 'clean', it would result in a clean build on EWS.

I would suggest a longer trigger, to avoid accidental matches. Maybe "[clean-build]"?

[Fujii Hironori]

If the patch needs clean build for EWS, it also for Buildbot. Patch file name is only applicable EWS. How about an idea including a keyword in ChangeLog or other file in the patch?

[Aakash Jain]

Comment on attachment 409151 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409151&action=review

>> Tools/ChangeLog:9
>> +        patch name contains the word 'clean', it would result in a clean build on EWS.
> 
> I would suggest a longer trigger, to avoid accidental matches. Maybe "[clean-build]"?

Changed to 'clean-build' in updated patch.

[Aakash Jain]

Created attachment 409168 [details]
Patch

[Jonathan Bedard]

Comment on attachment 409168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409168&action=review

> Tools/BuildSlaveSupport/ews-build/steps.py:-164
> -class CleanWorkingDirectory(shell.ShellCommand):

What motivated the rename?

[Aakash Jain]

Comment on attachment 409168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=409168&action=review

>> Tools/BuildSlaveSupport/ews-build/steps.py:-164
>> -class CleanWorkingDirectory(shell.ShellCommand):
> 
> What motivated the rename?

This class/step wasn't used earlier. Now that it is starting to be used, the new name seems more appropriate.

[Alexey Proskuryakov]

My suggestion was to have the brackets too.

I agree with Fujii, it would be good to have a complete plan that includes post-commit bots too.

[Darin Adler]

I don’t think this should be EWS-specific.

If there’s a patch that requires a clean build, we’d like that to also happen on non-EWS buildbots once the patch is landed, and also on all the developers’ machines when they update after the patch is landed, or even if someone downloads and applies it.

We would want to create s scheme for this that’s part of our build system, not part of EWS.

Unless I am missing something?

[Aakash Jain]

Agree. I would look into adding support in build.webkit.org for clean-build for commit (most likely based on keyword in ChangeLog or bug title). On build.webkit.org, buildbot can combine multiple revisions in a single build. I would look into how to access content of every revision in the builds (probably through sourcestamps).

I am not sure what's a good way to achieve clean-build-with-patch on developers’ machines, especially when someone downloads and applies the patch. It might be a larger task and should be tracked separately. It shouldn't block this work. I filed https://bugs.webkit.org/show_bug.cgi?id=216784 to track it. Comments/ideas are welcome.

Note that this patch was a quick attempt (coded and tested in few hours) to improve the current scenario where EWS doesn't have any ability to do clean-build-with-patch, it resulted from a quick slack discussion day before with Antoine when he needed a clean build with patch on multiple platforms on EWS. This wasn't a properly planned and tracked work.

[Aakash Jain]

Created attachment 409392 [details]
Patch

[Aakash Jain]

Updated patch check bug title and run clean-build if bug title contains the keyword: [clean-build].

Similarly, bug 216610 adds ability to build.webkit.org to run clean build if the bug-title in the commit contains this keyword.

[Aakash Jain]

(In reply to Aakash Jain from comment #16)
> Similarly, bug 216610 adds ability to build.webkit.org to run clean build if the bug-title in the commit contains this keyword.
correction: It's bug 216840