Re: git push --confirm ?

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

 



On Sat, Sep 12, 2009 at 05:41:23PM -0700, Junio C Hamano wrote:

> > But what _would_ be useful is doing it atomically. You can certainly do
> > all three of those steps from within one "git push" invocation, and I
> > think that is enough without any protocol changes. The protocol already
> > sends for each ref a line like:
> >
> >   <old-sha1> <new-sha1> <ref>
> >
> > and receive-pack will not proceed with the update unless the <old-sha1>
> > matches what is about to be changed.
> 
> Be careful that using that information and doing things in one session
> won't give you atomicity in the sense that it may still fail after you
> said "yes that is what I want to push, really" to the confirmation
> question.

Of course, but that issue exists already. It is just that the window
between receiving the refs and then asking them to be updated is much
smaller when there is no human input in the loop (and since we haven't
actually _shown_ the list to the user, it appears atomic to them).

I think this type of atomicity is fine for this application. The point
of this is to err on the side of caution. So it is OK to say "Push
this?" and then after the user has confirmed say "Oops, somebody pushed
something else while we were waiting for your input. Try again." The
important thing is to not say "Push this?", have the user confirm that
what they are pushing over is OK, and then end up pushing over something
different (which is what can happen with separate push invocations).

The only way to get true atomicity across the confirmation and push
would be to take a lock at the beginning of the push session. Which is
too coarse-grained in the first place (it disallows simultaneous update
of unrelated refs), but would also require protocol updates.

> It does save you an extra connection, compared to separate invocations
> without and then with --dry-run, so it still is a plus.
> 
> I do not think this is an unreasonable option to have.  Just please don't
> justify this change based on atomicity argument, but justify it as a mere
> convenience feature.

I don't agree. Making sure we use the _same_ <old-sha1> in the
confirmation output we show to the user and in the ref update we send to
the remote is critical for this to be safe.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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]