216840 2020-09-22 12:32:20 -0700 build.webkit.org should run clean build while building revisions having a specified keyword 2026-01-12 09:09:01 -0800 1 1 1 Unclassified WebKit Tools / Tests Other Unspecified Unspecified ASSIGNED https://bugs.webkit.org/show_bug.cgi?id=216610 InRadar P2 Normal --- 1 aakash_jain aakash_jain aakash_jain ap clopez darin fujii graouts jbedard ryanhaddad webkit-bug-importer oldest_to_newest 1690820 0 aakash_jain 2020-09-22 12:32:20 -0700 build.webkit.org should automatically run clean build while building revisions which have a specified keyword, like [clean-build]. Similar feature being added in EWS in Bug 216610. 1690822 1 409389 aakash_jain 2020-09-22 12:35:27 -0700 Created attachment 409389 Patch 1690828 2 darin 2020-09-22 12:51:27 -0700 The focus on bots exclusively seems wrong here. What about when human beings pull revisions with this keyword? 1691147 3 aakash_jain 2020-09-23 09:49:41 -0700 I agree that we should solve that problem as well (engineers needing clean build on their machine after a specific revision). It's a much larger task though. In case of buildbot (e.g.: build.webkit.org), it's straight-forward, buildbot already know which specific revisions are being built, and can check if any of those revisions needs a clean build (based on keyword like [clean-build]). For engineer's machines, we would need to figure out which revision was last built, and which revision is currently being build, so that we can check that revision range for [clean-build] keyword. We would probably need to store the last built revision number locally somewhere. Then we might need to support various mechanism in which engineers can build locally on their machine ,e.g.: build-webkit script, make, xcode etc. Do you think that work should block this patch, or we can land this patch (and patch in 216610), and tackle the clean-build-on-engineers-machine problem afterwards? 1691150 4 darin 2020-09-23 09:53:23 -0700 (In reply to Aakash Jain from comment #3) > Do you think that work should block this patch I do, but others may not agree with me. Adding more differences between bots and desktop way of building worry me. 1693086 5 webkit-bug-importer 2020-09-29 12:33:19 -0700 <rdar://problem/69758937> 1693634 6 clopez 2020-09-30 19:56:52 -0700 (In reply to Aakash Jain from comment #3) > For engineer's machines, we would need to figure out which revision was last > built, and which revision is currently being build, so that we can check > that revision range for [clean-build] keyword. We would probably need to > store the last built revision number locally somewhere. Then we might need > to support various mechanism in which engineers can build locally on their > machine ,e.g.: build-webkit script, make, xcode etc. Just sharing a possible idea of how this could be implemented: 1. Instead of relying on the commit log, rely on a new file on the main directory of the source three called CleanBuildsChangeLog or something like that. So when a clean build is needed, the commit should modify that file adding a new changelog entry with (ideally) the reason about why the clean build is needed. 2. The Script build-webkit calculates the hash (md5 or sha256) of the CleanBuildsChangeLog file and checks if there is a file in the build directory named WebKitBuild/Release/.cleanbuild.hashsum (or something similar). 3. If the .cleanbuild.hashsum is not there, then the script simply saves the value of the hash into the file .cleanbuild.hashsum and continues as usual. 4. If the file .cleanbuild.hashsum is there, the script compares both values. If the values are different, then it does a clean build. This should work for the GTK/WPE bots/developers becase we all use the script build-webkit. But I have no idea if this would work for the developers using Xcode. WDYT? 1693635 7 clopez 2020-09-30 19:59:25 -0700 (In reply to Carlos Alberto Lopez Perez from comment #6) > 4. If the file .cleanbuild.hashsum is there, the script compares both > values. If the values are different, then it does a clean build. ... And then it should save the new value of the hash after wiping the previous build and before starting the new one. 1693636 8 clopez 2020-09-30 20:05:53 -0700 Also I think it would be useful to have a way of ordering clean builds for a specific port. For example, the GTK developers update the flatpak SDK so they want to do clean builds for all the GTK developers. But that should not trigger clean builds for other ports. So on top of the main CleanBuildsChangeLog file there can be a CleanBuilds${PORTNAME}ChangeLog file and the script can calculate the hash of both files concatenated. That way it would be possible to order clean builds for only one port. 1693757 9 darin 2020-10-01 09:00:21 -0700 (In reply to Carlos Alberto Lopez Perez from comment #6) > WDYT? I think a scheme roughly like this is exactly what I had in mind. It can definitely work with Xcode, but not if the logic is in the build-webkit script. 1693758 10 darin 2020-10-01 09:02:52 -0700 (In reply to Darin Adler from comment #9) > I think a scheme roughly like this is exactly what I had in mind. It can > definitely work with Xcode, but not if the logic is in the build-webkit > script. Need a separate script that does this, and is called as an early-enough build step. One issue with Xcode and any other build technique other than using build-webkit is this isn’t perfectly compatible with building in only one directory. That can be addressed by making the build fail instead of auto-cleaning in cases like. 409389 2020-09-22 12:35:27 -0700 2020-09-22 12:35:27 -0700 Patch bug-216840-20200922153526.patch text/plain 1995 aakash_jain SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDI2NzQzMykKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE1IEBACisyMDIwLTA5LTIyICBBYWthc2ggSmFpbiAgPGFha2FzaF9qYWluQGFwcGxlLmNv bT4KKworICAgICAgICBidWlsZC53ZWJraXQub3JnIHNob3VsZCBydW4gY2xlYW4gYnVpbGQgd2hp bGUgYnVpbGRpbmcgcmV2aXNpb25zIGhhdmluZyBhIHNwZWNpZmllZCBrZXl3b3JkCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTY4NDAKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIEJ1aWxkU2xhdmVTdXBw b3J0L2J1aWxkLndlYmtpdC5vcmctY29uZmlnL3N0ZXBzLnB5OgorICAgICAgICAoQ2xlYW5CdWls ZElmU2NoZWR1bGVkKToKKyAgICAgICAgKENsZWFuQnVpbGRJZlNjaGVkdWxlZC5uZWVkX2NsZWFu X2J1aWxkX2Jhc2VkX29uX2NoYW5nZXMpOiBNZXRob2QgdG8gY2hlY2sgaWYgY2xlYW4gYnVpbGQg aXMgbmVlZGVkIGJhc2VkIG9uIHJldmlzaW9ucy4KKyAgICAgICAgKENsZWFuQnVpbGRJZlNjaGVk dWxlZC5zdGFydCk6CisKIDIwMjAtMDktMjIgIEtlaXRoIFJvbGxpbiAgPGtyb2xsaW5AYXBwbGUu Y29tPgogCiAgICAgICAgIFJlZmFjdG9yIGJ1aWxkIHJ1bGVzIGluIE1ha2VmaWxlcyBhbmQgTWFr ZWZpbGUuc2hhcmVkCkluZGV4OiBUb29scy9CdWlsZFNsYXZlU3VwcG9ydC9idWlsZC53ZWJraXQu b3JnLWNvbmZpZy9zdGVwcy5weQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9CdWlsZFNsYXZlU3VwcG9y dC9idWlsZC53ZWJraXQub3JnLWNvbmZpZy9zdGVwcy5weQkocmV2aXNpb24gMjY3NDMzKQorKysg VG9vbHMvQnVpbGRTbGF2ZVN1cHBvcnQvYnVpbGQud2Via2l0Lm9yZy1jb25maWcvc3RlcHMucHkJ KHdvcmtpbmcgY29weSkKQEAgLTE1OSw4ICsxNTksMTkgQEAgY2xhc3MgQ2xlYW5CdWlsZElmU2No ZWR1bGVkKHNoZWxsLkNvbXBpbAogICAgIGRlc2NyaXB0aW9uID0gWyJkZWxldGluZyBXZWJLaXRC dWlsZCBkaXJlY3RvcnkiXQogICAgIGRlc2NyaXB0aW9uRG9uZSA9IFsiZGVsZXRlZCBXZWJLaXRC dWlsZCBkaXJlY3RvcnkiXQogICAgIGNvbW1hbmQgPSBbInB5dGhvbiIsICIuL1Rvb2xzL0J1aWxk U2xhdmVTdXBwb3J0L2NsZWFuLWJ1aWxkIiwgV2l0aFByb3BlcnRpZXMoIi0tcGxhdGZvcm09JShm dWxsUGxhdGZvcm0pcyIpLCBXaXRoUHJvcGVydGllcygiLS0lKGNvbmZpZ3VyYXRpb24pcyIpXQor ICAgIGNsZWFuX2J1aWxkX3N0cmluZyA9ICdbY2xlYW4tYnVpbGRdJworCisgICAgZGVmIG5lZWRf Y2xlYW5fYnVpbGRfYmFzZWRfb25fY2hhbmdlcyhzZWxmKToKKyAgICAgICAgc291cmNlc3RhbXAg PSBzZWxmLmJ1aWxkLmdldFNvdXJjZVN0YW1wKCkKKyAgICAgICAgaWYgbm90IHNvdXJjZXN0YW1w LmNoYW5nZXM6CisgICAgICAgICAgICByZXR1cm4KKyAgICAgICAgZm9yIGNoYW5nZSBpbiBzb3Vy Y2VzdGFtcC5jaGFuZ2VzOgorICAgICAgICAgICAgaWYgc2VsZi5jbGVhbl9idWlsZF9zdHJpbmcg aW4gY2hhbmdlLmNvbW1lbnRzOgorICAgICAgICAgICAgICAgIHNlbGYuc2V0UHJvcGVydHkoJ2lz X2NsZWFuJywgVHJ1ZSkKKyAgICAgICAgICAgICAgICByZXR1cm4gVHJ1ZQogCiAgICAgZGVmIHN0 YXJ0KHNlbGYpOgorICAgICAgICBzZWxmLm5lZWRfY2xlYW5fYnVpbGRfYmFzZWRfb25fY2hhbmdl cygpCiAgICAgICAgIGlmIG5vdCBzZWxmLmdldFByb3BlcnR5KCdpc19jbGVhbicpOgogICAgICAg ICAgICAgc2VsZi5oaWRlU3RlcElmID0gVHJ1ZQogICAgICAgICAgICAgcmV0dXJuIFNLSVBQRUQK