| Message ID | bc1e396ae0ad380dffd7962cc223cc63e6facffe.1645102965.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Superseded |
| Commit | 62091b4c87a199c172556f15c5662c6c3679e9cd |
| Headers | show |
| Series | fetch: improve atomicity of `--atomic` flag | expand |
Patrick Steinhardt <ps@pks.im> writes: > +static int backfill_tags(struct transport *transport, > ... > } The change to this function is quite straight-forward. > static int do_fetch(struct transport *transport, > @@ -1632,8 +1634,16 @@ static int do_fetch(struct transport *transport, > struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; > > find_non_local_tags(remote_refs, &tags_ref_map, &tail); > - if (tags_ref_map) > - backfill_tags(transport, tags_ref_map, &fetch_head, worktrees); > + if (tags_ref_map) { > + /* > + * If backfilling of tags fails then we want to tell > + * the user so, but we have to continue regardless to > + * populate upstream information of the references we > + * have already fetched above. > + */ OK. Unless atomic, using the available information on successfully updated ones would be fine. Perhaps under --atomic mode, a failure to commit the ref transaction will also prevent the upstream information from getting updated? We'll see soon enough. > + if (backfill_tags(transport, tags_ref_map, &fetch_head, worktrees)) > + retcode = 1; > + } > free_refs(tags_ref_map); > } > diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh > index 6ffe2a5719..c057c49e80 100755 > --- a/t/t5503-tagfollow.sh > +++ b/t/t5503-tagfollow.sh > @@ -233,9 +233,7 @@ test_expect_success 'backfill failure causes command to fail' ' > done > EOF > > - # Even though we fail to create refs/tags/tag1 the below command > - # unexpectedly succeeds. > - git -C clone5 fetch .. $B:refs/heads/something && > + test_must_fail git -C clone5 fetch .. $B:refs/heads/something && OK. > test $B = $(git -C clone5 rev-parse --verify refs/heads/something) && > test $S = $(git -C clone5 rev-parse --verify tag2) && > test_must_fail git -C clone5 rev-parse --verify tag1
diff --git a/builtin/fetch.c b/builtin/fetch.c index f8adb40b45..d304314f16 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -1495,12 +1495,12 @@ static struct transport *prepare_transport(struct remote *remote, int deepen) return transport; } -static void backfill_tags(struct transport *transport, - struct ref *ref_map, - struct fetch_head *fetch_head, - struct worktree **worktrees) +static int backfill_tags(struct transport *transport, + struct ref *ref_map, + struct fetch_head *fetch_head, + struct worktree **worktrees) { - int cannot_reuse; + int retcode, cannot_reuse; /* * Once we have set TRANS_OPT_DEEPEN_SINCE, we can't unset it @@ -1519,12 +1519,14 @@ static void backfill_tags(struct transport *transport, transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL); transport_set_option(transport, TRANS_OPT_DEPTH, "0"); transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL); - fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); + retcode = fetch_and_consume_refs(transport, ref_map, fetch_head, worktrees); if (gsecondary) { transport_disconnect(gsecondary); gsecondary = NULL; } + + return retcode; } static int do_fetch(struct transport *transport, @@ -1632,8 +1634,16 @@ static int do_fetch(struct transport *transport, struct ref *tags_ref_map = NULL, **tail = &tags_ref_map; find_non_local_tags(remote_refs, &tags_ref_map, &tail); - if (tags_ref_map) - backfill_tags(transport, tags_ref_map, &fetch_head, worktrees); + if (tags_ref_map) { + /* + * If backfilling of tags fails then we want to tell + * the user so, but we have to continue regardless to + * populate upstream information of the references we + * have already fetched above. + */ + if (backfill_tags(transport, tags_ref_map, &fetch_head, worktrees)) + retcode = 1; + } free_refs(tags_ref_map); } diff --git a/t/t5503-tagfollow.sh b/t/t5503-tagfollow.sh index 6ffe2a5719..c057c49e80 100755 --- a/t/t5503-tagfollow.sh +++ b/t/t5503-tagfollow.sh @@ -233,9 +233,7 @@ test_expect_success 'backfill failure causes command to fail' ' done EOF - # Even though we fail to create refs/tags/tag1 the below command - # unexpectedly succeeds. - git -C clone5 fetch .. $B:refs/heads/something && + test_must_fail git -C clone5 fetch .. $B:refs/heads/something && test $B = $(git -C clone5 rev-parse --verify refs/heads/something) && test $S = $(git -C clone5 rev-parse --verify tag2) && test_must_fail git -C clone5 rev-parse --verify tag1
When the backfilling of tags fails we do not report this error to the caller, but only report it implicitly at a later point when reporting updated references. This leaves callers unable to act upon the information of whether the backfilling succeeded or not. Refactor the function to return an error code and pass it up the callstack. This causes us to correctly propagate the error back to the user of git-fetch(1). Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/fetch.c | 26 ++++++++++++++++++-------- t/t5503-tagfollow.sh | 4 +--- 2 files changed, 19 insertions(+), 11 deletions(-)