Thread (4 messages) flat view 4 messages, 2 authors, 2021-05-19

Re: [BUG] clone of large repo fails when server closes idle SSH session during "Resolving deltas"

From: Jeff King <hidden>
Date: 2021-05-19 09:57:08

On Wed, May 19, 2021 at 05:34:40AM -0400, Jeff King wrote:
Another side issue is that once the protocol conversation has finished,
I'm not sure if it's really useful for us to detect and complain about
ssh's exit code. We know the other side completed the conversation
successfully, and we have nothing left to ask it. So a fix for your
immediate pain would be to stop noticing that. I think the root issue is
still worth addressing, though; we are tying up network and local
resources with a useless to-be-closed ssh connection.
By the way, there's an interesting subtlety / bug related to this. While
"git clone" does return a failed exit code in this case, it leaves the
repository directory in place! And because no real error occurred with
the clone, you can use it as usual (though I think if it's a non-bare
clone, you'd need to run "git checkout" fill in the working tree).

Propagating the error code comes from aab179d937 (builtin/clone.c: don't
ignore transport_fetch_refs() errors, 2020-12-03). So prior to Git
v2.30.0, your case would kind-of work.

But:

  - I think that is just "clone"; a "git fetch" would always have
    propagated the error from transport_fetch_refs()

  - that commit was right to start propagating the error code from
    transport_fetch_refs(). While in this specific case, we happened to
    produce a useful repository directory, most other errors would not.

  - there _is_ a bug in aab179d937, though. When it sees the error it
    should clean up the repo directory. And that even happens
    automatically via an atexit() handler. But because rather than
    calling die() it jumps to cleanup code, it mistakenly sets the flag
    for "leave the directory in place".

  - any logic to ignore errors would have to be inside the transport
    code (i.e., it realizes that ssh exiting non-zero isn't a big deal
    anymore, and then transport_fetch_refs() still returns success).

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help