Re: [PATCH v4 0/8] Auto-configure advertised remotes via URL allowlist

[Toon Claes <toon@iotcl.com>]

_(resend because it seems I accidentally didn't reply-all)_

Christian Couder <christian.couder@gmail.com> writes:

> Changes compared to v3
> ======================
>
> Thanks to Toon, Kristoffer, Patrick and Junio for reviewing the
> previous versions of this series and of the preparatory series.
>
> This has been rebased onto master @ 56a4f3c3a2 (The 8th batch,
> 2026-05-25) to avoid a trivial conflict in "urlmatch.c".
>
> Only minor changes have been made since v3, in the following patches:
>
>  - Patch 4/8 ("promisor-remote: add 'local_name' to 'struct
>    promisor_info'"):
>
>    - The promisor_info_internal_name() function has been renamed
>      promisor_info_local_name() for clarity.
>
>    - A `const char *local` local variable has been renamed
>      `remote_name` for consistency with another similar variable.

I can really appreciate these two changes. Both make things more
consistent and cleaner.

>  - Patch 6/8 ("promisor-remote: trust known remotes matching
>    acceptFromServerUrl"):

I previously reviewed v2 and compared to that I like the changes you've
made toward being clear about precedence. And this consistency carries
through in PATCH 7/8.

And thanks for mentioning username and password components are ignored
intentionally.

But I previously mentioned I felt the naming of 'acceptFromServer' and
'acceptFromServerUrl' are a bit confusing. So I'm wondering whether we
can consider another proposal:

What if 'acceptFromServer' would configure if 'acceptFromServerUrl'
should be used? I mean, imagine we put this in the config:

    [promisor]
        acceptFromServer = Match
        acceptFromServerUrl = https://my-org.com/*

(we can still argue over naming, but to get the idea)

So the value "Match" for 'acceptFromServer' would inform Git to use
'acceptFromServerUrl'. This way precedence isn't a concern no more,
because every value for 'acceptFromServer' is mutually exclusive.

This has one downside though, you can no longer combine
acceptFromServer=KnownUrl with a 'acceptFromServerUrl'. So URLs
advertised by the server can no longer fall-through to
'acceptFromServer' if they don't match 'acceptFromServerUrl'. You can
argue whether that's a good thing or not.

What do you think? If you disagree, I'm fine with the current approach
and I think this version looks good.

-- 
Cheers,
Toon