{"thread":{"id":"62147","subject":"[PATCH 0/3] maintenance: configure credentials to be silent","startedAt":"2024-09-20T00:00:27Z","lastAt":"2024-09-23T16:24:43Z","messageCount":8,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"503108","messageId":"pull.1798.git.1726790423.gitgitgadget@gmail.com","threadId":"62147","inReplyTo":null,"subject":"[PATCH 0/3] maintenance: configure credentials to be silent","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-20T00:00:20Z","receivedAt":"2024-09-20T00:00:27Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"When background maintenance attempts to perform a prefetch to remote\nservers, this may trigger authentication requirements. If the credentials\nare expired, then the credential helper may need user input in order to get\nrefreshed credentials. It is not a good experience for users to get\ncredential pop-ups when not directly interacting with Git.\n\nAdd a new configuration value, 'credential.interactive', to specify to the\ncredential helper that it should not prompt for user interaction. This\noption has been respected by Git Credential Manager since 2020 [1], so this\nis now presenting it as an official Git config value.\n\nThese changes were first merged into the microsoft/git fork in August 2023\n[2] but were not upstreamed immediately. The change has been a positive one\nfor users of that fork, as they no longer get pop-ups and they also are not\ngetting maintenance.lock file blocks when the prefetch task waits for\ncredentials. This has become even more important recently as credential\nlifetimes have been restricted significantly, leading to a higher likelihood\nthat this will happen during a background prefetch.\n\nI was reminded of these changes when liuzhongbo started a discussion [3]\nabout maintenance.lock files and requesting that they are removed if they\nare stale. This does not address that issue directly, but is an important\nway to reduce the lifetime of maintenance.lock files when blocked on\ncredential prompts.\n\n[1] https://github.com/git-ecosystem/git-credential-manager/pull/91\n\n[2] https://github.com/microsoft/git/pull/598\n\n[3]\nhttps://lore.kernel.org/git/cce1d054-911e-407e-bc26-1c0bac4dd8e4@gmail.com/T/#t\n\nThanks, -Stolee\n\nDerrick Stolee (3):\n credential: add new interactive config option\n maintenance: add custom config to background jobs\n scalar: configure maintenance during 'reconfigure'\n\n Documentation/config/credential.txt | 8 +++++\n builtin/gc.c | 53 +++++++++++++++++++++++++----\n credential.c | 30 ++++++++++++++--\n scalar.c | 3 ++\n t/t5551-http-fetch-smart.sh | 22 ++++++++++++\n t/t7900-maintenance.sh | 3 ++\n t/t9210-scalar.sh | 7 ++--\n 7 files changed, 114 insertions(+), 12 deletions(-)\n\n\nbase-commit: 6531f31ef3bead57a3255fa08efa6e7553c5a9a7\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1798%2Fderrickstolee%2Fbackground-quiet-credentials-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1798/derrickstolee/background-quiet-credentials-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1798\n-- \ngitgitgadget\n"},{"id":"503109","messageId":"1e9bf2d09c17bc0cdcd0a8f8dbacab007e5c53e7.1726790424.git.gitgitgadget@gmail.com","threadId":"62147","inReplyTo":"pull.1798.git.1726790423.gitgitgadget@gmail.com","subject":"[PATCH 1/3] credential: add new interactive config option","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-20T00:00:21Z","receivedAt":"2024-09-20T00:00:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen scripts or background maintenance wish to perform HTTP(S) requests,\nthere is a risk that our stored credentials might be invalid. At the\nmoment, this causes the credential helper to ping the user and block the\nprocess. Even if the credential helper does not ping the user, Git falls\nback to the 'askpass' method, which includes a direct ping to the user\nvia the terminal.\n\nEven setting the 'core.askPass' config as something like 'echo' will\ncauses Git to fallback to a terminal prompt. It uses\ngit_terminal_prompt(), which finds the terminal from the environment and\nignores whether stdin has been redirected. This can also block the\nprocess awaiting input.\n\nCreate a new config option to prevent user interaction, favoring a\nfailure to a blocked process.\n\nThe chosen name, 'credential.interactive', is taken from the config\noption used by Git Credential Manager to already avoid user\ninteractivity, so there is already one credential helper that integrates\nwith this option. However, older versions of Git Credential Manager also\naccepted other string values, including 'auto', 'never', and 'always'.\nThe modern use is to use a boolean value, but we should still be\ncareful that some users could have these non-booleans. Further, we\nshould respect 'never' the same as 'false'. This is respected by the\nimplementation and test, but not mentioned in the documentation.\n\nThe implementation for the Git interactions takes place within\ncredential_getpass(). The method prototype is modified to return an\n'int' instead of 'void'. This allows us to detect that no attempt was\nmade to fill the given credential, changing the single caller slightly.\n\nAlso, a new trace2 region is added around the interactive portion of the\ncredential request. This provides a way to measure the amount of time\nspent in that region for commands that _are_ interactive. It also makes\na conventient way to test that the config option works with\n'test_region'.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/config/credential.txt | 8 ++++++++\n credential.c | 30 ++++++++++++++++++++++++++---\n t/t5551-http-fetch-smart.sh | 22 +++++++++++++++++++++\n 3 files changed, 57 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config/credential.txt b/Documentation/config/credential.txt\nindex 0221c3e620d..470482ff4c2 100644\n--- a/Documentation/config/credential.txt\n+++ b/Documentation/config/credential.txt\n@@ -9,6 +9,14 @@ credential.helper::\n Note that multiple helpers may be defined. See linkgit:gitcredentials[7]\n for details and examples.\n \n+credential.interactive::\n+\tBy default, Git and any configured credential helpers will ask for\n+\tuser input when new credentials are required. Many of these helpers\n+\twill succeed based on stored credentials if those credentials are\n+\tstill valid. To avoid the possibility of user interactivity from\n+\tGit, set `credential.interactive=false`. Some credential helpers\n+\trespect this option as well.\n+\n credential.useHttpPath::\n \tWhen acquiring credentials, consider the \"path\" component of an http\n \tor https URL to be important. Defaults to false. See\ndiff --git a/credential.c b/credential.c\nindex ee46351ce01..6dea3859ece 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -13,6 +13,8 @@\n #include \"strbuf.h\"\n #include \"urlmatch.h\"\n #include \"git-compat-util.h\"\n+#include \"trace2.h\"\n+#include \"repository.h\"\n \n void credential_init(struct credential *c)\n {\n@@ -251,14 +253,36 @@ static char *credential_ask_one(const char *what, struct credential *c,\n \treturn xstrdup(r);\n }\n \n-static void credential_getpass(struct credential *c)\n+static int credential_getpass(struct credential *c)\n {\n+\tint interactive;\n+\tchar *value;\n+\tif (!git_config_get_maybe_bool(\"credential.interactive\", &interactive) &&\n+\t !interactive) {\n+\t\ttrace2_data_intmax(\"credential\", the_repository,\n+\t\t\t\t \"interactive/skipped\", 1);\n+\t\treturn -1;\n+\t}\n+\tif (!git_config_get_string(\"credential.interactive\", &value)) {\n+\t\tint same = !strcmp(value, \"never\");\n+\t\tfree(value);\n+\t\tif (same) {\n+\t\t\ttrace2_data_intmax(\"credential\", the_repository,\n+\t\t\t\t\t \"interactive/skipped\", 1);\n+\t\t\treturn -1;\n+\t\t}\n+\t}\n+\n+\ttrace2_region_enter(\"credential\", \"interactive\", the_repository);\n \tif (!c->username)\n \t\tc->username = credential_ask_one(\"Username\", c,\n \t\t\t\t\t\t PROMPT_ASKPASS|PROMPT_ECHO);\n \tif (!c->password)\n \t\tc->password = credential_ask_one(\"Password\", c,\n \t\t\t\t\t\t PROMPT_ASKPASS);\n+\ttrace2_region_leave(\"credential\", \"interactive\", the_repository);\n+\n+\treturn 0;\n }\n \n int credential_has_capability(const struct credential_capability *capa,\n@@ -501,8 +525,8 @@ void credential_fill(struct credential *c, int all_capabilities)\n \t\t\t c->helpers.items[i].string);\n \t}\n \n-\tcredential_getpass(c);\n-\tif (!c->username && !c->password && !c->credential)\n+\tif (credential_getpass(c) ||\n+\t (!c->username && !c->password && !c->credential))\n \t\tdie(\"unable to get password from user\");\n }\n \ndiff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\nindex 7b5ab0eae16..ceb3336a5c4 100755\n--- a/t/t5551-http-fetch-smart.sh\n+++ b/t/t5551-http-fetch-smart.sh\n@@ -186,6 +186,28 @@ test_expect_success 'clone from password-protected repository' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'credential.interactive=false skips askpass' '\n+\tset_askpass bogus nonsense &&\n+\t(\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/interactive-true\" &&\n+\t\texport GIT_TRACE2_EVENT &&\n+\t\ttest_must_fail git clone --bare \"$HTTPD_URL/auth/smart/repo.git\" interactive-true-dir &&\n+\t\ttest_region credential interactive interactive-true &&\n+\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/interactive-false\" &&\n+\t\texport GIT_TRACE2_EVENT &&\n+\t\ttest_must_fail git -c credential.interactive=false \\\n+\t\t\tclone --bare \"$HTTPD_URL/auth/smart/repo.git\" interactive-false-dir &&\n+\t\ttest_region ! credential interactive interactive-false &&\n+\n+\t\tGIT_TRACE2_EVENT=\"$(pwd)/interactive-never\" &&\n+\t\texport GIT_TRACE2_EVENT &&\n+\t\ttest_must_fail git -c credential.interactive=never \\\n+\t\t\tclone --bare \"$HTTPD_URL/auth/smart/repo.git\" interactive-never-dir &&\n+\t\ttest_region ! credential interactive interactive-never\n+\t)\n+'\n+\n test_expect_success 'clone from auth-only-for-push repository' '\n \techo two >expect &&\n \tset_askpass wrong &&\n-- \ngitgitgadget\n\n"},{"id":"503110","messageId":"92ee323d31bf43decf0f230a7cc36b0f942bde6b.1726790424.git.gitgitgadget@gmail.com","threadId":"62147","inReplyTo":"pull.1798.git.1726790423.gitgitgadget@gmail.com","subject":"[PATCH 2/3] maintenance: add custom config to background jobs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-20T00:00:22Z","receivedAt":"2024-09-20T00:00:29Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nAt the moment, some background jobs are getting blocked on credentials\nduring the 'prefetch' task. This leads to other tasks, such as\nincremental repacks, getting blocked. Further, if a user manages to fix\ntheir credentials, then they still need to cancel the background process\nbefore their background maintenance can continue working.\n\nUpdate the background schedules for our four scheduler integrations to\ninclude these config options via '-c' options:\n\n * 'credential.interactive=false' will stop Git and some credential\n helpers from prompting in the UI (assuming the '-c' parameters are\n carried through and respected by GCM).\n\n * 'core.askPass=true' will replace the text fallback for a username\n and password into the 'true' command, which will return a success in\n its exit code, but Git will treat the empty string returned as an\n invalid password and move on.\n\nWe can do some testing that the credentials are passed, at least in the\nsystemd case due to writing the service files.\n\nSigned-off-by: Derrick Stolee \n---\n builtin/gc.c | 53 ++++++++++++++++++++++++++++++++++++------\n t/t7900-maintenance.sh | 3 +++\n 2 files changed, 49 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 7dac9714054..fb1be542e06 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -1766,6 +1766,42 @@ static const char *get_frequency(enum schedule_priority schedule)\n \t}\n }\n \n+static const char *extraconfig[] = {\n+\t\"credential.interactive=false\",\n+\t\"core.askPass=true\", /* 'true' returns success, but no output. */\n+\tNULL\n+};\n+\n+static const char *get_extra_config_parameters(void) {\n+\tstatic const char *result = NULL;\n+\tstruct strbuf builder = STRBUF_INIT;\n+\n+\tif (result)\n+\t\treturn result;\n+\n+\tfor (const char **s = extraconfig; s && *s; s++)\n+\t\tstrbuf_addf(&builder, \"-c %s \", *s);\n+\n+\tresult = strbuf_detach(&builder, NULL);\n+\treturn result;\n+}\n+\n+static const char *get_extra_launchctl_strings(void) {\n+\tstatic const char *result = NULL;\n+\tstruct strbuf builder = STRBUF_INIT;\n+\n+\tif (result)\n+\t\treturn result;\n+\n+\tfor (const char **s = extraconfig; s && *s; s++) {\n+\t\tstrbuf_addstr(&builder, \"-c\\n\");\n+\t\tstrbuf_addf(&builder, \"%s\\n\", *s);\n+\t}\n+\n+\tresult = strbuf_detach(&builder, NULL);\n+\treturn result;\n+}\n+\n /*\n * get_schedule_cmd` reads the GIT_TEST_MAINT_SCHEDULER environment variable\n * to mock the schedulers that `git maintenance start` rely on.\n@@ -1972,6 +2008,7 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit\n \t\t \"\\n\"\n \t\t \"%s/git\\n\"\n \t\t \"--exec-path=%s\\n\"\n+\t\t \"%s\" /* For extra config parameters. */\n \t\t \"for-each-repo\\n\"\n \t\t \"--keep-going\\n\"\n \t\t \"--config=maintenance.repo\\n\"\n@@ -1981,7 +2018,8 @@ static int launchctl_schedule_plist(const char *exec_path, enum schedule_priorit\n \t\t \"\\n\"\n \t\t \"StartCalendarInterval\\n\"\n \t\t \"\\n\";\n-\tstrbuf_addf(&plist, preamble, name, exec_path, exec_path, frequency);\n+\tstrbuf_addf(&plist, preamble, name, exec_path, exec_path,\n+\t\t get_extra_launchctl_strings(), frequency);\n \n \tswitch (schedule) {\n \tcase SCHEDULE_HOURLY:\n@@ -2216,11 +2254,12 @@ static int schtasks_schedule_task(const char *exec_path, enum schedule_priority\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\\"%s\\\\headless-git.exe\\\"\\n\"\n-\t \"--exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s\\n\"\n+\t \"--exec-path=\\\"%s\\\" %s for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%s\\n\"\n \t \"\\n\"\n \t \"\\n\"\n \t \"\\n\";\n-\tfprintf(tfile->fp, xml, exec_path, exec_path, frequency);\n+\tfprintf(tfile->fp, xml, exec_path, exec_path,\n+\t\tget_extra_config_parameters(), frequency);\n \tstrvec_split(&child.args, cmd);\n \tstrvec_pushl(&child.args, \"/create\", \"/tn\", name, \"/f\", \"/xml\",\n \t\t\t\t get_tempfile_path(tfile), NULL);\n@@ -2361,8 +2400,8 @@ static int crontab_update_schedule(int run_maintenance, int fd)\n \t\t\t\"# replaced in the future by a Git command.\\n\\n\");\n \n \t\tstrbuf_addf(&line_format,\n-\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n-\t\t\t exec_path, exec_path);\n+\t\t\t \"%%d %%s * * %%s \\\"%s/git\\\" --exec-path=\\\"%s\\\" %s for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%s\\n\",\n+\t\t\t exec_path, exec_path, get_extra_config_parameters());\n \t\tfprintf(cron_in, line_format.buf, minute, \"1-23\", \"*\", \"hourly\");\n \t\tfprintf(cron_in, line_format.buf, minute, \"0\", \"1-6\", \"daily\");\n \t\tfprintf(cron_in, line_format.buf, minute, \"0\", \"0\", \"weekly\");\n@@ -2562,7 +2601,7 @@ static int systemd_timer_write_service_template(const char *exec_path)\n \t \"\\n\"\n \t \"[Service]\\n\"\n \t \"Type=oneshot\\n\"\n-\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n+\t \"ExecStart=\\\"%s/git\\\" --exec-path=\\\"%s\\\" %s for-each-repo --keep-going --config=maintenance.repo maintenance run --schedule=%%i\\n\"\n \t \"LockPersonality=yes\\n\"\n \t \"MemoryDenyWriteExecute=yes\\n\"\n \t \"NoNewPrivileges=yes\\n\"\n@@ -2572,7 +2611,7 @@ static int systemd_timer_write_service_template(const char *exec_path)\n \t \"RestrictSUIDSGID=yes\\n\"\n \t \"SystemCallArchitectures=native\\n\"\n \t \"SystemCallFilter=@system-service\\n\";\n-\tif (fprintf(file, unit, exec_path, exec_path) < 0) {\n+\tif (fprintf(file, unit, exec_path, exec_path, get_extra_config_parameters()) < 0) {\n \t\terror(_(\"failed to write to '%s'\"), filename);\n \t\tfclose(file);\n \t\tgoto error;\ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex abae7a97546..3cd7e1fcacb 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -825,6 +825,9 @@ test_expect_success 'start and stop Linux/systemd maintenance' '\n \ttest_systemd_analyze_verify \"systemd/user/git-maintenance@daily.service\" &&\n \ttest_systemd_analyze_verify \"systemd/user/git-maintenance@weekly.service\" &&\n \n+\tgrep \"core.askPass=true\" \"systemd/user/git-maintenance@.service\" &&\n+\tgrep \"credential.interactive=false\" \"systemd/user/git-maintenance@.service\" &&\n+\n \tprintf -- \"--user enable --now git-maintenance@%s.timer\\n\" hourly daily weekly >expect &&\n \ttest_cmp expect args &&\n \n-- \ngitgitgadget\n\n"},{"id":"503111","messageId":"965a08a5d526ae75428727d0f9aa22ea22a25ed9.1726790424.git.gitgitgadget@gmail.com","threadId":"62147","inReplyTo":"pull.1798.git.1726790423.gitgitgadget@gmail.com","subject":"[PATCH 3/3] scalar: configure maintenance during 'reconfigure'","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2024-09-20T00:00:23Z","receivedAt":"2024-09-20T00:00:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe 'scalar reconfigure' command is intended to update registered repos\nwith the latest settings available. However, up to now we were not\nreregistering the repos with background maintenance.\n\nIn particular, this meant that the background maintenance schedule would\nnot be updated if there are improvements between versions.\n\nBe sure to register repos for maintenance during the reconfigure step.\n\nSigned-off-by: Derrick Stolee \n---\n scalar.c | 3 +++\n t/t9210-scalar.sh | 7 +++++--\n 2 files changed, 8 insertions(+), 2 deletions(-)\n\ndiff --git a/scalar.c b/scalar.c\nindex 09560aeab54..73b79a5d4c9 100644\n--- a/scalar.c\n+++ b/scalar.c\n@@ -733,6 +733,9 @@ static int cmd_reconfigure(int argc, const char **argv)\n \n \t\tthe_repository = old_repo;\n \n+\t\tif (toggle_maintenance(1) >= 0)\n+\t\t\tsucceeded = 1;\n+\n loop_end:\n \t\tif (!succeeded) {\n \t\t\tres = -1;\ndiff --git a/t/t9210-scalar.sh b/t/t9210-scalar.sh\nindex e8613990e13..027235d61aa 100755\n--- a/t/t9210-scalar.sh\n+++ b/t/t9210-scalar.sh\n@@ -194,8 +194,11 @@ test_expect_success 'scalar reconfigure' '\n \tscalar reconfigure one &&\n \ttest true = \"$(git -C one/src config core.preloadIndex)\" &&\n \tgit -C one/src config core.preloadIndex false &&\n-\tscalar reconfigure -a &&\n-\ttest true = \"$(git -C one/src config core.preloadIndex)\"\n+\trm one/src/cron.txt &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/reconfigure\" scalar reconfigure -a &&\n+\ttest_path_is_file one/src/cron.txt &&\n+\ttest true = \"$(git -C one/src config core.preloadIndex)\" &&\n+\ttest_subcommand git maintenance start writes:\n\n> Add a new configuration value, 'credential.interactive', to specify to the\n> credential helper that it should not prompt for user interaction. This\n> option has been respected by Git Credential Manager since 2020 [1], so this\n> is now presenting it as an official Git config value.\n\nSo, the other helpers are also supposed to check for the variable\nand fail when it has to go interactive now.\n\n> These changes were first merged into the microsoft/git fork in August 2023\n> [2] but were not upstreamed immediately. The change has been a positive one\n> for users of that fork, as they no longer get pop-ups and they also are not\n> getting maintenance.lock file blocks when the prefetch task waits for\n> credentials. This has become even more important recently as credential\n> lifetimes have been restricted significantly, leading to a higher likelihood\n> that this will happen during a background prefetch.\n\nSounds good.\n\n"},{"id":"503171","messageId":"xmqq7cb6dln7.fsf@gitster.g","threadId":"62147","inReplyTo":"1e9bf2d09c17bc0cdcd0a8f8dbacab007e5c53e7.1726790424.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/3] credential: add new interactive config option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-20T22:07:40Z","receivedAt":"2024-09-20T22:07:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> @@ -501,8 +525,8 @@ void credential_fill(struct credential *c, int all_capabilities)\n> \t\t\t c->helpers.items[i].string);\n> \t}\n> \n> -\tcredential_getpass(c);\n> -\tif (!c->username && !c->password && !c->credential)\n> +\tif (credential_getpass(c) ||\n> +\t (!c->username && !c->password && !c->credential))\n> \t\tdie(\"unable to get password from user\");\n> }\n\nThis is a fallback mode after credential helpers have failed to fill\nand return. Unless these helpers pay attention to the \"interactive\"\nconfiguration, they may still get stuck. So it would be #leftoverbits\nto update each credential helpers to do the right thing.\n\nThe sample credential-store backend does not have to be updated I\nguess ;-)\n\n> diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh\n> index 7b5ab0eae16..ceb3336a5c4 100755\n> --- a/t/t5551-http-fetch-smart.sh\n> +++ b/t/t5551-http-fetch-smart.sh\n> @@ -186,6 +186,28 @@ test_expect_success 'clone from password-protected repository' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'credential.interactive=false skips askpass' '\n> +\tset_askpass bogus nonsense &&\n> +\t(\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/interactive-true\" &&\n> +\t\texport GIT_TRACE2_EVENT &&\n> +\t\ttest_must_fail git clone --bare \"$HTTPD_URL/auth/smart/repo.git\" interactive-true-dir &&\n> +\t\ttest_region credential interactive interactive-true &&\n> +\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/interactive-false\" &&\n> +\t\texport GIT_TRACE2_EVENT &&\n> +\t\ttest_must_fail git -c credential.interactive=false \\\n> +\t\t\tclone --bare \"$HTTPD_URL/auth/smart/repo.git\" interactive-false-dir &&\n> +\t\ttest_region ! credential interactive interactive-false &&\n> +\n> +\t\tGIT_TRACE2_EVENT=\"$(pwd)/interactive-never\" &&\n> +\t\texport GIT_TRACE2_EVENT &&\n> +\t\ttest_must_fail git -c credential.interactive=never \\\n> +\t\t\tclone --bare \"$HTTPD_URL/auth/smart/repo.git\" interactive-never-dir &&\n> +\t\ttest_region ! credential interactive interactive-never\n> +\t)\n> +'\n> +\n> test_expect_success 'clone from auth-only-for-push repository' '\n> \techo two >expect &&\n> \tset_askpass wrong &&\n"},{"id":"503232","messageId":"5114e7fa-7858-458a-bc5d-5c82ea663a48@gmail.com","threadId":"62147","inReplyTo":"xmqqfrpudm51.fsf@gitster.g","subject":"Re: [PATCH 0/3] maintenance: configure credentials to be silent","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2024-09-23T01:36:02Z","receivedAt":"2024-09-23T01:36:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/20/24 5:56 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> Add a new configuration value, 'credential.interactive', to specify to the\n>> credential helper that it should not prompt for user interaction. This\n>> option has been respected by Git Credential Manager since 2020 [1], so this\n>> is now presenting it as an official Git config value.\n> \n> So, the other helpers are also supposed to check for the variable\n> and fail when it has to go interactive now.\n\nI would hold off from saying \"supposed to\" but Git is definitely hinting\ntowards that behavior.\n\nPerhaps I'm just hung up on the idea that we are not adding a new wrinkle\nto the \"contract\" but recommending a good thing that was previously not part\nof the interaction.\n\nThanks,\n-Stolee\n\n"},{"id":"503262","messageId":"xmqqsetq49tj.fsf@gitster.g","threadId":"62147","inReplyTo":"5114e7fa-7858-458a-bc5d-5c82ea663a48@gmail.com","subject":"Re: [PATCH 0/3] maintenance: configure credentials to be silent","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-09-23T16:24:40Z","receivedAt":"2024-09-23T16:24:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 9/20/24 5:56 PM, Junio C Hamano wrote:\n>> \"Derrick Stolee via GitGitGadget\" writes:\n>> \n>>> Add a new configuration value, 'credential.interactive', to specify to the\n>>> credential helper that it should not prompt for user interaction. This\n>>> option has been respected by Git Credential Manager since 2020 [1], so this\n>>> is now presenting it as an official Git config value.\n>> So, the other helpers are also supposed to check for the variable\n>> and fail when it has to go interactive now.\n>\n> I would hold off from saying \"supposed to\" but Git is definitely hinting\n> towards that behavior.\n\nI would too. I didn't mean \"they were behaving correctly, but we\nchanged the rules from under them and they need to be fixed\". With\nor without your patch, they would try to go interactive and make the\nprocess get stuck, until they start to check if they should refrain\nfrom going interactive. With your patch, they have a way to do that\ncheck in a documented way.\n\n> Perhaps I'm just hung up on the idea that we are not adding a new wrinkle\n> to the \"contract\" but recommending a good thing that was previously not part\n> of the interaction.\n>\n> Thanks,\n> -Stolee\n"}]}