Re: [PATCH 7/8] refs: add 'update-symref' command to 'update-ref'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> diff --git a/Documentation/git-update-ref.txt b/Documentation/git-update-ref.txt
>> index 0561808cca..2ea8bc8167 100644
>> --- a/Documentation/git-update-ref.txt
>> +++ b/Documentation/git-update-ref.txt
>> @@ -65,6 +65,7 @@ performs all modifications together.  Specify commands of the form:
>
> In pre-context, there is this entry.
>
> 	update SP <ref> SP <newvalue> [SP <oldvalue>] LF
>
> Unrelated to this patch, we probably should say <new-value> and
> <old-value> to follow the (recent) documentation guideline to write
> multiple words concatenated with hyphen.
>
> If we are updating these newvalue and oldvalue anyway, we may want
> to update them to <new-oid> and <old-oid>, as the existing commands
> on refs are about object names, and what we are adding here are not.
>
> I would prefer to see such an update of existing documentation as a
> separate preliminary clean-up patch, so that we can cleanly add the
> "update-symref" that takes <new-symref-target> on top of a
> cleaned-up base.
>
> The <newvalue> ...
>

I agree with your synopsis here, I'll send in a patch for this,
independent of these changes as a precursor, while we discuss the final
design of this series.

>> Chris Torek <chris.torek@xxxxxxxxx> writes:
>>
>>> For these reasons, I'd suggest that the all-zero hash be officially
>>> deprecated in favor of create/delete and of course create-symref
>>> and delete-symref. Of course, compatibility requires some sort
>>> of support for the old system for some time.  As to whether that
>>> means something like the suggestion of ".missing" etc, I have no
>>> particular opinion -- but since the symref options are new, they
>>> would not *need* symmetric options, if we just say that "update-symref"
>>> cannot create or delete a symref.
>>
>> I love that simplicity.
>
> Having said that, the loose "update that can create or delete" may
> actually be used by applications that do not care about overwriting
> competing operation, so I am not sure if we can easily deprecate
> that mode of operation.  Saying "update refs/heads/next to point at
> this object" and have it created if it does not exist may be handy
> for some loosely written applications.
>
> So perhaps we can say "update with a concrete <old-oid> will ensure
> that the <ref> poitns at <old-oid> before proceeding, but update
> with 0{40} as <old-oid> to ensure creation is deprecated.  update
> with 0{40} as <new-oid> as deletion is also deprecated.  Use create
> and delete for these two deprecated operation modes".
>
> This assumes that create and delete currently ensures that what is
> asked to be created does not exist, and what is asked to be deleted
> does exist, before the operation.  If we are loosely doing these two
> operations, then we cannot easily deprecate the checking-update,
> without breaking existing users.
>
> As {create,update,delete,verify}-symref do not exist yet, we can
> start with the right semantics from day one.  "update-symref" will
> accept a <old-ref> only to ensure that the symref is pointing to
> that ref and there is no "zero" value based creation/deletion
> validation offered via "update-symref".  "create-symref" will error
> out if the ref asked to be created already exists, "delete-symref"
> will error out if the ref asked to be deleted does not exist.

This seems very reasonable to me. This ensures that each of the commands
do not spill over to the other and most importantly we don't have to
deal with "zero" values.

But this still means we need to think of the best output for the
reference transaction hook (following commit).

My current implementation of:
   <symref-target> SP <ref-name> LF
Should be changed to:
   <old-ref> SP <new-ref> LF

But this means, for creation of symrefs <old-ref> needs to be "zero"
value. Also there is no way for clients to differentiate between regular
refs and symrefs here. I wonder if it makes sense to do something like:

   symref SP <old-ref> SP <new-ref> LF

Where symref is a fixed string at the start, used as a differentiator.
This however still would leave us to deal with "zero" values for
creation and deletion.

Perhaps the best way here to actually be a lot more verbose and have the
hook output the following:

   symref-create SP <new-ref> LF
   symref-delete SP <old-ref> LF
   symref-update SP <old-ref> SP <new-ref> LF
   symref-update-forced <new-ref> LF

I lean towards the latter, because its also much easier to extend in the
future and we don't have to deal with the "zero" values.

I'm against the "zero" values mostly cause there is no nice way to
describe a zero value for a ref unlike OIDs, which is inherently baked
into the type of data.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux