Denton Liu <liu.denton@xxxxxxxxx> writes: >> > local r1 r2 >> > r1=$(git rev-parse --verify "$1") && >> > r2=$(git rev-parse --verify "$2") && >> >> If either of the calls fail, the assignment itself would fail, and >> the &&-cascade would stop without executing the if statment below. >> >> I see the "!" feature, but where is the promised "fix" for >> segfaulting rev-parse? >> >> Puzzled. > > I suppose your puzzlement comes from my badly worded commit message > above. I meant to say that in the _hypothetical_ case that > `git rev-parse` segfaults, it wouldn't be caught because we're > blanket-ignoring failures if we do `! test_cmp_rev`. > > But I suppose I focused too much on segfaults. I guess I didn't realise > that the problem is more general than that; any failure of > `git rev-parse` should be reported. But if that is the case, shouldn't the part that runs two rev-parse read more like this? r1=$(git rev-parse --verify "$1") || error "'$1' does not name a valid object" r2=$(git rev-parse --verify "$2") || error "'$2' does not name a valid object" if ! test "$r1" $op "$r2" then ... they do not compare the same ... fi Offhand I do not know if the current callers depend on being able to pass a string that is not an object name in either $1 or $2 and a valid object name in the other one, and relying on the helper function to say "$1 and $2 are different!" If such callers exist, a defensive change like the above that requires the caller to always pass valid object names would need to be accompanied with changes to these callers, too. Overall, I think that would give us a better end result, but it might be a bit more work. Thanks.
