mbox series

[v10,0/8] set-head/fetch remote/HEAD updates

Message ID 20241021134354.705636-1-bence@ferdinandy.com (mailing list archive)
Headers show
Series set-head/fetch remote/HEAD updates | expand

Message

Bence Ferdinandy Oct. 21, 2024, 1:36 p.m. UTC
Sorry for sending a v10 so soon after v9 without really waiting for feedback,
but as I started digging into
https://lore.kernel.org/git/D4ZAELFWJMKN.S88LJ6YK31LZ@ferdinandy.com/ I
realized that the root cause is in fetch and not remote add, so the solution
belongs to this series. I'll leave my overall comments for v9 in here for
clarity.

v10: added a new patch (8/8) on top of v9 to handle HEAD in mirrored bare
repositories, rest of the patches are unchanged

v9:
the two most notable changes are
- the new patch 1/7 which address
  https://lore.kernel.org/git/Zw8IKyPkG0Hr6%2F5t@nand.local/, but see
  https://lore.kernel.org/git/D4ZAELFWJMKN.S88LJ6YK31LZ@ferdinandy.com/
- the refs_update_symref -> refs_update_symref_extended change in 2/7,
  reflecting on Phillip's comments (see
  https://lore.kernel.org/git/a7cb48e5-d8ba-44c1-9dbe-d1e8f8a63e3c@gmail.com/)

Hopefully with 1/7 the series is ready to move back to seen :)

Best,
Bence

Bence Ferdinandy (8):
  t/t5505-remote: set default branch to main
  refs: atomically record overwritten ref in update_symref
  remote set-head: refactor for readability
  remote set-head: better output for --auto
  refs: add TRANSACTION_CREATE_EXISTS error
  refs: add create_only option to refs_update_symref_extended
  fetch: set remote/HEAD if it does not exist
  fetch set_head: handle mirrored bare repositories

 builtin/fetch.c                  |  90 ++++++++++++
 builtin/remote.c                 |  52 +++++--
 refs.c                           |  41 +++++-
 refs.h                           |   8 +-
 refs/files-backend.c             |  24 ++--
 refs/reftable-backend.c          |   6 +-
 t/t4207-log-decoration-colors.sh |   3 +-
 t/t5505-remote.sh                |  83 ++++++++++-
 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 +++++++++
 17 files changed, 486 insertions(+), 154 deletions(-)

Comments

Taylor Blau Oct. 21, 2024, 8:43 p.m. UTC | #1
On Mon, Oct 21, 2024 at 03:36:57PM +0200, Bence Ferdinandy wrote:
> Sorry for sending a v10 so soon after v9 without really waiting for feedback,
> but as I started digging into
> https://lore.kernel.org/git/D4ZAELFWJMKN.S88LJ6YK31LZ@ferdinandy.com/ I
> realized that the root cause is in fetch and not remote add, so the solution
> belongs to this series. I'll leave my overall comments for v9 in here for
> clarity.
>
> v10: added a new patch (8/8) on top of v9 to handle HEAD in mirrored bare
> repositories, rest of the patches are unchanged

Thanks. For other readers, this is a continuation of the v9 from this
thread:

    https://lore.kernel.org/git/20241019225330.4001477-1-bence@ferdinandy.com/

Thanks,
Taylor
Bence Ferdinandy Oct. 21, 2024, 9:06 p.m. UTC | #2
2024. okt. 21. 22:43:38 Taylor Blau <me@ttaylorr.com>:

> On Mon, Oct 21, 2024 at 03:36:57PM +0200, Bence Ferdinandy wrote:
>> Sorry for sending a v10 so soon after v9 without really waiting for 
>> feedback,
>> but as I started digging into
>> https://lore.kernel.org/git/D4ZAELFWJMKN.S88LJ6YK31LZ@ferdinandy.com/ 
>> I
>> realized that the root cause is in fetch and not remote add, so the 
>> solution
>> belongs to this series. I'll leave my overall comments for v9 in here 
>> for
>> clarity.
>>
>> v10: added a new patch (8/8) on top of v9 to handle HEAD in mirrored 
>> bare
>> repositories, rest of the patches are unchanged
>
> Thanks. For other readers, this is a continuation of the v9 from this
> thread:
>
>     
> https://lore.kernel.org/git/20241019225330.4001477-1-bence@ferdinandy.com/

And people may also want to drop the accidental noreply address in Cc 
when replying :) Sorry for the mixup!
Taylor Blau Oct. 22, 2024, 5:39 p.m. UTC | #3
On Mon, Oct 21, 2024 at 03:36:57PM +0200, Bence Ferdinandy wrote:
> Bence Ferdinandy (8):
>   t/t5505-remote: set default branch to main
>   refs: atomically record overwritten ref in update_symref
>   remote set-head: refactor for readability
>   remote set-head: better output for --auto
>   refs: add TRANSACTION_CREATE_EXISTS error
>   refs: add create_only option to refs_update_symref_extended
>   fetch: set remote/HEAD if it does not exist
>   fetch set_head: handle mirrored bare repositories

After applying this topic to 'seen', I noticed some new CI breakage that
appears to be caused by this topic:

    https://github.com/git/git/actions/runs/11449483611/job/31855171514#step:4:2506

After dropping the topic locally and building with 'make SANITIZE=leak'
and then running t0410-partial-clone.sh, I was able to resolve the
failures.

Would you mind building with 'SANITIZE=leak' and running that script to
see if you can address the issue? Thanks.

Thanks,
Taylor
Bence Ferdinandy Oct. 22, 2024, 6:33 p.m. UTC | #4
On Tue Oct 22, 2024 at 19:39, Taylor Blau <me@ttaylorr.com> wrote:
> On Mon, Oct 21, 2024 at 03:36:57PM +0200, Bence Ferdinandy wrote:
>> Bence Ferdinandy (8):
>>   t/t5505-remote: set default branch to main
>>   refs: atomically record overwritten ref in update_symref
>>   remote set-head: refactor for readability
>>   remote set-head: better output for --auto
>>   refs: add TRANSACTION_CREATE_EXISTS error
>>   refs: add create_only option to refs_update_symref_extended
>>   fetch: set remote/HEAD if it does not exist
>>   fetch set_head: handle mirrored bare repositories
>
> After applying this topic to 'seen', I noticed some new CI breakage that
> appears to be caused by this topic:
>
>     https://github.com/git/git/actions/runs/11449483611/job/31855171514#step:4:2506
>
> After dropping the topic locally and building with 'make SANITIZE=leak'
> and then running t0410-partial-clone.sh, I was able to resolve the
> failures.
>
> Would you mind building with 'SANITIZE=leak' and running that script to
> see if you can address the issue? Thanks.

Thanks, I can reproduce the issue. I'm hoping there are no more tricks to
testing things :D

Let's see how fast I can handle this ...

Best,
Bence
Taylor Blau Oct. 22, 2024, 7:04 p.m. UTC | #5
On Tue, Oct 22, 2024 at 08:33:28PM +0200, Bence Ferdinandy wrote:
>
> On Tue Oct 22, 2024 at 19:39, Taylor Blau <me@ttaylorr.com> wrote:
> > On Mon, Oct 21, 2024 at 03:36:57PM +0200, Bence Ferdinandy wrote:
> >> Bence Ferdinandy (8):
> >>   t/t5505-remote: set default branch to main
> >>   refs: atomically record overwritten ref in update_symref
> >>   remote set-head: refactor for readability
> >>   remote set-head: better output for --auto
> >>   refs: add TRANSACTION_CREATE_EXISTS error
> >>   refs: add create_only option to refs_update_symref_extended
> >>   fetch: set remote/HEAD if it does not exist
> >>   fetch set_head: handle mirrored bare repositories
> >
> > After applying this topic to 'seen', I noticed some new CI breakage that
> > appears to be caused by this topic:
> >
> >     https://github.com/git/git/actions/runs/11449483611/job/31855171514#step:4:2506
> >
> > After dropping the topic locally and building with 'make SANITIZE=leak'
> > and then running t0410-partial-clone.sh, I was able to resolve the
> > failures.
> >
> > Would you mind building with 'SANITIZE=leak' and running that script to
> > see if you can address the issue? Thanks.
>
> Thanks, I can reproduce the issue. I'm hoping there are no more tricks to
> testing things :D

Great, I'm glad that it was easily reproducible on your end. This one
should have minimal differences from the setup in CI, as I think
building with 'SANITIZE=leak' will suffice.

> Let's see how fast I can handle this ...

Thanks for looking.

Thanks,
Taylor
Bence Ferdinandy Oct. 22, 2024, 7:09 p.m. UTC | #6
On Tue Oct 22, 2024 at 21:04, Taylor Blau <me@ttaylorr.com> wrote:
> On Tue, Oct 22, 2024 at 08:33:28PM +0200, Bence Ferdinandy wrote:
>>
>> On Tue Oct 22, 2024 at 19:39, Taylor Blau <me@ttaylorr.com> wrote:
>> > On Mon, Oct 21, 2024 at 03:36:57PM +0200, Bence Ferdinandy wrote:
>> >> Bence Ferdinandy (8):
>> >>   t/t5505-remote: set default branch to main
>> >>   refs: atomically record overwritten ref in update_symref
>> >>   remote set-head: refactor for readability
>> >>   remote set-head: better output for --auto
>> >>   refs: add TRANSACTION_CREATE_EXISTS error
>> >>   refs: add create_only option to refs_update_symref_extended
>> >>   fetch: set remote/HEAD if it does not exist
>> >>   fetch set_head: handle mirrored bare repositories
>> >
>> > After applying this topic to 'seen', I noticed some new CI breakage that
>> > appears to be caused by this topic:
>> >
>> >     https://github.com/git/git/actions/runs/11449483611/job/31855171514#step:4:2506
>> >
>> > After dropping the topic locally and building with 'make SANITIZE=leak'
>> > and then running t0410-partial-clone.sh, I was able to resolve the
>> > failures.
>> >
>> > Would you mind building with 'SANITIZE=leak' and running that script to
>> > see if you can address the issue? Thanks.
>>
>> Thanks, I can reproduce the issue. I'm hoping there are no more tricks to
>> testing things :D
>
> Great, I'm glad that it was easily reproducible on your end. This one
> should have minimal differences from the setup in CI, as I think
> building with 'SANITIZE=leak' will suffice.
>
>> Let's see how fast I can handle this ...
>
> Thanks for looking.

I'll send v11 to the correct thread, hopefully with no random companies in Cc :)