282792 2024-11-07 17:02:25 -0800 Non-git-trailers in "Canonical link" block 2025-02-04 15:01:00 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=266028 InRadar P2 Normal --- 1 gsnedders gsnedders bfan2 emw jbedard webkit-bug-importer oldest_to_newest 2073653 0 gsnedders 2024-11-07 17:02:25 -0800 For Bug 266028, we really want to move towards using actual git trailers for our "Canonical link". It would be nice if, at the moment, the only non-trailer line in the trailer block was our "Canonical link" line. Per the git-interpret-trailers man page: > Existing trailers are extracted from the input by looking for a group of one or more lines that (i) is all trailers, or (ii) contains at least one Git-generated or user-configured trailer and consists of at least 25% trailers. The group must be preceded by one or more empty (or whitespace-only) lines. The group must either be at the end of the input or be the last non-whitespace lines before a line that starts with --- (followed by a space or the end of the line). We do however have a number of commits where we have other non-trailer lines in the trailer block: 286098@main, 286074@main, 285122@main, 285092@main, 284409@main, 284248@main, 283054@main, 282969@main, 282950@main, 282415@main, 282327@main, 282204@main, 281380@main, 281218@main, 280897@main, 280750@main, 280635@main, 278661@main, 277268@main, 277054@main, 276989@main, 276928@main, 276721@main, 275313@main, 274905@main, 274777@main, 273007@main, 269958@main, 269186@main, 268926@main, 268819@main, 267564@main, 267441@main, 266913@main, 266711@main, 253704@main Specifically, we see the following lines in our pseudo-trailer block: * .gitignore: Add BenchmarkTemp to .gitignore * .submitproject-tools: Added. * .submitproject: Skip more ThirdParty to match installsrc a little better. * .submitproject: Added. * .submitproject: Exclude Source/ThirdParty/skia * Websites/webkit.org/wp-content/themes/webkit/images/twitter-card.png: Added. * Websites/webkit.org/wp-content/plugins/social-meta.php: * filenames: Removed. * hello.txt: Added. * modified: LayoutTests/accessibility/aria-current-expected.txt * modified: LayoutTests/accessibility/aria-current.html * tmp: Added. 014e584f75 Vulkan: Separate out XFB data from ShaderInterfaceVariableInfo 10217bb0 libsysprof-capture: fix set_fd_blocking() c228634f libsysprof-capture: handle unwind length < 0 gracefully 11724133cd Metal: Separate vars with normal types 1e269594df ssci: canonicalize / backfill dependencies managed by DEPS 2d4a027dc7 Tests: Add Poppy Playtime Trace 8482f44fab android_helper cleanup: Inline test package name 318e5e0260 Vulkan: Update EGL_EXT_buffer_age implementation 319907d2cd Vulkan: Propagate the support of shaderInt16/64 feature 62cc379056 Metal: Remove UseResource old SDK fallbacks e90cd0c412 Vulkan: Add check for int8 extension support 78dd9a8eec Metal: Support compiling without OpenGL on macOS 2f28264ea2 Trace perf: add origin/main revision to summary if != HEAD 579a58552f Vulkan: Add query for 64bit fp support feature check 319907d2cd Vulkan: Propagate the support of shaderInt16/64 feature 62cc379056 Metal: Remove UseResource old SDK fallbacks e90cd0c412 Vulkan: Add check for int8 extension support 78dd9a8eec Metal: Support compiling without OpenGL on macOS 2f28264ea2 Trace perf: add origin/main revision to summary if != HEAD 579a58552f Vulkan: Add query for 64bit fp support feature check Drive-by fix to spelling: deafult -> default Drive-by fix: Unified sources. Drive-by fix: always call the completion handler on the main thread. Drive-by fix: remove an unnecessary header import for `FixedVector.h`. Fix `error: equality comparison operator can only be defaulted in a class definition`. Fix typo: `uint` -> `uint8_t` Import of revision: 7fe9e78691410d5e2eb9bd78e79f14988e5b5643 Mark the button as `will-change: opacity` to give it its own layer. Store active document in CapturingData: https://w3c.github.io/pointerevents/#dfn-active-document Test expectation update: removed occlusion layers. Upstream commit: https://github.com/web-platform-tests/wpt/commit/4e3b5de2eb8218cf18a1674618994efeb96e2cc0 Upstream commit: https://github.com/web-platform-tests/wpt/commit/b2528beea2a54778f7314768d2352ccee8b7993f [1]: https://w3c.github.io/mathml-core/spec.html#prescripts-and-tensor-indices-mmultiscripts#:~:text=that%20is%20not%20an%20mprescripts%20element [2]: https://github.com/WebKit/WebKit/commit/b70287103be9d7a826cb2697e7a951536dc2a04b [1]: https://github.com/WebKit/WebKit/commit/d8b1589e4915042608bb1a1a55bd291bcec76d51#diff-955991178e0e7971ad17ceface65532b503e98975aec31d0fc7093e0d80c3dc5R167-R169 `appearance: -apple-pay-button`. `vertical-rl`: Left and right arrow keys can be used for next and previous respectively. `vertical-lr`: Right and left arrow keys can be used for next and previous respectively. b380ed1f98 Vulkan: Add EGL_ANGLE_global_fence_sync ef78e57015 Revert "Vulkan: disable warmUpPipelineCacheAtLink for Venus" ef78e57015 Revert "Vulkan: disable warmUpPipelineCacheAtLink for Venus" of the line and `line-break: after-white-space` is used. We should make our canonicalization stricter to match the git-interpret-trailers parser _aside_ from "Canonical link", because these are all clearly _not_ trailers. 2073884 1 emw 2024-11-08 10:41:08 -0800 Sounds like a potential (easy) solution is for canonicalization to always ensure a blank line at the end of the committer's message. But this would be complicated by the things that may appear in commit messages which really _are_ trailers, such as Co-authored-by and Originally-landed-as. Maybe canonicalization needs to look for a trailer group in the commit message, and if one does NOT exist, ensure an empty line before adding its own. 2074311 2 gsnedders 2024-11-11 10:55:46 -0800 (In reply to Elliott Williams from comment #1) > Maybe canonicalization needs to look for a trailer group in the commit > message, and if one does NOT exist, ensure an empty line before adding its > own. It tries to do this; it's just overly lax about parsing trailers, and thus picks up basically any line with a colon in it as a trailer. I'm gonna post a PR which progresses this, which just alters our regex we use to find trailers. This doesn't solve many of the more fundamental bugs (we don't support folded trailers, we don't deal with comments in the commit message), but it does stop us from doing surprising reformatting of commit messages that end in many seemingly reasonable lines. e.g., currently: ``` Subject not a trailer: line just: a line ``` Turns into: ``` Subject not a trailer: line just: a line Canonical link: xxx ``` 2074384 3 gsnedders 2024-11-11 15:59:04 -0800 Pull request: https://github.com/WebKit/WebKit/pull/36498 2075247 4 webkit-bug-importer 2024-11-14 17:03:14 -0800 <rdar://problem/139923051> 2091963 5 ews-feeder 2025-02-04 15:00:58 -0800 Committed 289828@main (09a80b732c63): <https://commits.webkit.org/289828@main> Reviewed commits have been landed. Closing PR #36498 and removing active labels.