Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:24

Ramkumar Ramachandra [off-list ref] writes:
Junio C Hamano wrote:
quoted
The patch may have been done by a wrong motivation, in that it does
not fundamentally "fix" the itch.  The particular "itch" is not
something we are going to promise to the end users, ever, anyway.
Just out of curiosity, is it possible to write a correct fix at all?
Is there anything to "fix" in the first place, you have to wonder.

Your "git push there HEAD:master" would roughly do the following:

    (1) read HEAD to learn what commit you are pushing;

    (2) contact the other side and find where there tips are;

    (3) send a packfile that should be enough for the other side to
        have complete history leading to the commit you read in (1);

    (4) tell the other side to update its 'master' to the commit you
        read in (1).

If you drop step (1) and replace "the commit you read in (1)" in
steps (3) and (4) with "the commit you see in HEAD at this point by
re-reading HEAD", then such a "git push" that races with something
else you do in your other terminal may break---you can cause it to
see different commits at steps (3) and (4), potentially getting the
other side out of sync (but the receiving end does an independent
connectivity check so your push will likely to be rejected).

And the fix to such a breakage is to structure the code like the
above four steps to make it race-free.

If I understand your example correctly, you are talking about a
quite different thing.  "git push there HEAD:master" racing with
your other terminal that changes the HEAD sees different HEAD
depending on the order:

    (a) if the other terminal changes the HEAD first, step (1) will
        see that updated HEAD; or

    (b) if the step (1) reads HEAD before you change it in the other
        terminal, it will see the original HEAD.

But that is very much to be expected, isn't it?  It sounds similar
to

    I have "largedir" I want to get rid of, but there is a directory
    I want to save, "largedir/precious", in it, so I do

        cp -R largedir/precious precious

    and then run 'rm -rf largedir' in another terminal in parallel.

Is there anything to fix?

Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:57:25

Junio C Hamano wrote:
    I have "largedir" I want to get rid of, but there is a directory
    I want to save, "largedir/precious", in it, so I do

        cp -R largedir/precious precious

    and then run 'rm -rf largedir' in another terminal in parallel.
I would argue that there is something to "fix", but that fix involves
making the cp a purely atomic operation which is super-complicated,
and totally not worth it.  Would you _not_ like the above example to
work?  Then how can you say that there's nothing to be fixed?

Consider a slightly different example: I rename a file while having an
active file handle open in a process that's reading the file.  Will
the rename fail or will the fread() in the process fail?  Nope, both
work fine.  Replace "rename" with "remove", and we still have the same
answer.  Ofcourse there are no guarantees: I can start up another
process to overwrite the sectors corresponding to that file's data
with zeros; unless the complete file is there in the kernel buffer, a
read() will eventually end up slurping in the zeros (or fail?), right?
 It's just that it works in practice.

Yet another example: I have a terminal pointing to a directory, and I
remove that directory in another terminal.  When I switch back to the
original terminal, I can't cd .., because getcwd() fails.  This has
annoyed me endlessly in practice, and I would really like to fix this
if I can.

Don't accept the way things are, and assume that there's nothing to be
"fixed".  In my opinion, if something about a piece of software annoys
you, there is always something to fix.  It just depends on what _can_
be fixed in a reasonable amount of time with a good engineering
solution.  There's no need to go to the other extreme: I'm not
interested in rewriting the whole operating system in Haskell and
providing theoretical guarantees for everything.

Coming back to our push example, I don't see why you think HEAD is
special: I could even say git push master and expect it to race with
an update-ref.  But nobody is complaining about that: if someone does
complain, I would seriously consider copying master to PUSH_HEAD early
(and push that).  With HEAD, however, someone is complaining (namely,
me): pushing usually means that I've finished working on that branch,
and want to switch to another branch and continue working.  Why should
I have to wait for the push to complete?  I've hit this bug several
times (from terminal as well as Magit), and this patch fixes the
problem for me in practice.

That said, I agree that my patch does not guarantee anything (and I
will modify my commit message to clarify this).  I'm just expressing
my opinion on the issue of "fixing problems".

Re: [PATCH 0/3] Fixing volatile HEAD in push.default = current

From: Andreas Krey <hidden>
Date: 2016-06-15 22:57:25

On Thu, 23 May 2013 13:25:55 +0000, Ramkumar Ramachandra wrote:
Junio C Hamano wrote:
quoted
    I have "largedir" I want to get rid of, but there is a directory
    I want to save, "largedir/precious", in it, so I do

        cp -R largedir/precious precious

    and then run 'rm -rf largedir' in another terminal in parallel.
'mv largedir/precious precious; rm -rf largedir'? No race here.

...
Consider a slightly different example: I rename a file while having an
active file handle open in a process that's reading the file.  Will
the rename fail or will the fread() in the process fail?  Nope, both
work fine.  Replace "rename" with "remove", and we still have the same
answer.  Ofcourse there are no guarantees: I can start up another
process to overwrite the sectors corresponding to that file's data
with zeros; unless the complete file is there in the kernel buffer, a
read() will eventually end up slurping in the zeros (or fail?), right?
Oh, there are guarantees, they just don't include the case where you
take a shotgun to the disk. (Or do it on an nfs mount and delete the
file from another machine.)

...

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help