221991 2021-02-16 13:20:34 -0800 [RFC] Remove many webkit-patch commands 2023-12-06 17:07:27 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=226230 InRadar P2 Normal --- 265939 222058 222106 222617 222745 227980 231032 1 gsnedders gsnedders aakash_jain ap ben_schwartz bfulgham clopez dino dpino fujii jbedard jean-yves.avenard jer.noble koivisto pangle peng.liu6 pnormand rniwa ross.kirsling sam simon.fraser webkit-bug-importer wenson_hsieh zalan oldest_to_newest 1729893 0 gsnedders 2021-02-16 13:20:34 -0800 We have a lot of commands under webkit-patch, many of which exist primarily for the sake of EWS, though EWS doesn't use them any more. I'd like to avoid having to update many of them when refactoring code elsewhere in webkitpy. I'm largely filing this to have somewhere to ask people to post what commands they do use, especially from forthcoming deprecation notices that I intend on adding. Specifically, I'd like to get rid of all the build commands (build, build-and-test, build-and-test-attachment, build-attachment) as well as the --build and --test arguments for all the land commands (land, land-attachment, land-cowboy, land-cowhand, land-from-bug, land-from-url, land-safely). Jonathan had previously said his first starting point was going to be all commands not shown in the default help. These are: add-users-to-groups analyze-changelog apply-watchlist apply-watchlist-local assign-to-committer attach-to-bug bug-for-test bug-search bugs-to-commit build build-and-test build-and-test-attachment build-attachment check-patch-relevance check-style check-style-local clean clean-pending-commit clean-review-queue commit-message crash-log create-bug create-revert create-rollout failure-reason find-flaky-tests find-resolved-bugs find-users garden-o-matic gtk-wk2-ews has-landed help ios-ews ios-sim-ews jsc-armv7-ews jsc-ews jsc-i386-ews jsc-mips-ews land-cowboy land-cowhand land-from-url mac-debug-ews mac-ews mac-wk2-ews mark-bug-fixed obsolete-attachments open-bugs patches-in-commit-queue patches-to-commit-queue patches-to-review post post-commits prepare prepare-revert prepare-rollout print-baselines print-expectations rebaseline rebaseline-expectations rebaseline-json rebaseline-server rebaseline-test-internal results-for rollout setup-git-clone suggest-nominations suggest-reviewers tree-status update validate-changelog what-broke win-ews wincairo-ews wpe-ews wpt-change-export 1730429 1 gsnedders 2021-02-17 11:54:56 -0800 CCing basically everyone who's touched this code in recent years and who presumably cares about it. I'm pretty sure the above list is _too_ complete, and some of those need kept (e.g., at least some of the rebaselining commands?), but I'd be more than happy to be told which of those do actually get used! 1730433 2 aakash_jain 2021-02-17 11:58:51 -0800 current EWS uses two of these commands: apply-watchlist-local is used by current EWS in https://github.com/WebKit/WebKit/blob/main/Tools/CISupport/ews-build/steps.py#L2943 validate-changelog is used by current EWS in https://github.com/WebKit/WebKit/blob/main/Tools/CISupport/ews-build/steps.py#L900 1730456 3 simon.fraser 2021-02-17 12:42:27 -0800 I see 'print-expectations' here, but that's a run-webkit-tests command, not a webkit-patch command. I also use it frequently. 1730462 4 fujii 2021-02-17 12:51:30 -0800 I'm using prepare-revert and check-style. WebKitBot is using create-revert. https://github.com/WebKit/WebKit/blob/3d2a79d4c0562fc0f2ea2c3198d6280ec55abc0f/Tools/WebKitBot/src/WebKitBot.mjs#L393 1730471 5 jbedard 2021-02-17 13:07:22 -0800 (In reply to Simon Fraser (smfr) from comment #3) > I see 'print-expectations' here, but that's a run-webkit-tests command, not > a webkit-patch command. I also use it frequently. Inexplicably, `print-expectations` is also a webkit-patch command.... webkit-patch --all-commands | grep print-expectations print-expectations Print the expected result for the given test(s) on the given port(s) 1730478 6 simon.fraser 2021-02-17 13:13:22 -0800 OpenSource % ./Tools/Scripts/webkit-patch --print-expectations Usage: webkit-patch [options] COMMAND [ARGS] webkit-patch: error: no such option: --print-expectations Type "webkit-patch --help" to see usage. 1730480 7 simon.fraser 2021-02-17 13:14:07 -0800 Oh, it's a command, not an argument. Fine. 1730665 8 bfulgham 2021-02-17 17:25:56 -0800 I 100% object to removing "webkit-patch clean". I use it many times a day, every day. 1730672 9 aakash_jain 2021-02-17 17:42:56 -0800 It's fine to delete all these (ews related): gtk-wk2-ews ios-ews ios-sim-ews jsc-armv7-ews jsc-ews jsc-i386-ews jsc-mips-ews mac-debug-ews mac-ews mac-wk2-ews win-ews wincairo-ews wpe-ews 1731105 10 clopez 2021-02-18 17:09:15 -0800 I find the following commands useful: > check-style > check-style-local I use this two ones to check the style before uploading the patch > help I guess we want to keep help > rollout I have used this to rollout patches. > setup-git-clone I have used this to setup the git clone from svn > suggest-nominations > suggest-reviewers This two are useful. Specially the suggest-nominations one to check new comitter/reviewers that are candidate to be nominated 1731156 11 pangle 2021-02-18 18:57:18 -0800 I've been using `webkit-patch prepare --update-changelogs 222006` regularly. I remember starting with `prepare-ChangeLog -b 222006` before moving to `...prepare...` and can't remember why I switched. I've also been eyeing `has-landed` to help me manage the proliferation of git branches on my machine, but it's not part of my workflow _yet_. 1731183 12 sam 2021-02-18 20:03:36 -0800 I use `webkit-patch post` for every patch. 1731370 13 jbedard 2021-02-19 09:14:26 -0800 (In reply to Carlos Alberto Lopez Perez from comment #10) > I find the following commands useful: > > > check-style > > check-style-local What are the reasons to use those commands over check-webkit-style? 1731373 14 jbedard 2021-02-19 09:17:47 -0800 (In reply to Sam Weinig from comment #12) > I use `webkit-patch post` for every patch. Would seem that `webkit-patch post` is just an alias for `webkit-patch upload` then. 1731811 15 peng.liu6 2021-02-20 10:58:36 -0800 I frequently use "webkit-patch prepare" to file bugs and update(refresh) change logs. 1732042 16 dpino 2021-02-21 22:35:27 -0800 Some webkit-patch commands I regularly use are: - land-cowhand for landing patches that do not require review such as build fixes of test gardening patches. I'm fine with removing this command if there's an alternative for doing the same thing. - mark-bug-fixed for automatically closing bugs that have been fixed (for instance, I used it often for closing bugs reporting LayoutTests failures which are not in TestExpectations anymore). 1732094 17 pnormand 2021-02-22 04:29:04 -0800 I still use mark-bug-fixed. 1732147 18 jbedard 2021-02-22 08:58:06 -0800 (In reply to Diego Pino from comment #16) > Some webkit-patch commands I regularly use are: > > - land-cowhand > > for landing patches that do not require review such as build fixes of test > gardening patches. I'm fine with removing this command if there's an > alternative for doing the same thing. > > - mark-bug-fixed > > for automatically closing bugs that have been fixed (for instance, I used it > often for closing bugs reporting LayoutTests failures which are not in > TestExpectations anymore). Any reason you're using `land-cowhand` over `land`? My understanding was that `land-cowhand` skipped the pre-commit tests, but `land` no longer has pre-commit tests in it. 1732319 19 dino 2021-02-22 15:08:30 -0800 +1 to mark-bug-fixed +1 to prepare 1732350 20 rniwa 2021-02-22 15:55:21 -0800 I do use setup-git-clone all the time since I can never remember all the random git configuration commands I need to run to setup a WebKit checkout. 1732364 21 jbedard 2021-02-22 16:10:24 -0800 Some folks reached out and `webkit-patch setup-git-clone` is not entirely duplicated by `git-webkit setup-git-svn`, so we need to keep that around for a bit while we move the parts of it that we want to keep to `git-webkit` 1732431 22 dpino 2021-02-22 19:37:56 -0800 (In reply to Jonathan Bedard from comment #18) > Any reason you're using `land-cowhand` over `land`? My understanding was > that `land-cowhand` skipped the pre-commit tests, but `land` no longer has > pre-commit tests in it. No other reason than tradition. When I asked other more experienced WebKit developers how I could land a patch without filing a bug I was taught `land-cowhand` and I've been keep using it since then for that kind of task. If it's actually better to use `land` I will stop using `land-cowhand`. 1732432 23 rniwa 2021-02-22 19:45:28 -0800 I also use `webkit-patch clean` a lot. 1732670 24 webkit-bug-importer 2021-02-23 13:21:13 -0800 <rdar://problem/74657982> 1732720 25 wenson_hsieh 2021-02-23 16:11:37 -0800 (In reply to Diego Pino from comment #22) > (In reply to Jonathan Bedard from comment #18) > > > Any reason you're using `land-cowhand` over `land`? My understanding was > > that `land-cowhand` skipped the pre-commit tests, but `land` no longer has > > pre-commit tests in it. > > No other reason than tradition. When I asked other more experienced WebKit > developers how I could land a patch without filing a bug I was taught > `land-cowhand` and I've been keep using it since then for that kind of task. > > If it's actually better to use `land` I will stop using `land-cowhand`. FWIW, I generally prefer `land-cowhand` over `land`, since it automatically generates and presents a pretty-printed version of the patch that I can check for sanity. 1733202 26 jer.noble 2021-02-24 21:23:32 -0800 +1 for `webkit-patch prepare` 1735629 27 clopez 2021-03-03 17:51:08 -0800 (In reply to Jonathan Bedard from comment #13) > (In reply to Carlos Alberto Lopez Perez from comment #10) > > I find the following commands useful: > > > > > check-style > > > check-style-local > > What are the reasons to use those commands over check-webkit-style? Mmm.. I didn't knew about the check-webkit-style script That works for me as well. 1735883 28 gsnedders 2021-03-04 09:52:02 -0800 (In reply to Carlos Alberto Lopez Perez from comment #10) > > rollout > > I have used this to rollout patches. Note that this has been marked as deprecated since bug 208775; this is not a new deprecation. --- Bug 222745 should undeprecate the rest of everything mentioned above; still interested in anything else that people run into (especially more infrequently!). 1742288 29 ross.kirsling 2021-03-22 10:44:22 -0700 I make use of `land-cowhand` every time I fix a broken build. 1777144 30 gsnedders 2021-07-15 00:17:18 -0700 FYI: I've filed Bug 227980 for some of the land-cowhand v. land discussion above. Aside from land-cowhand pending that other bug, my intention is to remove everything currently marked as deprecated at the end of this month. 1777146 31 rniwa 2021-07-15 00:26:21 -0700 (In reply to Sam Sneddon [:gsnedders] from comment #30) > FYI: I've filed Bug 227980 for some of the land-cowhand v. land discussion > above. > > Aside from land-cowhand pending that other bug, my intention is to remove > everything currently marked as deprecated at the end of this month. What are things currently marked as deprecated? 1777178 32 gsnedders 2021-07-15 05:34:02 -0700 (In reply to Ryosuke Niwa from comment #31) > (In reply to Sam Sneddon [:gsnedders] from comment #30) > > FYI: I've filed Bug 227980 for some of the land-cowhand v. land discussion > > above. > > > > Aside from land-cowhand pending that other bug, my intention is to remove > > everything currently marked as deprecated at the end of this month. > > What are things currently marked as deprecated? They are: add-users-to-groups analyze-changelog apply-watchlist assign-to-committer attach-to-bug bug-for-test bug-search bugs-to-commit build build-attachment check-patch-relevance clean-pending-commit clean-review-queue commit-message crash-log create-bug create-rollout failure-reason find-flaky-tests find-resolved-bugs find-users garden-o-matic has-landed land-cowboy land-cowhand land-from-url obsolete-attachments open-bugs patches-in-commit-queue patches-to-commit-queue patches-to-review post-commits prepare-rollout print-baselines print-expectations rebaseline rebaseline-expectations rebaseline-json rebaseline-server rebaseline-test-internal results-for rollout tree-status update what-broke wpt-change-export 1790858 33 zalan 2021-09-06 20:38:51 -0700 I use rebaseline-server occasionally. 1978948 34 koivisto 2023-09-21 04:37:16 -0700 I just used rebaseline-server 1981899 35 ben_schwartz 2023-10-02 16:16:45 -0700 Obligatory "just used rebaseline-server" 1997258 36 gsnedders 2023-12-05 15:59:07 -0800 Pull request: https://github.com/WebKit/WebKit/pull/21363 1997324 37 ews-feeder 2023-12-05 22:34:26 -0800 Committed 271597@main (7eae473bde06): <https://commits.webkit.org/271597@main> Reviewed commits have been landed. Closing PR #21363 and removing active labels. 1997378 38 jbedard 2023-12-06 07:10:40 -0800 Needed to be reverted because it broke all webkit-patch commands. 1997439 39 gsnedders 2023-12-06 11:05:34 -0800 (In reply to Jonathan Bedard from comment #38) > Needed to be reverted because it broke all webkit-patch commands. Reverted in bug 265939. 1997453 40 gsnedders 2023-12-06 11:43:32 -0800 Pull request: https://github.com/WebKit/WebKit/pull/21406 1997560 41 ews-feeder 2023-12-06 17:07:23 -0800 Committed 271648@main (ba8968d7e2b3): <https://commits.webkit.org/271648@main> Reviewed commits have been landed. Closing PR #21406 and removing active labels.