gitgitgadget / git

GitGitGadget's Git fork. Open Pull Requests here to submit them to the Git mailing list
https://gitgitgadget.github.io/
Other
222 stars 133 forks source link

maintenance: configure credentials to be silent #1798

Closed derrickstolee closed 2 months ago

derrickstolee commented 2 months ago

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

cc: gitster@pobox.com cc: liuzhongbo.gg@gmail.com cc: Johannes.Schindelin@gmx.de

derrickstolee commented 2 months ago

/submit

gitgitgadget[bot] commented 2 months ago

Submitted as pull.1798.git.1726790423.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-1798/derrickstolee/background-quiet-credentials-v1

To fetch this version to local tag pr-1798/derrickstolee/background-quiet-credentials-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-1798/derrickstolee/background-quiet-credentials-v1
gitgitgadget[bot] commented 2 months ago

On the Git mailing list, Junio C Hamano wrote (reply to this):

"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.
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/b52518a6ae6abec6d29a257a26e538d945cc6840.

gitgitgadget[bot] commented 2 months ago

On the Git mailing list, Derrick Stolee wrote (reply to this):

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
gitgitgadget[bot] commented 2 months ago

On the Git mailing list, Junio C Hamano wrote (reply to this):

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
gitgitgadget[bot] commented 2 months ago

This branch is now known as ds/background-maintenance-with-credential.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/e47c2b8d9145157a734ce7542e58494ab4dc3452.

gitgitgadget[bot] commented 2 months ago

There was a status update in the "Cooking" section about the branch ds/background-maintenance-with-credential on the Git mailing list:

Background tasks "git maintenance" runs may need to use credential
information when going over the network, but a credential helper
may work only in an interactive environment, and end up blocking a
scheduled task waiting for UI.  Credential helpers can now behave
differently when they are not running interactively.

Will merge to 'next'.
source: <pull.1798.git.1726790423.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/fc5446781a00bbb62c298e5762875e35ea29dfe9.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into next via https://github.com/git/git/commit/379a7a1003d2a1183e1afa614e69c4381b376f77.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/c261349883958a23fadd97d6fdbe3ef50cf7a5bc.

gitgitgadget[bot] commented 2 months ago

There was a status update in the "Cooking" section about the branch ds/background-maintenance-with-credential on the Git mailing list:

Background tasks "git maintenance" runs may need to use credential
information when going over the network, but a credential helper
may work only in an interactive environment, and end up blocking a
scheduled task waiting for UI.  Credential helpers can now behave
differently when they are not running interactively.

Will merge to 'master'.
source: <pull.1798.git.1726790423.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/9617c1716ca172ec7b7fa604b69452a78ebd1363.

gitgitgadget[bot] commented 2 months ago

There was a status update in the "Cooking" section about the branch ds/background-maintenance-with-credential on the Git mailing list:

Background tasks "git maintenance" runs may need to use credential
information when going over the network, but a credential helper
may work only in an interactive environment, and end up blocking a
scheduled task waiting for UI.  Credential helpers can now behave
differently when they are not running interactively.

Will merge to 'master'.
source: <pull.1798.git.1726790423.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/a99e922652f2add9bc0acc2a07081ddc9ca7322c.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/8999b8ed15a1b4bce4fb4e3bda057aee915160b5.

gitgitgadget[bot] commented 2 months ago

There was a status update in the "Cooking" section about the branch ds/background-maintenance-with-credential on the Git mailing list:

Background tasks "git maintenance" runs may need to use credential
information when going over the network, but a credential helper
may work only in an interactive environment, and end up blocking a
scheduled task waiting for UI.  Credential helpers can now behave
differently when they are not running interactively.

Will merge to 'master'.
source: <pull.1798.git.1726790423.gitgitgadget@gmail.com>
gitgitgadget[bot] commented 2 months ago

This patch series was integrated into seen via https://github.com/git/git/commit/4251403327392b6384137149c87f6738171f4537.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into master via https://github.com/git/git/commit/4251403327392b6384137149c87f6738171f4537.

gitgitgadget[bot] commented 2 months ago

This patch series was integrated into next via https://github.com/git/git/commit/4251403327392b6384137149c87f6738171f4537.

gitgitgadget[bot] commented 2 months ago

Closed via 4251403327392b6384137149c87f6738171f4537.