From: Eric Sunshine Date: Thu, 20 Oct 2022 17:54:46 GMT Subject: Re: [PATCH v2 00/12] fsmonitor: Implement fsmonitor for Linux Message-ID: In-Reply-To: On Thu, Oct 20, 2022 at 1:01 PM Junio C Hamano wrote: > Eric Sunshine writes: > > On Thu, Oct 20, 2022 at 12:13 PM Junio C Hamano wrote: > >> All other jobs were using the default DC_SHA1 (which is a > >> recommended practice), but the default for macOS jobs being Apple's > >> common crypt, we didn't catch recent breakage soon enough. > > > > "recent breakage" is quite vague and probably won't help future > > readers understand what this is actually fixing. Possible > > improvements: (1) a prose description of the breakage; (2) the actual > > compiler error message; (3) a pointer[1] to the email reporting the > > problem. One or more of the above improvements to the commit message > > would help future readers. > > I do not think (2) or (3) would help all that much. A finger that > points at the exact commit that broke the build (with the condition > under which the build breaks) would probably be the most useful to > those who read "git log" output. > > ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ----- > Subject: [PATCH] ci: use DC_SHA1=YesPlease on osx-clang job for CI > > 7b8cfe34 (Merge branch 'ed/fsmonitor-on-networked-macos', > 2022-10-17) broke the build on macOS with sha1dc by bypassing our > hash abstraction (git_SHA_CTX etc.), but it wasn't caught before the > problematic topic was merged down to the 'master' branch. Nobody > was even compile testing with DC_SHA1 set, although it is the > recommended choice in these days for folks when they use SHA-1. > > This was because the default for macOS uses Apple Common Crypto, and > both of the two CI jobs did not override the default. Tweak one of > them to use DC_SHA1 to improve the coverage. Thanks. This revised commit message does a much better job of explaining the problem the patch is addressing.