Hi Takashi, On Fri, 4 Apr 2025, Takashi Yano wrote:
> On Fri, 04 Apr 2025 07:27:09 +0200 Johannes Schindelin wrote: > > > Is Jeremy's guess "if raw_write doesn't need to wait (ie, there's room > > in the pipe for the write) it doesn't hit the signal stuff" correct? > > If so, it would be good to add that part to the commit message because > > the commit would otherwise still be incomplete. > > That's not correct. Indeed, raw_write() waits for room in the pipe, > however, it does not matter in this case. The probelm occurs at > cygwait() which waits for pipe mutex as already mentioned in the commit > message. So what is the explanation, then, that this hung only occasionally and not all the time? > > Also: referring to 7ed9adb356df may be technically correct, but human > > readers have a much easier time when that shortened commit OID is > > accompanied by some human-readable information, such as the string > > obtained via `git show --format=reference` (see > > <https://git-scm.com/docs/git-show#_pretty_formats>). > > Do you mean "the commit 7ed9adb356df (Cygwin: pipe: Switch pipe mode to > blocking mode by default, 2024-09-05)"? Not just "the commit > 7ed9adb356df"? Yes. Remember, SHA-1 is considered cryptographically broken (and has actually been so since 2005, Git's original decision to hard-code it everywhere with no upgrade path whatsoever notwithstanding). Git already supports SHA-256 repositories, even if most of the Git repository hosters do not. There may very well be a time when 7ed9adb356df won't refer to the intended commit anymore. In any case, only when you are super familiar with the commits, and only for a short time, can you associate "7ed9adb356df" with "Ah, that was the change that switched to blocking pipe mode by default" as a human. You can easily reduce the cognitive load even for those who associate them by providing the `--format=reference` form. We're still spending a lot of time on getting reviewers such as myself up to speed on the context, we're not even talking about the specifics of the patch yet. Helpful commit messages help with such issues and accelerate reviews, and by making it easier to review they also help ensure that the patches are correct (and won't introduce yet other regressions that will have to be fixed in the future, a pattern I seem to notice in this part of Cygwin's code). Ciao, Johannes
