diff mbox series

[v2] doc: document how uploadpack.allowAnySHA1InWant impact other allow options

Message ID pull.1814.v2.git.git.1729355997353.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit bddfccead1f4a94f4029d1efeacaed0d23e2fbb4
Headers show
Series [v2] doc: document how uploadpack.allowAnySHA1InWant impact other allow options | expand

Commit Message

Piotr Szlazak Oct. 19, 2024, 4:39 p.m. UTC
From: Piotr Szlazak <piotr.szlazak@gmail.com>

Document how setting of `uploadpack.allowAnySHA1InWant`
influences other `uploadpack` options - `allowTipSHA1InWant`
and `allowReachableSHA1InWant`.

Signed-off-by: Piotr Szlazak <piotr.szlazak@gmail.com>
---
    doc: document how uploadpack.allowAnySHA1InWant impact other allow
    options

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1814%2Fpszlazak%2Fupload-pack-allow-flags-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1814/pszlazak/upload-pack-allow-flags-v2
Pull-Request: https://github.com/git/git/pull/1814

Range-diff vs v1:

 1:  8a2673bdf31 < -:  ----------- upload-pack: fix how ALLOW_ANY_SHA1 flag is disabled
 -:  ----------- > 1:  2a9fa4dabba doc: document how uploadpack.allowAnySHA1InWant impact other allow options


 Documentation/config/uploadpack.txt | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)


base-commit: 34b6ce9b30747131b6e781ff718a45328aa887d0

Comments

Piotr Szlazak Oct. 19, 2024, 4:46 p.m. UTC | #1
On 19.10.2024 18:39, Piotr Szlazak via GitGitGadget wrote:
> From: Piotr Szlazak <piotr.szlazak@gmail.com>
>
> Document how setting of `uploadpack.allowAnySHA1InWant`
> influences other `uploadpack` options - `allowTipSHA1InWant`
> and `allowReachableSHA1InWant`.
>
> Signed-off-by: Piotr Szlazak <piotr.szlazak@gmail.com>
> ---
>      doc: document how uploadpack.allowAnySHA1InWant impact other allow
>      options
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1814%2Fpszlazak%2Fupload-pack-allow-flags-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1814/pszlazak/upload-pack-allow-flags-v2
> Pull-Request: https://github.com/git/git/pull/1814
>
> Range-diff vs v1:
>
>   1:  8a2673bdf31 < -:  ----------- upload-pack: fix how ALLOW_ANY_SHA1 flag is disabled
>   -:  ----------- > 1:  2a9fa4dabba doc: document how uploadpack.allowAnySHA1InWant impact other allow options
>
>
>   Documentation/config/uploadpack.txt | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
> index 16264d82a72..0e1dda944a5 100644
> --- a/Documentation/config/uploadpack.txt
> +++ b/Documentation/config/uploadpack.txt
> @@ -25,7 +25,11 @@ uploadpack.allowReachableSHA1InWant::
>   uploadpack.allowAnySHA1InWant::
>   	Allow `upload-pack` to accept a fetch request that asks for any
>   	object at all.
> -	Defaults to `false`.
> +	It implies `uploadpack.allowTipSHA1InWant` and
> +	`uploadpack.allowReachableSHA1InWant`. If set to `true` it will
> +	enable both of them, it set to `false` it will disable both of
> +	them.
> +	By default not set.
>   
>   uploadpack.keepAlive::
>   	When `upload-pack` has started `pack-objects`, there may be a
>
> base-commit: 34b6ce9b30747131b6e781ff718a45328aa887d0

PATCH v2 which updates documentation.

I wrote 'By default not set', as definitely 'Defaults to `false`' in not 
true.

Only when `uploadpack.allowAnySHA1InWant` set to `true` or `false` it 
will affect reported capabilities.

Regards,

Piotr
Piotr Szlazak Oct. 21, 2024, 5:55 a.m. UTC | #2
On 19.10.2024 18:46, Piotr Szlazak wrote:
>
> On 19.10.2024 18:39, Piotr Szlazak via GitGitGadget wrote:
>> From: Piotr Szlazak <piotr.szlazak@gmail.com>
>>
>> Document how setting of `uploadpack.allowAnySHA1InWant`
>> influences other `uploadpack` options - `allowTipSHA1InWant`
>> and `allowReachableSHA1InWant`.
>>
>> Signed-off-by: Piotr Szlazak <piotr.szlazak@gmail.com>
>> ---
>>      doc: document how uploadpack.allowAnySHA1InWant impact other allow
>>      options
>>
>> Published-As: 
>> https://github.com/gitgitgadget/git/releases/tag/pr-git-1814%2Fpszlazak%2Fupload-pack-allow-flags-v2
>> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git 
>> pr-git-1814/pszlazak/upload-pack-allow-flags-v2
>> Pull-Request: https://github.com/git/git/pull/1814
>>
>> Range-diff vs v1:
>>
>>   1:  8a2673bdf31 < -:  ----------- upload-pack: fix how 
>> ALLOW_ANY_SHA1 flag is disabled
>>   -:  ----------- > 1:  2a9fa4dabba doc: document how 
>> uploadpack.allowAnySHA1InWant impact other allow options
>>
>>
>>   Documentation/config/uploadpack.txt | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/config/uploadpack.txt 
>> b/Documentation/config/uploadpack.txt
>> index 16264d82a72..0e1dda944a5 100644
>> --- a/Documentation/config/uploadpack.txt
>> +++ b/Documentation/config/uploadpack.txt
>> @@ -25,7 +25,11 @@ uploadpack.allowReachableSHA1InWant::
>>   uploadpack.allowAnySHA1InWant::
>>       Allow `upload-pack` to accept a fetch request that asks for any
>>       object at all.
>> -    Defaults to `false`.
>> +    It implies `uploadpack.allowTipSHA1InWant` and
>> +    `uploadpack.allowReachableSHA1InWant`. If set to `true` it will
>> +    enable both of them, it set to `false` it will disable both of
>> +    them.
>> +    By default not set.
>>     uploadpack.keepAlive::
>>       When `upload-pack` has started `pack-objects`, there may be a
>>
>> base-commit: 34b6ce9b30747131b6e781ff718a45328aa887d0
>
> PATCH v2 which updates documentation.
>
> I wrote 'By default not set', as definitely 'Defaults to `false`' in 
> not true.
>
> Only when `uploadpack.allowAnySHA1InWant` set to `true` or `false` it 
> will affect reported capabilities.
>
> Regards,
>
> Piotr

On the second look code changes will be needed, as at the moment final 
result will differ between:
[uploadpack]
         allowTipSHA1InWant = true
         allowReachableSHA1InWant = true
         allowAnySHAInWant = false

and:

[uploadpack]
         allowAnySHAInWant = false
         allowTipSHA1InWant = true
         allowReachableSHA1InWant = true

Regards,
Piotr
Jeff King Oct. 21, 2024, 7:03 p.m. UTC | #3
On Mon, Oct 21, 2024 at 07:55:06AM +0200, Piotr Szlazak wrote:

> On the second look code changes will be needed, as at the moment final
> result will differ between:
> [uploadpack]
>         allowTipSHA1InWant = true
>         allowReachableSHA1InWant = true
>         allowAnySHAInWant = false
> 
> and:
> 
> [uploadpack]
>         allowAnySHAInWant = false
>         allowTipSHA1InWant = true
>         allowReachableSHA1InWant = true

I'd expect them to differ. The config is read in order with a "last one
wins" rule. The thing that the documentation should be making clear is
that the three overlap in what they are affecting, and so "last one" is
not just a single key, but these three inter-related keys.

-Peff
Taylor Blau Oct. 21, 2024, 7:47 p.m. UTC | #4
On Mon, Oct 21, 2024 at 03:03:09PM -0400, Jeff King wrote:
> On Mon, Oct 21, 2024 at 07:55:06AM +0200, Piotr Szlazak wrote:
>
> > On the second look code changes will be needed, as at the moment final
> > result will differ between:
> > [uploadpack]
> >         allowTipSHA1InWant = true
> >         allowReachableSHA1InWant = true
> >         allowAnySHAInWant = false
> >
> > and:
> >
> > [uploadpack]
> >         allowAnySHAInWant = false
> >         allowTipSHA1InWant = true
> >         allowReachableSHA1InWant = true
>
> I'd expect them to differ. The config is read in order with a "last one
> wins" rule. The thing that the documentation should be making clear is
> that the three overlap in what they are affecting, and so "last one" is
> not just a single key, but these three inter-related keys.

Agreed. I think the "last one wins" policy is well understood throughout
Git's rules for how configuration layers are parsed, and not worth
repeating purely for the purposes of this specific piece of the
documentation.

Thanks,
Taylor
Taylor Blau Oct. 21, 2024, 7:48 p.m. UTC | #5
On Sat, Oct 19, 2024 at 04:39:57PM +0000, Piotr Szlazak via GitGitGadget wrote:
> From: Piotr Szlazak <piotr.szlazak@gmail.com>
>
> Document how setting of `uploadpack.allowAnySHA1InWant`
> influences other `uploadpack` options - `allowTipSHA1InWant`
> and `allowReachableSHA1InWant`.
>
> Signed-off-by: Piotr Szlazak <piotr.szlazak@gmail.com>
> ---
>     doc: document how uploadpack.allowAnySHA1InWant impact other allow
>     options

Thanks, this version looks quite good to me.

Thanks,
Taylor
diff mbox series

Patch

diff --git a/Documentation/config/uploadpack.txt b/Documentation/config/uploadpack.txt
index 16264d82a72..0e1dda944a5 100644
--- a/Documentation/config/uploadpack.txt
+++ b/Documentation/config/uploadpack.txt
@@ -25,7 +25,11 @@  uploadpack.allowReachableSHA1InWant::
 uploadpack.allowAnySHA1InWant::
 	Allow `upload-pack` to accept a fetch request that asks for any
 	object at all.
-	Defaults to `false`.
+	It implies `uploadpack.allowTipSHA1InWant` and
+	`uploadpack.allowReachableSHA1InWant`. If set to `true` it will
+	enable both of them, it set to `false` it will disable both of
+	them.
+	By default not set.
 
 uploadpack.keepAlive::
 	When `upload-pack` has started `pack-objects`, there may be a