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

[Michael Haggerty <mhagger@alum.mit.edu>]

On 03/11/2014 10:41 PM, Brad King wrote:
> 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 :(

It's not your fault; anybody could have reviewed your code at the time
(I most of all, because I have been so active in this area of the code).

> 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

This last suggestion is what I am implementing for the re-roll (coming
shortly).  Thanks for the discussion.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/