Re: [PATCH 0/2] Fix a refname trimming problem in `log --bisect`
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 14, 2017, 10:09 UTC
- Message-ID
- <xmqqbmpq519f.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <cover.1497430232.git.mhagger@alum.mit.edu>
Michael Haggerty <mhagger@alum.mit.edu> writes:
Show 27 quoted lines
> The code for `git log --bisect` was calling > `for_each_ref_in_submodule()` with prefix set to "refs/bisect/bad", > which is the actual name of the reference that it wants. This resulted > in the refname being trimmed completely away and the empty string > being passed to the callback. That became impermissible after > > b9c8e7f2fb prefix_ref_iterator: don't trim too much, 2017-05-22 > > , so the command was failing. > > Fix the problem in two orthogonal ways: > > 1. Add a new function, `for_each_fullref_in_submodule()`, that doesn't > trim the refnames that it passes to callbacks, and us that instead. > I *think* that this is a strict improvement, though I don't know > the `git log` code well enough to be sure that it won't have bad > side-effects. > > 2. Relax the "trimming too many characters" check to allow the full > length of the refname to be trimmed away (though not more than > that). > > In an ideal world the second patch shouldn't be necessary, because > this calling pattern is questionable and it might be better that we > learn about any other offenders. But if we'd rather be conservative > and not break any other code that might rely on the old behavior, > patch 2 is my suggestion for how to do it.
Thanks for a nice summary.
I agree that 2. is a nice safety to have, especially if the code before b9c8e7f2 ("prefix_ref_iterator: don't trim too much", 2017-05-22) has been seeing the completely trimmed result (i.e. an empty string) in the callback function.
And I agree that 1. is also a good interface to have.