mbox series

[v4,0/5] improve handling of remote/HEAD

Message ID 20240930222025.2349008-1-bence@ferdinandy.com (mailing list archive)
Headers show
Series improve handling of remote/HEAD | expand

Message

Bence Ferdinandy Sept. 30, 2024, 10:19 p.m. UTC 
This version has dropped some previous patches in favour of new ones.
These solve the problem of atomically deciding what the head was before
we updated it and also not reinventing the wheel for having an "create
this ref, but if it already exists, fail silently" call.

Since now both changes to remote set-head and fetch utilize some common
preceding patches I have again included in this series my changes to
remote set-head. Sorry for the mess tangent there.

All tests (that run on my system) pass for the patches up to the
last patch, which changes fetch. For the last patch I updated some of
the test where it was trivial (lot of remote/HEAD -> something appeared
as expected), but there is still work to be done there (i.e. figure out
which tests need updating and which are actual bugs in the patch).

One specific thing I wanted to mention here. Junio asked earlier what
happens on fetch, when we have a config like this:

> If I am only fetching refs (or HEAD) in FETCH_HEAD for immediate
> consumtion by doing "git pull bence HEAD" with something like
>
>     [remote "bence"]
>          URL = http://github.com/bence/git

In this case setting head will error out with the generic error message
I put at the end.

Bence Ferdinandy (5):
  refs_update_symref: atomically record overwritten ref
  set-head: better output for --auto
  transaction: add TRANSACTION_CREATE_EXISTS error
  refs_update_symref: add create_only option
  fetch: set remote/HEAD if it does not exist

 builtin/branch.c          |  2 +-
 builtin/checkout.c        |  5 ++-
 builtin/clone.c           |  8 ++--
 builtin/fetch.c           | 83 +++++++++++++++++++++++++++++++++++++++
 builtin/notes.c           |  3 +-
 builtin/remote.c          | 43 ++++++++++++++++----
 builtin/symbolic-ref.c    |  2 +-
 builtin/worktree.c        |  2 +-
 refs.c                    | 35 ++++++++++++-----
 refs.h                    |  7 +++-
 refs/files-backend.c      | 29 ++++++++++----
 refs/refs-internal.h      |  8 ++++
 refs/reftable-backend.c   |  6 ++-
 reset.c                   |  2 +-
 sequencer.c               |  3 +-
 setup.c                   |  3 +-
 t/helper/test-ref-store.c |  2 +-
 t/t5505-remote.sh         | 13 +++++-
 t/t5514-fetch-multiple.sh |  9 +++++
 19 files changed, 222 insertions(+), 43 deletions(-)

Comments

Junio C Hamano Oct. 3, 2024, 7:21 p.m. UTC | #1 
Bence Ferdinandy <bence@ferdinandy.com> writes:

> Bence Ferdinandy (5):
>   refs_update_symref: atomically record overwritten ref
>   set-head: better output for --auto
>   transaction: add TRANSACTION_CREATE_EXISTS error
>   refs_update_symref: add create_only option
>   fetch: set remote/HEAD if it does not exist
>
>  builtin/branch.c          |  2 +-
>  builtin/checkout.c        |  5 ++-
>  builtin/clone.c           |  8 ++--
>  builtin/fetch.c           | 83 +++++++++++++++++++++++++++++++++++++++
>  builtin/notes.c           |  3 +-
>  builtin/remote.c          | 43 ++++++++++++++++----
>  builtin/symbolic-ref.c    |  2 +-
>  builtin/worktree.c        |  2 +-
>  refs.c                    | 35 ++++++++++++-----
>  refs.h                    |  7 +++-
>  refs/files-backend.c      | 29 ++++++++++----
>  refs/refs-internal.h      |  8 ++++
>  refs/reftable-backend.c   |  6 ++-
>  reset.c                   |  2 +-
>  sequencer.c               |  3 +-
>  setup.c                   |  3 +-
>  t/helper/test-ref-store.c |  2 +-
>  t/t5505-remote.sh         | 13 +++++-
>  t/t5514-fetch-multiple.sh |  9 +++++
>  19 files changed, 222 insertions(+), 43 deletions(-)

These seem to break some of the tests, either standalone or when
merged to 'seen'.

I have the topic queued directly on top of e9356ba3 (another batch
after 2.47-rc0, 2024-09-30); here is how the summary report looks
like.

Thanks.

Test Summary Report
-------------------
t0410-partial-clone.sh                           (Wstat: 256 (exited 1) Tests: 38 Failed: 1)
  Failed test:  32
  Non-zero exit status: 1
t4207-log-decoration-colors.sh                   (Wstat: 256 (exited 1) Tests: 4 Failed: 1)
  Failed test:  2
  Non-zero exit status: 1
t5300-pack-object.sh                             (Wstat: 256 (exited 1) Tests: 56 Failed: 1)
  Failed test:  51
  Non-zero exit status: 1
t5512-ls-remote.sh                               (Wstat: 256 (exited 1) Tests: 40 Failed: 1)
  Failed test:  32
  Non-zero exit status: 1
t5527-fetch-odd-refs.sh                          (Wstat: 256 (exited 1) Tests: 5 Failed: 1)
  Failed test:  4
  Non-zero exit status: 1
t5514-fetch-multiple.sh                          (Wstat: 256 (exited 1) Tests: 25 Failed: 4)
  Failed tests:  9-10, 18, 24
  Non-zero exit status: 1
t5521-pull-options.sh                            (Wstat: 256 (exited 1) Tests: 22 Failed: 8)
  Failed tests:  2-9
  Non-zero exit status: 1
t5505-remote.sh                                  (Wstat: 256 (exited 1) Tests: 114 Failed: 7)
  Failed tests:  4, 31, 51-52, 54-56
  Non-zero exit status: 1
t5574-fetch-output.sh                            (Wstat: 256 (exited 1) Tests: 14 Failed: 3)
  Failed tests:  11-13
  Non-zero exit status: 1
t5703-upload-pack-ref-in-want.sh                 (Wstat: 256 (exited 1) Tests: 26 Failed: 1)
  Failed test:  12
  Non-zero exit status: 1
t5616-partial-clone.sh                           (Wstat: 256 (exited 1) Tests: 46 Failed: 2)
  Failed tests:  10, 22
  Non-zero exit status: 1
t5516-fetch-push.sh                              (Wstat: 256 (exited 1) Tests: 120 Failed: 1)
  Failed test:  102
  Non-zero exit status: 1
t7814-grep-recurse-submodules.sh                 (Wstat: 256 (exited 1) Tests: 34 Failed: 1)
  Failed test:  33
  Non-zero exit status: 1
t7900-maintenance.sh                             (Wstat: 256 (exited 1) Tests: 53 Failed: 1)
  Failed test:  23
  Non-zero exit status: 1
t7406-submodule-update.sh                        (Wstat: 256 (exited 1) Tests: 68 Failed: 1)
  Failed test:  59
  Non-zero exit status: 1
t9211-scalar-clone.sh                            (Wstat: 256 (exited 1) Tests: 13 Failed: 2)
  Failed tests:  8-9
  Non-zero exit status: 1
t9210-scalar.sh                                  (Wstat: 256 (exited 1) Tests: 21 Failed: 1)
  Failed test:  11
  Non-zero exit status: 1
t9902-completion.sh                              (Wstat: 256 (exited 1) Tests: 256 Failed: 56)
  Failed tests:  54-55, 59, 61, 63, 71-75, 77, 86, 88-89
                92-93, 95, 101, 113, 116-117, 119-129, 138-141
                143-157, 162-163, 168, 170-171
  Non-zero exit status: 1
Files=1029, Tests=31709, 371 wallclock secs (11.74 usr  4.74 sys + 853.87 cusr 6465.88 csys = 7336.23 CPU)
Result: FAIL
Bence Ferdinandy Oct. 3, 2024, 7:48 p.m. UTC | #2 
2024. okt. 3. 21:22:13 Junio C Hamano <gitster@pobox.com>:

> Bence Ferdinandy <bence@ferdinandy.com> writes:
>
>> Bence Ferdinandy (5):
>>   refs_update_symref: atomically record overwritten ref
>>   set-head: better output for --auto
>>   transaction: add TRANSACTION_CREATE_EXISTS error
>>   refs_update_symref: add create_only option
>>   fetch: set remote/HEAD if it does not exist
>>
>> builtin/branch.c          |  2 +-
>> builtin/checkout.c        |  5 ++-
>> builtin/clone.c           |  8 ++--
>> builtin/fetch.c           | 83 +++++++++++++++++++++++++++++++++++++++
>> builtin/notes.c           |  3 +-
>> builtin/remote.c          | 43 ++++++++++++++++----
>> builtin/symbolic-ref.c    |  2 +-
>> builtin/worktree.c        |  2 +-
>> refs.c                    | 35 ++++++++++++-----
>> refs.h                    |  7 +++-
>> refs/files-backend.c      | 29 ++++++++++----
>> refs/refs-internal.h      |  8 ++++
>> refs/reftable-backend.c   |  6 ++-
>> reset.c                   |  2 +-
>> sequencer.c               |  3 +-
>> setup.c                   |  3 +-
>> t/helper/test-ref-store.c |  2 +-
>> t/t5505-remote.sh         | 13 +++++-
>> t/t5514-fetch-multiple.sh |  9 +++++
>> 19 files changed, 222 insertions(+), 43 deletions(-)
>
> These seem to break some of the tests, either standalone or when
> merged to 'seen'.
>
> I have the topic queued directly on top of e9356ba3 (another batch
> after 2.47-rc0, 2024-09-30); here is how the summary report looks
> like.
>
> Thanks.

Sorry, I should have probably left in the RFC prefix to be more clear
about this... As I noted in the cover letter, the first four patches pass 
the
tests, the last one doesn't (yet), as I'm still not quite sure if even 
the
approach is fine. If those look good I'll send a v5 and also fix the 
tests
as well. Although based on your review of the first 2 patches there's 
already
some changes needed there, so I can just do a v5 with that and tests 
fixed
and see if that version looks good or not.

I'm currently on a holiday, so my ETA on this is early next week.

Thanks,
Bence


>
> Test Summary Report
> -------------------
> t0410-partial-clone.sh                           (Wstat: 256 (exited 1) 
> Tests: 38 Failed: 1)
>   Failed test:  32
>   Non-zero exit status: 1
> t4207-log-decoration-colors.sh                   (Wstat: 256 (exited 1) 
> Tests: 4 Failed: 1)
>   Failed test:  2
>   Non-zero exit status: 1
> t5300-pack-object.sh                             (Wstat: 256 (exited 1) 
> Tests: 56 Failed: 1)
>   Failed test:  51
>   Non-zero exit status: 1
> t5512-ls-remote.sh                               (Wstat: 256 (exited 1) 
> Tests: 40 Failed: 1)
>   Failed test:  32
>   Non-zero exit status: 1
> t5527-fetch-odd-refs.sh                          (Wstat: 256 (exited 1) 
> Tests: 5 Failed: 1)
>   Failed test:  4
>   Non-zero exit status: 1
> t5514-fetch-multiple.sh                          (Wstat: 256 (exited 1) 
> Tests: 25 Failed: 4)
>   Failed tests:  9-10, 18, 24
>   Non-zero exit status: 1
> t5521-pull-options.sh                            (Wstat: 256 (exited 1) 
> Tests: 22 Failed: 8)
>   Failed tests:  2-9
>   Non-zero exit status: 1
> t5505-remote.sh                                  (Wstat: 256 (exited 1) 
> Tests: 114 Failed: 7)
>   Failed tests:  4, 31, 51-52, 54-56
>   Non-zero exit status: 1
> t5574-fetch-output.sh                            (Wstat: 256 (exited 1) 
> Tests: 14 Failed: 3)
>   Failed tests:  11-13
>   Non-zero exit status: 1
> t5703-upload-pack-ref-in-want.sh                 (Wstat: 256 (exited 1) 
> Tests: 26 Failed: 1)
>   Failed test:  12
>   Non-zero exit status: 1
> t5616-partial-clone.sh                           (Wstat: 256 (exited 1) 
> Tests: 46 Failed: 2)
>   Failed tests:  10, 22
>   Non-zero exit status: 1
> t5516-fetch-push.sh                              (Wstat: 256 (exited 1) 
> Tests: 120 Failed: 1)
>   Failed test:  102
>   Non-zero exit status: 1
> t7814-grep-recurse-submodules.sh                 (Wstat: 256 (exited 1) 
> Tests: 34 Failed: 1)
>   Failed test:  33
>   Non-zero exit status: 1
> t7900-maintenance.sh                             (Wstat: 256 (exited 1) 
> Tests: 53 Failed: 1)
>   Failed test:  23
>   Non-zero exit status: 1
> t7406-submodule-update.sh                        (Wstat: 256 (exited 1) 
> Tests: 68 Failed: 1)
>   Failed test:  59
>   Non-zero exit status: 1
> t9211-scalar-clone.sh                            (Wstat: 256 (exited 1) 
> Tests: 13 Failed: 2)
>   Failed tests:  8-9
>   Non-zero exit status: 1
> t9210-scalar.sh                                  (Wstat: 256 (exited 1) 
> Tests: 21 Failed: 1)
>   Failed test:  11
>   Non-zero exit status: 1
> t9902-completion.sh                              (Wstat: 256 (exited 1) 
> Tests: 256 Failed: 56)
>   Failed tests:  54-55, 59, 61, 63, 71-75, 77, 86, 88-89
>                 92-93, 95, 101, 113, 116-117, 119-129, 138-141
>                 143-157, 162-163, 168, 170-171
>   Non-zero exit status: 1
> Files=1029, Tests=31709, 371 wallclock secs (11.74 usr  4.74 sys + 
> 853.87 cusr 6465.88 csys = 7336.23 CPU)
> Result: FAIL