| Message ID | 20241014225431.1394565-1-bence@ferdinandy.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | set-head/fetch remote/HEAD updates, small change from v7 | expand |
On Tue, Oct 15, 2024 at 12:53:09AM +0200, Bence Ferdinandy wrote: > Bence Ferdinandy (6): > refs: atomically record overwritten ref in update_symref > remote set-head: add new variable for readability > remote set-head: better output for --auto > refs: add TRANSACTION_CREATE_EXISTS error > refs: add create_only option to refs_update_symref > fetch: set remote/HEAD if it does not exist I integrated this new round into my copy of 'seen' today and noticed some test breakage in t5505 here: https://github.com/ttaylorr/git/actions/runs/11356267070 I'm going to temporarily eject this topic out of 'seen' until we can get an analysis of what's going on here. Thanks, Taylor
On Wed Oct 16, 2024 at 02:26, Taylor Blau <me@ttaylorr.com> wrote: > On Tue, Oct 15, 2024 at 12:53:09AM +0200, Bence Ferdinandy wrote: >> Bence Ferdinandy (6): >> refs: atomically record overwritten ref in update_symref >> remote set-head: add new variable for readability >> remote set-head: better output for --auto >> refs: add TRANSACTION_CREATE_EXISTS error >> refs: add create_only option to refs_update_symref >> fetch: set remote/HEAD if it does not exist > > I integrated this new round into my copy of 'seen' today and noticed > some test breakage in t5505 here: > > https://github.com/ttaylorr/git/actions/runs/11356267070 > > I'm going to temporarily eject this topic out of 'seen' until we can get > an analysis of what's going on here. Thanks for the heads up! I see that indeed the failing output is different than what I have locally (e.g. no "apis/HEAD -> apis/main" in any of the test files I have). On the other hand I can't reproduce it so I will need some help with this I think. I tested with both a rebase on master and a rebase on your seen branch. I'm using these to commands in t/ to run tests: prove --timer --jobs 15 ./t[0-9]*.sh GIT_TEST_DEFAULT_REF_FORMAT=reftable prove --timer --jobs 15 ./t[0-9]*.sh Everything passes. Should I be running this differently? I'm assuming the test do not use any global or local git config I may have? I'm testing on Ubuntu 22.04 natively and in WSL, but the nature of the error doesn't seem like it could or should be related to the OS. Thanks, Bence
On Wed, Oct 16, 2024 at 10:18:44AM +0200, Bence Ferdinandy wrote: > > On Wed Oct 16, 2024 at 02:26, Taylor Blau <me@ttaylorr.com> wrote: > > On Tue, Oct 15, 2024 at 12:53:09AM +0200, Bence Ferdinandy wrote: > >> Bence Ferdinandy (6): > >> refs: atomically record overwritten ref in update_symref > >> remote set-head: add new variable for readability > >> remote set-head: better output for --auto > >> refs: add TRANSACTION_CREATE_EXISTS error > >> refs: add create_only option to refs_update_symref > >> fetch: set remote/HEAD if it does not exist > > > > I integrated this new round into my copy of 'seen' today and noticed > > some test breakage in t5505 here: > > > > https://github.com/ttaylorr/git/actions/runs/11356267070 > > > > I'm going to temporarily eject this topic out of 'seen' until we can get > > an analysis of what's going on here. > > Thanks for the heads up! I see that indeed the failing output is different than > what I have locally (e.g. no "apis/HEAD -> apis/main" in any of the test files > I have). On the other hand I can't reproduce it so I will need some help with > this I think. I similarly could not reproduce it (I ran 'make test' on this topic before integrating it into 'seen', and it passed, otherwise I wouldn't have picked it up). I am not sure what the differences are. The 'ci' directory has some more bits on how the various suites are run, and the '.github/workflows' directory has some more bits still. Thanks, Taylor
On Wed Oct 16, 2024 at 23:05, Taylor Blau <me@ttaylorr.com> wrote: > On Wed, Oct 16, 2024 at 10:18:44AM +0200, Bence Ferdinandy wrote: >> Thanks for the heads up! I see that indeed the failing output is different than >> what I have locally (e.g. no "apis/HEAD -> apis/main" in any of the test files >> I have). On the other hand I can't reproduce it so I will need some help with >> this I think. > > I similarly could not reproduce it (I ran 'make test' on this topic > before integrating it into 'seen', and it passed, otherwise I wouldn't > have picked it up). > > I am not sure what the differences are. The 'ci' directory has some more > bits on how the various suites are run, and the '.github/workflows' > directory has some more bits still. I managed to reproduce it in docker once, but I'm not sure how and I wasn't able to do it again ... On the other hand (using some printf) I managed to figure out that `fetch --multiple --all` is running both in the CI and locally and the difference is actually what we see in the output (so no error per se). E.g. in t5505.55 update, if you run it locally refs/remotes/apis/HEAD does not exist, while in the CI it does. I'm not sure why this difference exists yet, so I'm hoping someone might have an idea. I'll keep poking around in the meanwhile. Thanks, Bence
v8 is minor non-functional fix from v7 ❯ git range-diff v7~6..v7 v8~6..v8 1: e4a2d01bca = 1: e4a2d01bca refs: atomically record overwritten ref in update_symref 2: e6c40ef04c = 2: e6c40ef04c remote set-head: add new variable for readability 3: aefa49aa0d = 3: aefa49aa0d remote set-head: better output for --auto 4: ad8907c6a3 = 4: ad8907c6a3 refs: add TRANSACTION_CREATE_EXISTS error 5: ff616d8b77 = 5: ff616d8b77 refs: add create_only option to refs_update_symref 6: 35b93f454c ! 6: 7b844b1dbb fetch: set remote/HEAD if it does not exist @@ Notes v7: - no change + v8: - changed logmsg in call to refs_update_symref from "remote + set-head" to "fetch" + ## builtin/fetch.c ## @@ builtin/fetch.c: static int backfill_tags(struct display_state *display_state, return retcode; @@ builtin/fetch.c: static int backfill_tags(struct display_state *display_state, + if (!refs_ref_exists(refs, b_remote_head.buf)) + result = 1; + else if (refs_update_symref(refs, b_head.buf, b_remote_head.buf, -+ "remote set-head", &b_local_head, 1)) ++ "fetch", &b_local_head, 1)) + result = 1; + else + report_set_head(remote, head_name, &b_local_head); Bence Ferdinandy (6): refs: atomically record overwritten ref in update_symref remote set-head: add new variable for readability remote set-head: better output for --auto refs: add TRANSACTION_CREATE_EXISTS error refs: add create_only option to refs_update_symref fetch: set remote/HEAD if it does not exist builtin/branch.c | 2 +- builtin/checkout.c | 5 +- builtin/clone.c | 8 +- builtin/fetch.c | 82 +++++++++++ builtin/notes.c | 3 +- builtin/remote.c | 47 +++++-- builtin/symbolic-ref.c | 2 +- builtin/worktree.c | 2 +- refs.c | 38 +++-- refs.h | 7 +- refs/files-backend.c | 24 ++-- refs/reftable-backend.c | 6 +- reset.c | 2 +- sequencer.c | 3 +- setup.c | 3 +- t/helper/test-ref-store.c | 2 +- t/t4207-log-decoration-colors.sh | 3 +- t/t5505-remote.sh | 62 ++++++++- t/t5510-fetch.sh | 229 ++++++++++++++++--------------- t/t5512-ls-remote.sh | 2 + t/t5514-fetch-multiple.sh | 17 ++- t/t5516-fetch-push.sh | 3 +- t/t5527-fetch-odd-refs.sh | 3 +- t/t7900-maintenance.sh | 3 +- t/t9210-scalar.sh | 5 +- t/t9211-scalar-clone.sh | 6 +- t/t9902-completion.sh | 65 +++++++++ 27 files changed, 469 insertions(+), 165 deletions(-)