Re: [PATCH] fixup! propagate errno from failing read
From: Junio C Hamano <hidden>
Date: 2021-08-18 18:30:46
Han-Wen Nienhuys [off-list ref] writes:
quoted
I think the convention to assign errno to myerr in this codepath originates in a0731250 (refs: explicitly return failure_errno from parse_loose_ref_contents, 2021-07-20), and it forgot the part of the code being fixed with this patch. The commit being fixed is already is in 'next' as part of the hn/refs-errno-cleanup topic. Usually, a flaw in a topic that is already in 'next' is corrected by a follow-up patch, but then they won't say "fixup!" (none of our bugfix patches do). But a post-release is a special time, as we will soon be rewinding 'next', restarting it from the latest release and we have a choice to tentatively eject a topic, fix it up or even replace it, before merging the corrected topic to 'next'. Do you mean that you want me to squash this change into that commit before the topic graduates to 'master' during the new development cycle? If so please be a bit more explicit next time. Using the title of the commit after "fixup!" would be a good starting point.The problem fixed here affects anyone who uses git-repo (ie. does Android development) and runs "git-branch -m", which is a large group of people, so I think it should not be allowed to get into a release.
OK. The problem already is in 'next' and we want to make sure it won't graduate to 'master' for the next release as-is. I agree with that ;-)
So it could be squashed into commit a0731250, or put on top of next as a separate commit (probably with 'fixup!' removed).
I'd try the former first and will fall back on the latter, then.
Note that, even though commit a0731250 originates from a branch called "hn/XXX" and has me as Author, the BUG() call causing the crash was actually introduced by AEvar when he reworked the series.
Yup, I see his Sob after yours and it is quite understandable if a new bug was introduced by his changes. It also would be understandable if his change was only to add a call to BUG() in order to assert that the original patch used myerr consistently, and it uncovered a bug in the original version he took from you. I do not care too much about how exactly the bug was introduced and uncovered---it matters more that the end result has one fewer bug thanks to the team effort.