On Tue, Mar 11, 2014 at 4:06 PM, Junio C Hamano <gits...@pobox.com> wrote: > I may be misremembering things, but your first sentence quoted above > was exactly my reaction while reviewing the original change, and I > might have even raised that as an issue myself, saying something > like "consistency across values is more important than type-saving > in a machine format".
For reference, the original design discussion of the format was here: http://thread.gmane.org/gmane.comp.version-control.git/233842 I do not recall this issue being raised before, but now that it has been raised I fully agree: http://thread.gmane.org/gmane.comp.version-control.git/243754/focus=243862 In -z mode an empty <newvalue> should be treated as missing just as it is for <oldvalue>. This is obvious now in hindsight and I wish I had realized this at the time. Back then I went through a lot of iterations on the format and missed this simplification in the final version :( Moving forward: The "create" command rejects a zero <newvalue> so the change in question for that command is merely the wording of the error message and there is no compatibility issue. The "update" command supports a zero <newvalue> so that it can be used for all operations (create, update, delete, verify) with the proper combination of old and new values. The change in question makes an empty <newvalue> an error where it was previously treated as zero. (BTW, Michael, I do not see a test case for the new error in your series. Something like the patch below should work.) > I am not against deprecating and removing > the support for it in the longer term, though. As I reported in my above-linked response, I'm not depending on the old behavior myself. Also if one were to start seeing this error then generated input needs only trivial changes to avoid it. If we do want to preserve compatibility for others then perhaps an empty <newvalue> with -z should produce: warning: update $ref: missing <newvalue>, treating as zero Then after a few releases it can be switched to an error. Thanks, -Brad diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh index 3cc5c66..1e9fe7c 100755 --- a/t/t1400-update-ref.sh +++ b/t/t1400-update-ref.sh @@ -730,6 +730,12 @@ test_expect_success 'stdin -z fails update with bad ref name' ' grep "fatal: invalid ref format: ~a" err ' +test_expect_success 'stdin -z fails update with empty new value' ' + printf $F "update $a" "" >stdin && + test_must_fail git update-ref -z --stdin <stdin 2>err && + grep "fatal: update $a: missing <newvalue>" err +' + test_expect_success 'stdin -z fails update with no new value' ' printf $F "update $a" >stdin && test_must_fail git update-ref -z --stdin <stdin 2>err && -- 1.8.5.2 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
