| Message ID | 20201027073000.GA3651896@coredump.intra.peff.net (mailing list archive) |
|---|---|
| Headers | show |
| Series | fix error handling in checkout-index | expand |
Jeff King <peff@peff.net> writes: > While working on another topic, I noticed that "git checkout-index -- > path" does not propagate errors through its exit code. It has been that > way for so long that I almost wondered if it is intentional, but I'm > pretty sure it's not. A bit scary, though. :) Quite honestly, at this point, I do not think the intention matters any more. If somebody depends on the behaviour and wrote do some thing && git checkout-index -- $path_that_is_possibly_missing && do another thing && then this change _will_ be a regression, whether it was originally done this way on purpose or not. I do not think it is the kind of regression that we should avoid, though. I'd say that we should bite the bullet and fix it, as it should also be easy to fix/adjust such a collateral damage. That would make the world a better place in the end. Thanks. > [1/2]: checkout-index: drop error message from empty --stage=all > [2/2]: checkout-index: propagate errors to exit code > > builtin/checkout-index.c | 16 ++++++++++++++-- > t/t2004-checkout-cache-temp.sh | 10 +++++++++- > t/t2006-checkout-index-basic.sh | 11 +++++++++++ > 3 files changed, 34 insertions(+), 3 deletions(-) > > -Peff
On Tue, Oct 27, 2020 at 12:13:03PM -0700, Junio C Hamano wrote: > Jeff King <peff@peff.net> writes: > > > While working on another topic, I noticed that "git checkout-index -- > > path" does not propagate errors through its exit code. It has been that > > way for so long that I almost wondered if it is intentional, but I'm > > pretty sure it's not. A bit scary, though. :) > > Quite honestly, at this point, I do not think the intention matters > any more. If somebody depends on the behaviour and wrote > > do some thing && > git checkout-index -- $path_that_is_possibly_missing && > do another thing && > > then this change _will_ be a regression, whether it was originally > done this way on purpose or not. > > I do not think it is the kind of regression that we should avoid, > though. I'd say that we should bite the bullet and fix it, as it > should also be easy to fix/adjust such a collateral damage. That > would make the world a better place in the end. Right, agreed with all of that. What I meant more with "intentional" was: is there some really clever reason I was missing that it was done this way in the first place? And I think the answer is "no", it was just an oversight. Whether it is OK to change a plumbing command's behavior is somewhat orthogonal, but I agree it a bug fix and OK to do here. -Peff