Re: [PATCH 03/26] t1400: Pass a legitimate <newvalue> to update command

[Brad King <brad.king@kitware.com>]

On Tue, Mar 11, 2014 at 4:06 PM, Junio C Hamano <gitster@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