mbox series

[0/3] maintenance: configure credentials to be silent

Message ID pull.1798.git.1726790423.gitgitgadget@gmail.com (mailing list archive)
Headers show
Series maintenance: configure credentials to be silent | expand

Message

Philippe Blain via GitGitGadget Sept. 20, 2024, midnight UTC
When background maintenance attempts to perform a prefetch to remote
servers, this may trigger authentication requirements. If the credentials
are expired, then the credential helper may need user input in order to get
refreshed credentials. It is not a good experience for users to get
credential pop-ups when not directly interacting with Git.

Add a new configuration value, 'credential.interactive', to specify to the
credential helper that it should not prompt for user interaction. This
option has been respected by Git Credential Manager since 2020 [1], so this
is now presenting it as an official Git config value.

These changes were first merged into the microsoft/git fork in August 2023
[2] but were not upstreamed immediately. The change has been a positive one
for users of that fork, as they no longer get pop-ups and they also are not
getting maintenance.lock file blocks when the prefetch task waits for
credentials. This has become even more important recently as credential
lifetimes have been restricted significantly, leading to a higher likelihood
that this will happen during a background prefetch.

I was reminded of these changes when liuzhongbo started a discussion [3]
about maintenance.lock files and requesting that they are removed if they
are stale. This does not address that issue directly, but is an important
way to reduce the lifetime of maintenance.lock files when blocked on
credential prompts.

[1] https://github.com/git-ecosystem/git-credential-manager/pull/91

[2] https://github.com/microsoft/git/pull/598

[3]
https://lore.kernel.org/git/cce1d054-911e-407e-bc26-1c0bac4dd8e4@gmail.com/T/#t

Thanks, -Stolee

Derrick Stolee (3):
  credential: add new interactive config option
  maintenance: add custom config to background jobs
  scalar: configure maintenance during 'reconfigure'

 Documentation/config/credential.txt |  8 +++++
 builtin/gc.c                        | 53 +++++++++++++++++++++++++----
 credential.c                        | 30 ++++++++++++++--
 scalar.c                            |  3 ++
 t/t5551-http-fetch-smart.sh         | 22 ++++++++++++
 t/t7900-maintenance.sh              |  3 ++
 t/t9210-scalar.sh                   |  7 ++--
 7 files changed, 114 insertions(+), 12 deletions(-)


base-commit: 6531f31ef3bead57a3255fa08efa6e7553c5a9a7
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1798%2Fderrickstolee%2Fbackground-quiet-credentials-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1798/derrickstolee/background-quiet-credentials-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1798

Comments

Junio C Hamano Sept. 20, 2024, 9:56 p.m. UTC | #1
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Add a new configuration value, 'credential.interactive', to specify to the
> credential helper that it should not prompt for user interaction. This
> option has been respected by Git Credential Manager since 2020 [1], so this
> is now presenting it as an official Git config value.

So, the other helpers are also supposed to check for the variable
and fail when it has to go interactive now.

> These changes were first merged into the microsoft/git fork in August 2023
> [2] but were not upstreamed immediately. The change has been a positive one
> for users of that fork, as they no longer get pop-ups and they also are not
> getting maintenance.lock file blocks when the prefetch task waits for
> credentials. This has become even more important recently as credential
> lifetimes have been restricted significantly, leading to a higher likelihood
> that this will happen during a background prefetch.

Sounds good.
Derrick Stolee Sept. 23, 2024, 1:36 a.m. UTC | #2
On 9/20/24 5:56 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> Add a new configuration value, 'credential.interactive', to specify to the
>> credential helper that it should not prompt for user interaction. This
>> option has been respected by Git Credential Manager since 2020 [1], so this
>> is now presenting it as an official Git config value.
> 
> So, the other helpers are also supposed to check for the variable
> and fail when it has to go interactive now.

I would hold off from saying "supposed to" but Git is definitely hinting
towards that behavior.

Perhaps I'm just hung up on the idea that we are not adding a new wrinkle
to the "contract" but recommending a good thing that was previously not part
of the interaction.

Thanks,
-Stolee
Junio C Hamano Sept. 23, 2024, 4:24 p.m. UTC | #3
Derrick Stolee <stolee@gmail.com> writes:

> On 9/20/24 5:56 PM, Junio C Hamano wrote:
>> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>> 
>>> Add a new configuration value, 'credential.interactive', to specify to the
>>> credential helper that it should not prompt for user interaction. This
>>> option has been respected by Git Credential Manager since 2020 [1], so this
>>> is now presenting it as an official Git config value.
>> So, the other helpers are also supposed to check for the variable
>> and fail when it has to go interactive now.
>
> I would hold off from saying "supposed to" but Git is definitely hinting
> towards that behavior.

I would too.  I didn't mean "they were behaving correctly, but we
changed the rules from under them and they need to be fixed".  With
or without your patch, they would try to go interactive and make the
process get stuck, until they start to check if they should refrain
from going interactive.  With your patch, they have a way to do that
check in a documented way.

> Perhaps I'm just hung up on the idea that we are not adding a new wrinkle
> to the "contract" but recommending a good thing that was previously not part
> of the interaction.
>
> Thanks,
> -Stolee