From b1299de4a1e9a193db2bfeb23508250ebbe2d67b Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 7 Jun 2022 14:50:50 -0700 Subject: cocci: retire is_null_sha1() rule Since 8d4d86b0 (cache: remove null_sha1, 2019-08-18) removed the is_null_sha1() function, rewrite rules to correct callers of the function to use is_null_oid() instead has become irrelevant, as any new callers of the function will get caught by the compiler much more quickly without spending cycles on Coccinelle. Remove these rules. Signed-off-by: Junio C Hamano --- contrib/coccinelle/object_id.cocci | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci index ddf4f22bd7..01f8d6935b 100644 --- a/contrib/coccinelle/object_id.cocci +++ b/contrib/coccinelle/object_id.cocci @@ -1,15 +1,3 @@ -@@ -struct object_id OID; -@@ -- is_null_sha1(OID.hash) -+ is_null_oid(&OID) - -@@ -struct object_id *OIDPTR; -@@ -- is_null_sha1(OIDPTR->hash) -+ is_null_oid(OIDPTR) - @@ struct object_id OID; @@ -- cgit v1.3 From 5aeb145780ffdfd516ad6fa697e7e14ba29be0bf Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 8 Jun 2022 10:43:18 +0000 Subject: ci(github): bring back the 'print test failures' step Git now shows better information in the GitHub workflow runs when a test case failed. However, when a test case was implemented incorrectly and therefore does not even run, nothing is shown. Let's bring back the step that prints the full logs of the failed tests, and to improve the user experience, print out an informational message for readers so that they do not have to know/remember where to see the full logs. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- .github/workflows/main.yml | 16 ++++++++++++++++ ci/lib.sh | 3 ++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 3fa88b78b6..cd1f52692a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -119,6 +119,10 @@ jobs: - name: test shell: bash run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10 + - name: print test failures + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + shell: bash + run: ci/print-test-failures.sh - name: Upload failed tests' directories if: failure() && env.FAILED_TEST_ARTIFACTS != '' uses: actions/upload-artifact@v2 @@ -200,6 +204,10 @@ jobs: env: NO_SVN_TESTS: 1 run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10 + - name: print test failures + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + shell: bash + run: ci/print-test-failures.sh - name: Upload failed tests' directories if: failure() && env.FAILED_TEST_ARTIFACTS != '' uses: actions/upload-artifact@v2 @@ -253,6 +261,10 @@ jobs: - uses: actions/checkout@v2 - run: ci/install-dependencies.sh - run: ci/run-build-and-tests.sh + - name: print test failures + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + shell: bash + run: ci/print-test-failures.sh - name: Upload failed tests' directories if: failure() && env.FAILED_TEST_ARTIFACTS != '' uses: actions/upload-artifact@v2 @@ -282,6 +294,10 @@ jobs: - uses: actions/checkout@v1 - run: ci/install-docker-dependencies.sh - run: ci/run-build-and-tests.sh + - name: print test failures + if: failure() && env.FAILED_TEST_ARTIFACTS != '' + shell: bash + run: ci/print-test-failures.sh - name: Upload failed tests' directories if: failure() && env.FAILED_TEST_ARTIFACTS != '' uses: actions/upload-artifact@v1 diff --git a/ci/lib.sh b/ci/lib.sh index 2f6d9d26e4..b142d254ec 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -177,7 +177,8 @@ then test_name="${test_exit%.exit}" test_name="${test_name##*/}" printf "\\e[33m\\e[1m=== Failed test: ${test_name} ===\\e[m\\n" - echo "The full logs are in the artifacts attached to this run." + echo "The full logs are in the 'print test failures' step below." + echo "See also the 'failed-tests-*' artifacts attached to this run." cat "t/test-results/$test_name.markup" trash_dir="t/trash directory.$test_name" -- cgit v1.3 From df5fed9c34a394b55194b3fb69413bcc4c76fd64 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Jun 2022 13:13:07 +0000 Subject: ci(github): use grouping also in the `win-build` job We already do the same when building Git in all the other jobs. This will allow us to piggy-back on top of grouping to mark up compiler errors in the next commit. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- ci/make-test-artifacts.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/make-test-artifacts.sh b/ci/make-test-artifacts.sh index 646967481f..74141af0cc 100755 --- a/ci/make-test-artifacts.sh +++ b/ci/make-test-artifacts.sh @@ -7,6 +7,6 @@ mkdir -p "$1" # in case ci/lib.sh decides to quit early . ${0%/*}/lib.sh -make artifacts-tar ARTIFACTS_DIRECTORY="$1" +group Build make artifacts-tar ARTIFACTS_DIRECTORY="$1" check_unignored_build_artifacts -- cgit v1.3 From cadcafc3311de7f6fae5f3add10cde4f93268ff8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 13 Jun 2022 13:13:08 +0000 Subject: ci(github): also mark up compile errors When GCC produces those helpful errors, we will want to present them in the GitHub workflow runs in the most helpful manner. To that end, we want to use workflow commands to render errors and warnings: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions In the previous commit, we ensured that grouping is used for the build in all jobs, and this allows us to piggy-back onto the `group` function to transmogrify the output. Note: If `set -o pipefail` was available, we could do this in a little more elegant way. But since some of the steps are run using `dash`, we have to do a little `{ ...; echo $? >exit.status; } | ...` dance. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- ci/lib.sh | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ci/lib.sh b/ci/lib.sh index b142d254ec..f095519f8d 100755 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -29,8 +29,14 @@ else set +x begin_group "$1" shift - "$@" - res=$? + # work around `dash` not supporting `set -o pipefail` + ( + "$@" 2>&1 + echo $? >exit.status + ) | + sed 's/^\(\([^ ]*\):\([0-9]*\):\([0-9]*:\) \)\(error\|warning\): /::\5 file=\2,line=\3::\1/' + res=$(cat exit.status) + rm exit.status end_group return $res } -- cgit v1.3 From 4a169da280aa6d22bdf0cf5baea65f47bd363a3a Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 15 Jun 2022 12:44:11 +0200 Subject: fetch doc: note "pushurl" caveat about "credentialsInUrl", elaborate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Amend the documentation and release notes entry for the "fetch.credentialsInUrl" feature added in 6dcbdc0d661 (remote: create fetch.credentialsInUrl config, 2022-06-06), it currently doesn't detect passwords in `remote..pushurl` configuration. We shouldn't lull users into a false sense of security, so we need to mention that prominently. This also elaborates and clarifies the "exposes the password in multiple ways" part of the documentation. As noted in [1] a user unfamiliar with git's implementation won't know what to make of that scary claim, e.g. git hypothetically have novel git-specific ways of exposing configured credentials. The reality is that this configuration is intended as an aid for users who can't fully trust their OS's or system's security model, so lets say that's what this is intended for, and mention the most common ways passwords stored in configuration might inadvertently get exposed. 1. https://lore.kernel.org/git/220524.86ilpuvcqh.gmgdl@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.37.0.txt | 4 +++- Documentation/config/fetch.txt | 34 ++++++++++++++++++++++++++++------ 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/Documentation/RelNotes/2.37.0.txt b/Documentation/RelNotes/2.37.0.txt index 8f1ff3a596..39ca3606de 100644 --- a/Documentation/RelNotes/2.37.0.txt +++ b/Documentation/RelNotes/2.37.0.txt @@ -55,7 +55,9 @@ UI, Workflows & Features * Update the doctype written in gitweb output to xhtml5. * The "fetch.credentialsInUrl" configuration variable controls what - happens when a URL with embedded login credential is used. + happens when a URL with embedded login credential is used on either + "fetch" or "push". Credentials are currently only detected in + `remote..url` config, not `remote..pushurl`. Performance, Internal Implementation, Development Support etc. diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt index 0db7fe85bb..827961059f 100644 --- a/Documentation/config/fetch.txt +++ b/Documentation/config/fetch.txt @@ -98,12 +98,34 @@ fetch.writeCommitGraph:: `git push -f`, and `git log --graph`. Defaults to false. fetch.credentialsInUrl:: - A URL can contain plaintext credentials in the form - `://:@/`. Using such URLs - is not recommended as it exposes the password in multiple ways, - including Git storing the URL as plaintext in the repository config. - The `fetch.credentialsInUrl` option provides instruction for how Git - should react to seeing such a URL, with these values: + A configured URL can contain plaintext credentials in the form + `://:@/`. You may want + to warn or forbid the use of such configuration (in favor of + using linkgit:git-credential[1]). ++ +Note that this is currently limited to detecting credentials in +`remote..url` configuration, it won't detect credentials in +`remote..pushurl` configuration. ++ +You might want to enable this to prevent inadvertent credentials +exposure, e.g. because: ++ +* The OS or system where you're running git may not provide way way or + otherwise allow you to configure the permissions of the + configuration file where the username and/or password are stored. +* Even if it does, having such data stored "at rest" might expose you + in other ways, e.g. a backup process might copy the data to another + system. +* The git programs will pass the full URL to one another as arguments + on the command-line, meaning the credentials will be exposed to oher + users on OS's or systems that allow other users to see the full + process list of other users. On linux the "hidepid" setting + documented in procfs(5) allows for configuring this behavior. ++ +If such concerns don't apply to you then you probably don't need to be +concerned about credentials exposure due to storing that sensitive +data in git's configuration files. If you do want to use this, set +`fetch.credentialsInUrl` to one of these values: + * `allow` (default): Git will proceed with its activity without warning. * `warn`: Git will write a warning message to `stderr` when parsing a URL -- cgit v1.3 From 7281c196b1166f1c00df33948c67b0ef81ba63a9 Mon Sep 17 00:00:00 2001 From: Ævar Arnfjörð Bjarmason Date: Wed, 15 Jun 2022 12:44:12 +0200 Subject: transfer doc: move fetch.credentialsInUrl to "transfer" config namespace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rename the "fetch.credentialsInUrl" configuration variable introduced in 6dcbdc0d661 (remote: create fetch.credentialsInUrl config, 2022-06-06) to "transfer". There are existing exceptions, but generally speaking the "." configuration should only apply to command described in the "namespace" (and its sub-commands, so e.g. "clone.*" or "fetch.*" might also configure "git-remote-https"). But in the case of "fetch.credentialsInUrl" we've got a configuration variable that configures the behavior of all of "clone", "push" and "fetch", someone adjusting "fetch.*" configuration won't expect to have the behavior of "git push" altered, especially as we have the pre-existing "{transfer,fetch,receive}.fsckObjects", which configures different parts of the transfer dialog. So let's move this configuration variable to the "transfer" namespace before it's exposed in a release. We could add all of "{transfer,fetch,pull}.credentialsInUrl" at some other time, but once we have "fetch" configure "pull" such an arrangement would would be a confusing mess, as we'd at least need to have "fetch" configure "push" (but not the other way around), or change existing behavior. Signed-off-by: Ævar Arnfjörð Bjarmason Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.37.0.txt | 2 +- Documentation/config/fetch.txt | 36 ------------------------------------ Documentation/config/transfer.txt | 38 ++++++++++++++++++++++++++++++++++++++ remote.c | 4 ++-- t/t5516-fetch-push.sh | 14 +++++++------- t/t5601-clone.sh | 10 +++++----- 6 files changed, 53 insertions(+), 51 deletions(-) diff --git a/Documentation/RelNotes/2.37.0.txt b/Documentation/RelNotes/2.37.0.txt index 39ca3606de..9902a74f34 100644 --- a/Documentation/RelNotes/2.37.0.txt +++ b/Documentation/RelNotes/2.37.0.txt @@ -54,7 +54,7 @@ UI, Workflows & Features * Update the doctype written in gitweb output to xhtml5. - * The "fetch.credentialsInUrl" configuration variable controls what + * The "transfer.credentialsInUrl" configuration variable controls what happens when a URL with embedded login credential is used on either "fetch" or "push". Credentials are currently only detected in `remote..url` config, not `remote..pushurl`. diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt index 827961059f..cd65d236b4 100644 --- a/Documentation/config/fetch.txt +++ b/Documentation/config/fetch.txt @@ -96,39 +96,3 @@ fetch.writeCommitGraph:: merge and the write may take longer. Having an updated commit-graph file helps performance of many Git commands, including `git merge-base`, `git push -f`, and `git log --graph`. Defaults to false. - -fetch.credentialsInUrl:: - A configured URL can contain plaintext credentials in the form - `://:@/`. You may want - to warn or forbid the use of such configuration (in favor of - using linkgit:git-credential[1]). -+ -Note that this is currently limited to detecting credentials in -`remote..url` configuration, it won't detect credentials in -`remote..pushurl` configuration. -+ -You might want to enable this to prevent inadvertent credentials -exposure, e.g. because: -+ -* The OS or system where you're running git may not provide way way or - otherwise allow you to configure the permissions of the - configuration file where the username and/or password are stored. -* Even if it does, having such data stored "at rest" might expose you - in other ways, e.g. a backup process might copy the data to another - system. -* The git programs will pass the full URL to one another as arguments - on the command-line, meaning the credentials will be exposed to oher - users on OS's or systems that allow other users to see the full - process list of other users. On linux the "hidepid" setting - documented in procfs(5) allows for configuring this behavior. -+ -If such concerns don't apply to you then you probably don't need to be -concerned about credentials exposure due to storing that sensitive -data in git's configuration files. If you do want to use this, set -`fetch.credentialsInUrl` to one of these values: -+ -* `allow` (default): Git will proceed with its activity without warning. -* `warn`: Git will write a warning message to `stderr` when parsing a URL - with a plaintext credential. -* `die`: Git will write a failure message to `stderr` when parsing a URL - with a plaintext credential. diff --git a/Documentation/config/transfer.txt b/Documentation/config/transfer.txt index b49429eb4d..b4475c0690 100644 --- a/Documentation/config/transfer.txt +++ b/Documentation/config/transfer.txt @@ -1,3 +1,41 @@ +transfer.credentialsInUrl:: + A configured URL can contain plaintext credentials in the form + `://:@/`. You may want + to warn or forbid the use of such configuration (in favor of + using linkgit:git-credential[1]). This will be used on + linkgit:git-clone[1], linkgit:git-fetch[1], linkgit:git-push[1], + and any other direct use of the configured URL. ++ +Note that this is currently limited to detecting credentials in +`remote..url` configuration, it won't detect credentials in +`remote..pushurl` configuration. ++ +You might want to enable this to prevent inadvertent credentials +exposure, e.g. because: ++ +* The OS or system where you're running git may not provide way way or + otherwise allow you to configure the permissions of the + configuration file where the username and/or password are stored. +* Even if it does, having such data stored "at rest" might expose you + in other ways, e.g. a backup process might copy the data to another + system. +* The git programs will pass the full URL to one another as arguments + on the command-line, meaning the credentials will be exposed to oher + users on OS's or systems that allow other users to see the full + process list of other users. On linux the "hidepid" setting + documented in procfs(5) allows for configuring this behavior. ++ +If such concerns don't apply to you then you probably don't need to be +concerned about credentials exposure due to storing that sensitive +data in git's configuration files. If you do want to use this, set +`transfer.credentialsInUrl` to one of these values: ++ +* `allow` (default): Git will proceed with its activity without warning. +* `warn`: Git will write a warning message to `stderr` when parsing a URL + with a plaintext credential. +* `die`: Git will write a failure message to `stderr` when parsing a URL + with a plaintext credential. + transfer.fsckObjects:: When `fetch.fsckObjects` or `receive.fsckObjects` are not set, the value of this variable is used instead. diff --git a/remote.c b/remote.c index 9b9bbfe80e..42c891d44f 100644 --- a/remote.c +++ b/remote.c @@ -623,7 +623,7 @@ static void validate_remote_url(struct remote *remote) struct strbuf redacted = STRBUF_INIT; int warn_not_die; - if (git_config_get_string_tmp("fetch.credentialsinurl", &value)) + if (git_config_get_string_tmp("transfer.credentialsinurl", &value)) return; if (!strcmp("warn", value)) @@ -633,7 +633,7 @@ static void validate_remote_url(struct remote *remote) else if (!strcmp("allow", value)) return; else - die(_("unrecognized value fetch.credentialsInURL: '%s'"), value); + die(_("unrecognized value transfer.credentialsInURL: '%s'"), value); for (i = 0; i < remote->url_nr; i++) { struct url_info url_info = { 0 }; diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh index dedca106a7..79d8a7b367 100755 --- a/t/t5516-fetch-push.sh +++ b/t/t5516-fetch-push.sh @@ -1836,18 +1836,18 @@ test_expect_success 'refuse to push a hidden ref, and make sure do not pollute t test_expect_success 'fetch warns or fails when using username:password' ' message="URL '\''https://username:@localhost/'\'' uses plaintext credentials" && - test_must_fail git -c fetch.credentialsInUrl=allow fetch https://username:password@localhost 2>err && + test_must_fail git -c transfer.credentialsInUrl=allow fetch https://username:password@localhost 2>err && ! grep "$message" err && - test_must_fail git -c fetch.credentialsInUrl=warn fetch https://username:password@localhost 2>err && + test_must_fail git -c transfer.credentialsInUrl=warn fetch https://username:password@localhost 2>err && grep "warning: $message" err >warnings && test_line_count = 3 warnings && - test_must_fail git -c fetch.credentialsInUrl=die fetch https://username:password@localhost 2>err && + test_must_fail git -c transfer.credentialsInUrl=die fetch https://username:password@localhost 2>err && grep "fatal: $message" err >warnings && test_line_count = 1 warnings && - test_must_fail git -c fetch.credentialsInUrl=die fetch https://username:@localhost 2>err && + test_must_fail git -c transfer.credentialsInUrl=die fetch https://username:@localhost 2>err && grep "fatal: $message" err >warnings && test_line_count = 1 warnings ' @@ -1855,12 +1855,12 @@ test_expect_success 'fetch warns or fails when using username:password' ' test_expect_success 'push warns or fails when using username:password' ' message="URL '\''https://username:@localhost/'\'' uses plaintext credentials" && - test_must_fail git -c fetch.credentialsInUrl=allow push https://username:password@localhost 2>err && + test_must_fail git -c transfer.credentialsInUrl=allow push https://username:password@localhost 2>err && ! grep "$message" err && - test_must_fail git -c fetch.credentialsInUrl=warn push https://username:password@localhost 2>err && + test_must_fail git -c transfer.credentialsInUrl=warn push https://username:password@localhost 2>err && grep "warning: $message" err >warnings && - test_must_fail git -c fetch.credentialsInUrl=die push https://username:password@localhost 2>err && + test_must_fail git -c transfer.credentialsInUrl=die push https://username:password@localhost 2>err && grep "fatal: $message" err >warnings && test_line_count = 1 warnings ' diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh index d2f046b4b9..e6a248bbdc 100755 --- a/t/t5601-clone.sh +++ b/t/t5601-clone.sh @@ -73,24 +73,24 @@ test_expect_success 'clone respects GIT_WORK_TREE' ' test_expect_success 'clone warns or fails when using username:password' ' message="URL '\''https://username:@localhost/'\'' uses plaintext credentials" && - test_must_fail git -c fetch.credentialsInUrl=allow clone https://username:password@localhost attempt1 2>err && + test_must_fail git -c transfer.credentialsInUrl=allow clone https://username:password@localhost attempt1 2>err && ! grep "$message" err && - test_must_fail git -c fetch.credentialsInUrl=warn clone https://username:password@localhost attempt2 2>err && + test_must_fail git -c transfer.credentialsInUrl=warn clone https://username:password@localhost attempt2 2>err && grep "warning: $message" err >warnings && test_line_count = 2 warnings && - test_must_fail git -c fetch.credentialsInUrl=die clone https://username:password@localhost attempt3 2>err && + test_must_fail git -c transfer.credentialsInUrl=die clone https://username:password@localhost attempt3 2>err && grep "fatal: $message" err >warnings && test_line_count = 1 warnings && - test_must_fail git -c fetch.credentialsInUrl=die clone https://username:@localhost attempt3 2>err && + test_must_fail git -c transfer.credentialsInUrl=die clone https://username:@localhost attempt3 2>err && grep "fatal: $message" err >warnings && test_line_count = 1 warnings ' test_expect_success 'clone does not detect username:password when it is https://username@domain:port/' ' - test_must_fail git -c fetch.credentialsInUrl=warn clone https://username@localhost:8080 attempt3 2>err && + test_must_fail git -c transfer.credentialsInUrl=warn clone https://username@localhost:8080 attempt3 2>err && ! grep "uses plaintext credentials" err ' -- cgit v1.3 From 55d9d4bbd044afa004c6962aa50635158dc8719e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Jun 2022 03:09:32 -0400 Subject: perf-lib: fix missing test titles in output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 5dccd9155f (t/perf: add iteration setup mechanism to perf-lib, 2022-04-04) modified the parameter parsing of test_wrapper() such that the test title was no longer in $1, and is instead in $test_title_. We correctly pass the new variable to the code which outputs the title to the log, but missed the spot in test_wrapper() where the title is written to the ".descr" file which is used to produce the final output table. As a result, all of the titles are missing from that table (or worse, using whatever was left in $1): $ ./p0000-perf-lib-sanity.sh [...] Test this tree ------------------------------ 0000.1: 0.01(0.01+0.00) 0000.2: 0.01(0.00+0.01) 0000.4: 0.00(0.00+0.00) 0000.5: true 0.00(0.00+0.00) 0000.7: 0.00(0.00+0.00) 0000.8: 0.00(0.00+0.00) After this patch, we get the pre-5dccd9155f output: Test this tree -------------------------------------------------------------------------- 0000.1: test_perf_default_repo works 0.00(0.00+0.00) 0000.2: test_checkout_worktree works 0.01(0.00+0.01) 0000.4: export a weird var 0.00(0.00+0.00) 0000.5: éḿíẗ ńöń-ÁŚĆÍÍ ćḧáŕáćẗéŕś 0.00(0.00+0.00) 0000.7: important variables available in subshells 0.00(0.00+0.00) 0000.8: test-lib-functions correctly loaded in subshells 0.00(0.00+0.00) Signed-off-by: Jeff King Acked-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/perf/perf-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh index df5b1f1c37..850ca55b04 100644 --- a/t/perf/perf-lib.sh +++ b/t/perf/perf-lib.sh @@ -219,7 +219,7 @@ test_wrapper_ () { then base=$(basename "$0" .sh) echo "$test_count" >>"$perf_results_dir"/$base.subtests - echo "$1" >"$perf_results_dir"/$base.$test_count.descr + echo "$test_title_" >"$perf_results_dir"/$base.$test_count.descr base="$perf_results_dir"/"$PERF_RESULTS_PREFIX$(basename "$0" .sh)"."$test_count" "$test_wrapper_func_" "$test_title_" "$@" fi -- cgit v1.3 From 624b8cfdcea32e4f023415097e611fb1566512fd Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 16 Jun 2022 13:13:47 +0000 Subject: t2107: test 'git update-index --verbose' The '--verbose' option reports what is being added and removed from the index, but has not been tested up to this point. Augment the tests in t2107 to check the '--verbose' option in some scenarios. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/t2107-update-index-basic.sh | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh index a30b7ca6bc..07e6de84e6 100755 --- a/t/t2107-update-index-basic.sh +++ b/t/t2107-update-index-basic.sh @@ -36,9 +36,14 @@ test_expect_success '--cacheinfo does not accept blob null sha1' ' echo content >file && git add file && git rev-parse :file >expect && - test_must_fail git update-index --cacheinfo 100644 $ZERO_OID file && + test_must_fail git update-index --verbose --cacheinfo 100644 $ZERO_OID file >out && git rev-parse :file >actual && - test_cmp expect actual + test_cmp expect actual && + + cat >expect <<-\EOF && + add '\''file'\'' + EOF + test_cmp expect out ' test_expect_success '--cacheinfo does not accept gitlink null sha1' ' @@ -59,9 +64,14 @@ test_expect_success '--cacheinfo mode,sha1,path (new syntax)' ' git rev-parse :file >actual && test_cmp expect actual && - git update-index --add --cacheinfo "100644,$(cat expect),elif" && + git update-index --add --verbose --cacheinfo "100644,$(cat expect),elif" >out && git rev-parse :elif >actual && - test_cmp expect actual + test_cmp expect actual && + + cat >expect <<-\EOF && + add '\''elif'\'' + EOF + test_cmp expect out ' test_expect_success '.lock files cleaned up' ' @@ -74,7 +84,8 @@ test_expect_success '.lock files cleaned up' ' git config core.worktree ../../worktree && # --refresh triggers late setup_work_tree, # active_cache_changed is zero, rollback_lock_file fails - git update-index --refresh && + git update-index --refresh --verbose >out && + test_must_be_empty out && ! test -f .git/index.lock ) ' @@ -83,7 +94,15 @@ test_expect_success '--chmod=+x and chmod=-x in the same argument list' ' >A && >B && git add A B && - git update-index --chmod=+x A --chmod=-x B && + git update-index --verbose --chmod=+x A --chmod=-x B >out && + cat >expect <<-\EOF && + add '\''A'\'' + chmod +x '\''A'\'' + add '\''B'\'' + chmod -x '\''B'\'' + EOF + test_cmp expect out && + cat >expect <<-EOF && 100755 $EMPTY_BLOB 0 A 100644 $EMPTY_BLOB 0 B -- cgit v1.3 From 9aa1cba01a90a82dc798cc6d2f18074335f24d2e Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 16 Jun 2022 13:13:48 +0000 Subject: t5329: test 'git gc --cruft' without '--prune=now' Replace a 'git repack --cruft -d' with the wrapper 'git gc --cruft' to exercise some logic in builtin/gc.c that adds the '--cruft' option to the underlying 'git repack' command. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- t/t5329-pack-objects-cruft.sh | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/t/t5329-pack-objects-cruft.sh b/t/t5329-pack-objects-cruft.sh index b481224b93..8968f7a08d 100755 --- a/t/t5329-pack-objects-cruft.sh +++ b/t/t5329-pack-objects-cruft.sh @@ -451,11 +451,13 @@ test_expect_success 'expiring cruft objects with git gc' ' sort reachable && comm -13 reachable objects >unreachable && - git repack --cruft -d && + # Write a cruft pack containing all unreachable objects. + git gc --cruft --prune="01-01-1980" && mtimes=$(ls .git/objects/pack/pack-*.mtimes) && test_path_is_file $mtimes && + # Prune all unreachable objects from the cruft pack. git gc --cruft --prune=now && git cat-file --batch-all-objects --batch-check="%(objectname)" >objects && -- cgit v1.3 From 82db195e1be63cfa274c26351ef782b2df0a21fd Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 16 Jun 2022 13:13:49 +0000 Subject: pack-write: drop always-NULL parameter write_mtimes_file() takes an mtimes parameter as its first option, but the only caller passes a NULL constant. Drop this parameter to simplify logic. This can be reverted if that parameter is needed in the future. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- pack-write.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/pack-write.c b/pack-write.c index 23c0342018..00787e306d 100644 --- a/pack-write.c +++ b/pack-write.c @@ -310,26 +310,21 @@ static void write_mtimes_trailer(struct hashfile *f, const unsigned char *hash) hashwrite(f, hash, the_hash_algo->rawsz); } -static const char *write_mtimes_file(const char *mtimes_name, - struct packing_data *to_pack, +static const char *write_mtimes_file(struct packing_data *to_pack, struct pack_idx_entry **objects, uint32_t nr_objects, const unsigned char *hash) { + struct strbuf tmp_file = STRBUF_INIT; + const char *mtimes_name; struct hashfile *f; int fd; if (!to_pack) BUG("cannot call write_mtimes_file with NULL packing_data"); - if (!mtimes_name) { - struct strbuf tmp_file = STRBUF_INIT; - fd = odb_mkstemp(&tmp_file, "pack/tmp_mtimes_XXXXXX"); - mtimes_name = strbuf_detach(&tmp_file, NULL); - } else { - unlink(mtimes_name); - fd = xopen(mtimes_name, O_CREAT|O_EXCL|O_WRONLY, 0600); - } + fd = odb_mkstemp(&tmp_file, "pack/tmp_mtimes_XXXXXX"); + mtimes_name = strbuf_detach(&tmp_file, NULL); f = hashfd(fd, mtimes_name); write_mtimes_header(f); @@ -561,7 +556,7 @@ void stage_tmp_packfiles(struct strbuf *name_buffer, pack_idx_opts->flags); if (pack_idx_opts->flags & WRITE_MTIMES) { - mtimes_tmp_name = write_mtimes_file(NULL, to_pack, written_list, + mtimes_tmp_name = write_mtimes_file(to_pack, written_list, nr_written, hash); } -- cgit v1.3 From 86aa250aa871a4d8314f1f51ee007e93a9461e1e Mon Sep 17 00:00:00 2001 From: Derrick Stolee Date: Thu, 16 Jun 2022 13:13:50 +0000 Subject: cache-tree: remove cache_tree_find_path() This reverts 080ab56a46 (cache-tree: implement cache_tree_find_path(), 2022-05-23). The cache_tree_find_path() method was never actually called in the topic that added it. I cannot find any reference to it in any of my forks, so this appears to not be needed at the moment. Signed-off-by: Derrick Stolee Signed-off-by: Junio C Hamano --- cache-tree.c | 27 --------------------------- cache-tree.h | 2 -- 2 files changed, 29 deletions(-) diff --git a/cache-tree.c b/cache-tree.c index ff794d940f..56db0b5026 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -101,33 +101,6 @@ struct cache_tree_sub *cache_tree_sub(struct cache_tree *it, const char *path) return find_subtree(it, path, pathlen, 1); } -struct cache_tree *cache_tree_find_path(struct cache_tree *it, const char *path) -{ - const char *slash; - int namelen; - struct cache_tree_sub it_sub = { - .cache_tree = it, - }; - struct cache_tree_sub *down = &it_sub; - - while (down) { - slash = strchrnul(path, '/'); - namelen = slash - path; - down->cache_tree->entry_count = -1; - if (!*slash) { - int pos; - pos = cache_tree_subtree_pos(down->cache_tree, path, namelen); - if (0 <= pos) - return down->cache_tree->down[pos]->cache_tree; - return NULL; - } - down = find_subtree(it, path, namelen, 0); - path = slash + 1; - } - - return NULL; -} - static int do_invalidate_path(struct cache_tree *it, const char *path) { /* a/b/c diff --git a/cache-tree.h b/cache-tree.h index f75f8e74dc..8efeccebfc 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -29,8 +29,6 @@ struct cache_tree_sub *cache_tree_sub(struct cache_tree *, const char *); int cache_tree_subtree_pos(struct cache_tree *it, const char *path, int pathlen); -struct cache_tree *cache_tree_find_path(struct cache_tree *it, const char *path); - void cache_tree_write(struct strbuf *, struct cache_tree *root); struct cache_tree *cache_tree_read(const char *buffer, unsigned long size); -- cgit v1.3 From 5a09991e32f3487702bd032703bacba1c4c46612 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:36 +0000 Subject: fsmonitor: avoid memory leak in `fsm_settings__get_incompatible_msg()` Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- fsmonitor-settings.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fsmonitor-settings.c b/fsmonitor-settings.c index 658cb79da0..464424a1e9 100644 --- a/fsmonitor-settings.c +++ b/fsmonitor-settings.c @@ -202,11 +202,15 @@ char *fsm_settings__get_incompatible_msg(const struct repository *r, case FSMONITOR_REASON_OK: goto done; - case FSMONITOR_REASON_BARE: + case FSMONITOR_REASON_BARE: { + char *cwd = xgetcwd(); + strbuf_addf(&msg, _("bare repository '%s' is incompatible with fsmonitor"), - xgetcwd()); + cwd); + free(cwd); goto done; + } case FSMONITOR_REASON_ERROR: strbuf_addf(&msg, -- cgit v1.3 From f53559227ccb600f4fdd1bfe08e1164a5aed60b5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:39 +0000 Subject: submodule-config: avoid memory leak In 961b130d20c9 (branch: add --recurse-submodules option for branch creation, 2022-01-28), a funny pattern was introduced where first some struct is `xmalloc()`ed, then we resize an array whose element type is the same struct, and then the first struct's contents are copied into the last element of that array. Crucially, the `xmalloc()`ed memory never gets released. Let's avoid that memory leak and that memory allocation dance altogether by first reallocating the array, then using a pointer to the last array element to go forward. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- submodule-config.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/submodule-config.c b/submodule-config.c index ce3beaf5d4..51ecbe901e 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -756,7 +756,10 @@ static void traverse_tree_submodules(struct repository *r, if (S_ISGITLINK(name_entry->mode) && is_tree_submodule_active(r, root_tree, tree_path)) { - st_entry = xmalloc(sizeof(*st_entry)); + ALLOC_GROW(out->entries, out->entry_nr + 1, + out->entry_alloc); + st_entry = &out->entries[out->entry_nr++]; + st_entry->name_entry = xmalloc(sizeof(*st_entry->name_entry)); *st_entry->name_entry = *name_entry; st_entry->submodule = @@ -766,9 +769,6 @@ static void traverse_tree_submodules(struct repository *r, root_tree)) FREE_AND_NULL(st_entry->repo); - ALLOC_GROW(out->entries, out->entry_nr + 1, - out->entry_alloc); - out->entries[out->entry_nr++] = *st_entry; } else if (S_ISDIR(name_entry->mode)) traverse_tree_submodules(r, root_tree, tree_path, &name_entry->oid, out); -- cgit v1.3 From 41a86b64c093a6f36ffe0959aeed2e3f2590c7e8 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:41 +0000 Subject: submodule--helper: avoid memory leak when fetching submodules In c51f8f94e5b3 (submodule--helper: run update procedures from C, 2021-08-24), we added code that first obtains the default remote, and then adds that to a `strvec`. However, we never released the default remote's memory. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 5c77dfcffe..c597df7528 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -2208,6 +2208,7 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, str char *hex = oid_to_hex(oid); char *remote = get_default_remote(); strvec_pushl(&cp.args, remote, hex, NULL); + free(remote); } return run_command(&cp); -- cgit v1.3 From 652891de4ff164d545daa5472ab67f4f9d41319b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:42 +0000 Subject: read_index_from(): avoid memory leak In 998330ac2e7c (read-cache: look for shared index files next to the index, too, 2021-08-26), we added code that allocates memory to store the base path of a shared index, but we never released that memory. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- read-cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/read-cache.c b/read-cache.c index e61af3a3d4..76f372ff91 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2473,15 +2473,15 @@ int read_index_from(struct index_state *istate, const char *path, the_repository, "%s", base_path); if (!ret) { char *path_copy = xstrdup(path); - const char *base_path2 = xstrfmt("%s/sharedindex.%s", - dirname(path_copy), - base_oid_hex); + char *base_path2 = xstrfmt("%s/sharedindex.%s", + dirname(path_copy), base_oid_hex); free(path_copy); trace2_region_enter_printf("index", "shared/do_read_index", the_repository, "%s", base_path2); ret = do_read_index(split_index->base, base_path2, 1); trace2_region_leave_printf("index", "shared/do_read_index", the_repository, "%s", base_path2); + free(base_path2); } if (!oideq(&split_index->base_oid, &split_index->base->oid)) die(_("broken index, expect %s in %s, got %s"), -- cgit v1.3 From 41f1a8e6a417bc3e56a0eef687e28247138276d1 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:43 +0000 Subject: pack-mtimes: avoid closing a bogus file descriptor In 94cd775a6c52 (pack-mtimes: support reading .mtimes files, 2022-05-20), code was added to close the file descriptor corresponding to the mtimes file. However, it is possible that opening that file failed, in which case we are closing a file descriptor with the value `-1`. Let's guard that `close()` call. Reported by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- pack-mtimes.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pack-mtimes.c b/pack-mtimes.c index 0e0aafdcb0..0f9785fc5e 100644 --- a/pack-mtimes.c +++ b/pack-mtimes.c @@ -89,7 +89,8 @@ cleanup: *data_p = data; } - close(fd); + if (fd >= 0) + close(fd); return ret; } -- cgit v1.3 From c918f5c1ab0c4dec916b747916236ca0d3655be5 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Wed, 15 Jun 2022 23:35:44 +0000 Subject: relative_url(): fix incorrect condition In 63e95beb085c (submodule: port resolve_relative_url from shell to C, 2016-04-15), we added a loop over `url` where we are looking for `../` or `./` components. The loop condition we used is the pointer `url` itself, which is clearly not what we wanted. Pointed out by Coverity. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- remote.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/remote.c b/remote.c index 9b9bbfe80e..bded6acbfe 100644 --- a/remote.c +++ b/remote.c @@ -2846,7 +2846,7 @@ char *relative_url(const char *remote_url, const char *url, * When the url starts with '../', remove that and the * last directory in remoteurl. */ - while (url) { + while (*url) { if (starts_with_dot_dot_slash_native(url)) { url += 3; colonsep |= chop_last_dir(&remoteurl, is_relative); -- cgit v1.3 From f8535596aa7bd7f6862af3fe6420ac12b17c9912 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Jun 2022 16:04:25 -0400 Subject: bug_fl(): correctly initialize trace2 va_list The code added 0cc05b044f (usage.c: add a non-fatal bug() function to go with BUG(), 2022-06-02) sets up two va_list variables: one to output to stderr, and one to trace2. But the order of initialization is wrong: va_list ap, cp; va_copy(cp, ap); va_start(ap, fmt); We copy the contents of "ap" into "cp" before it is initialized, meaning it is full of garbage. The two should be swapped. However, there's another bug, noticed by Johannes Schindelin: we forget to call va_end() for the copy. So instead of just fixing the copy's initialization, let's do two separate start/end pairs. This is allowed by the standard, and we don't need to use copy here since we have access to the original varargs. Matching the pairs with the calls makes it more obvious that everything is being done correctly. Note that we do call bug_fl() in the tests, but it didn't trigger this problem because our format string doesn't have any placeholders. So even though we were passing a garbage va_list through the stack, nobody ever needed to look at it. We can easily adjust one of the trace2 tests to trigger this, both for bug() and for BUG(). The latter isn't broken, but it's nice to exercise both a bit more. Without the fix in this patch (but with the test change), the bug() case causes a segfault. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- t/helper/test-trace2.c | 4 ++-- usage.c | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/t/helper/test-trace2.c b/t/helper/test-trace2.c index 180c7f53f3..a714130ece 100644 --- a/t/helper/test-trace2.c +++ b/t/helper/test-trace2.c @@ -224,8 +224,8 @@ static int ut_009bug_BUG(int argc, const char **argv) static int ut_010bug_BUG(int argc, const char **argv) { - bug("a bug message"); - BUG("a BUG message"); + bug("a %s message", "bug"); + BUG("a %s message", "BUG"); } /* diff --git a/usage.c b/usage.c index 79900d0287..56e29d6cd6 100644 --- a/usage.c +++ b/usage.c @@ -334,15 +334,17 @@ NORETURN void BUG_fl(const char *file, int line, const char *fmt, ...) int bug_called_must_BUG; void bug_fl(const char *file, int line, const char *fmt, ...) { - va_list ap, cp; + va_list ap; bug_called_must_BUG = 1; - va_copy(cp, ap); va_start(ap, fmt); BUG_vfl_common(file, line, fmt, ap); va_end(ap); - trace2_cmd_error_va(fmt, cp); + + va_start(ap, fmt); + trace2_cmd_error_va(fmt, ap); + va_end(ap); } #ifdef SUPPRESS_ANNOTATED_LEAKS -- cgit v1.3 From b81b98f818fdacdc472f2afed2ae67d9d0893fe2 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 17 Jun 2022 10:33:42 -0700 Subject: Another batch of fixes before -rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.37.0.txt | 5 ++++- GIT-VERSION-GEN | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Documentation/RelNotes/2.37.0.txt b/Documentation/RelNotes/2.37.0.txt index bf00ccc437..f1b93f3c59 100644 --- a/Documentation/RelNotes/2.37.0.txt +++ b/Documentation/RelNotes/2.37.0.txt @@ -54,7 +54,7 @@ UI, Workflows & Features * Update the doctype written in gitweb output to xhtml5. - * The "transfer.credentialsInUrl" configuration variable controls what + * The "transfer.credentialsInURL" configuration variable controls what happens when a URL with embedded login credential is used on either "fetch" or "push". Credentials are currently only detected in `remote..url` config, not `remote..pushurl`. @@ -309,6 +309,9 @@ Fixes since v2.36 * Use-after-free (with another forget-to-free) fix. (merge 323822c72b ab/remote-free-fix later to maint). + * Remove a coccinelle rule that is no longer relevant. + (merge b1299de4a1 jc/cocci-cleanup later to maint). + * Other code cleanup, docfix, build fix, etc. (merge e6b2582da3 cm/reftable-0-length-memset later to maint). (merge 0b75e5bf22 ab/misc-cleanup later to maint). diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index 1d667699e1..c0b5e722dd 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.36.GIT +DEF_VER=v2.37.0-rc1 ;# not quite LF=' ' -- cgit v1.3 From b4eda05d58ca3e4808d3d86ab5826c77995a06f7 Mon Sep 17 00:00:00 2001 From: Jiang Xin Date: Fri, 17 Jun 2022 18:03:09 +0800 Subject: i18n: fix mismatched camelCase config variables Some config variables are combinations of multiple words, and we typically write them in camelCase forms in manpage and translatable strings. It's not easy to find mismatches for these camelCase config variables during code reviews, but occasionally they are identified during localization translations. To check for mismatched config variables, I introduced a new feature in the helper program for localization[^1]. The following mismatched config variables have been identified by running the helper program, such as "git-po-helper check-pot". Lowercase in manpage should use camelCase: * Documentation/config/http.txt: http.pinnedpubkey Lowercase in translable strings should use camelCase: * builtin/fast-import.c: pack.indexversion * builtin/gc.c: gc.logexpiry * builtin/index-pack.c: pack.indexversion * builtin/pack-objects.c: pack.indexversion * builtin/repack.c: pack.writebitmaps * commit.c: i18n.commitencoding * gpg-interface.c: user.signingkey * http.c: http.postbuffer * submodule-config.c: submodule.fetchjobs Mismatched camelCases, choose the former: * Documentation/config/transfer.txt: transfer.credentialsInUrl remote.c: transfer.credentialsInURL [^1]: https://github.com/git-l10n/git-po-helper Signed-off-by: Jiang Xin Signed-off-by: Junio C Hamano --- Documentation/config/http.txt | 2 +- builtin/fast-import.c | 2 +- builtin/gc.c | 2 +- builtin/index-pack.c | 2 +- builtin/pack-objects.c | 2 +- builtin/repack.c | 2 +- commit.c | 2 +- gpg-interface.c | 2 +- http.c | 2 +- remote.c | 2 +- submodule-config.c | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/Documentation/config/http.txt b/Documentation/config/http.txt index 179d03e57b..afeeccfbfa 100644 --- a/Documentation/config/http.txt +++ b/Documentation/config/http.txt @@ -203,7 +203,7 @@ http.schannelUseSSLCAInfo:: when the `schannel` backend was configured via `http.sslBackend`, unless `http.schannelUseSSLCAInfo` overrides this behavior. -http.pinnedpubkey:: +http.pinnedPubkey:: Public key of the https service. It may either be the filename of a PEM or DER encoded public key file or a string starting with 'sha256//' followed by the base64 encoded sha256 hash of the diff --git a/builtin/fast-import.c b/builtin/fast-import.c index 28d3193c38..14113cfd82 100644 --- a/builtin/fast-import.c +++ b/builtin/fast-import.c @@ -3465,7 +3465,7 @@ static void git_pack_config(void) pack_idx_opts.version = indexversion_value; if (pack_idx_opts.version > 2) git_die_config("pack.indexversion", - "bad pack.indexversion=%"PRIu32, pack_idx_opts.version); + "bad pack.indexVersion=%"PRIu32, pack_idx_opts.version); } if (!git_config_get_ulong("pack.packsizelimit", &packsizelimit_value)) max_packsize = packsizelimit_value; diff --git a/builtin/gc.c b/builtin/gc.c index 4ea70089c9..021e9256ae 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -581,7 +581,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) /* default expiry time, overwritten in gc_config */ gc_config(); if (parse_expiry_date(gc_log_expire, &gc_log_expire_time)) - die(_("failed to parse gc.logexpiry value %s"), gc_log_expire); + die(_("failed to parse gc.logExpiry value %s"), gc_log_expire); if (pack_refs < 0) pack_refs = !is_bare_repository(); diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 3e385b4800..6648f2daef 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1575,7 +1575,7 @@ static int git_index_pack_config(const char *k, const char *v, void *cb) if (!strcmp(k, "pack.indexversion")) { opts->version = git_config_int(k, v); if (opts->version > 2) - die(_("bad pack.indexversion=%"PRIu32), opts->version); + die(_("bad pack.indexVersion=%"PRIu32), opts->version); return 0; } if (!strcmp(k, "pack.threads")) { diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index cc5f41086d..39e28cfcaf 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -3170,7 +3170,7 @@ static int git_pack_config(const char *k, const char *v, void *cb) if (!strcmp(k, "pack.indexversion")) { pack_idx_opts.version = git_config_int(k, v); if (pack_idx_opts.version > 2) - die(_("bad pack.indexversion=%"PRIu32), + die(_("bad pack.indexVersion=%"PRIu32), pack_idx_opts.version); return 0; } diff --git a/builtin/repack.c b/builtin/repack.c index c957b2959f..4a7ae4cf48 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -41,7 +41,7 @@ static const char *const git_repack_usage[] = { static const char incremental_bitmap_conflict_error[] = N_( "Incremental repacks are incompatible with bitmap indexes. Use\n" -"--no-write-bitmap-index or disable the pack.writebitmaps configuration." +"--no-write-bitmap-index or disable the pack.writeBitmaps configuration." ); struct pack_objects_args { diff --git a/commit.c b/commit.c index 73865fee15..1fb1b2ea90 100644 --- a/commit.c +++ b/commit.c @@ -1526,7 +1526,7 @@ static int verify_utf8(struct strbuf *buf) static const char commit_utf8_warn[] = N_("Warning: commit message did not conform to UTF-8.\n" "You may want to amend it after fixing the message, or set the config\n" - "variable i18n.commitencoding to the encoding your project uses.\n"); + "variable i18n.commitEncoding to the encoding your project uses.\n"); int commit_tree_extended(const char *msg, size_t msg_len, const struct object_id *tree, diff --git a/gpg-interface.c b/gpg-interface.c index 280f1fa1a5..947b58ad4d 100644 --- a/gpg-interface.c +++ b/gpg-interface.c @@ -985,7 +985,7 @@ static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature, if (!signing_key || signing_key[0] == '\0') return error( - _("user.signingkey needs to be set for ssh signing")); + _("user.signingKey needs to be set for ssh signing")); if (is_literal_ssh_key(signing_key, &literal_key)) { /* A literal ssh key */ diff --git a/http.c b/http.c index 11c6f69fac..168ca30c55 100644 --- a/http.c +++ b/http.c @@ -349,7 +349,7 @@ static int http_options(const char *var, const char *value, void *cb) if (!strcmp("http.postbuffer", var)) { http_post_buffer = git_config_ssize_t(var, value); if (http_post_buffer < 0) - warning(_("negative value for http.postbuffer; defaulting to %d"), LARGE_PACKET_MAX); + warning(_("negative value for http.postBuffer; defaulting to %d"), LARGE_PACKET_MAX); if (http_post_buffer < LARGE_PACKET_MAX) http_post_buffer = LARGE_PACKET_MAX; return 0; diff --git a/remote.c b/remote.c index 549ba5862a..b19e3a2f01 100644 --- a/remote.c +++ b/remote.c @@ -633,7 +633,7 @@ static void validate_remote_url(struct remote *remote) else if (!strcmp("allow", value)) return; else - die(_("unrecognized value transfer.credentialsInURL: '%s'"), value); + die(_("unrecognized value transfer.credentialsInUrl: '%s'"), value); for (i = 0; i < remote->url_nr; i++) { struct url_info url_info = { 0 }; diff --git a/submodule-config.c b/submodule-config.c index 51ecbe901e..c2ac7e7bf3 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -302,7 +302,7 @@ int parse_submodule_fetchjobs(const char *var, const char *value) { int fetchjobs = git_config_int(var, value); if (fetchjobs < 0) - die(_("negative values not allowed for submodule.fetchjobs")); + die(_("negative values not allowed for submodule.fetchJobs")); return fetchjobs; } -- cgit v1.3 From 6b11e3d52e919cce91011f4f9025e6f4b61375f2 Mon Sep 17 00:00:00 2001 From: Carlo Marcelo Arenas Belón Date: Fri, 17 Jun 2022 13:23:38 -0700 Subject: git-compat-util: allow root to access both SUDO_UID and root owned MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous changes introduced a regression which will prevent root for accessing repositories owned by thyself if using sudo because SUDO_UID takes precedence. Loosen that restriction by allowing root to access repositories owned by both uid by default and without having to add a safe.directory exception. A previous workaround that was documented in the tests is no longer needed so it has been removed together with its specially crafted prerequisite. Helped-by: Johanness Schindelin Signed-off-by: Carlo Marcelo Arenas Belón Signed-off-by: Junio C Hamano --- Documentation/config/safe.txt | 7 ++++--- git-compat-util.h | 7 ++++++- t/t0034-root-safe-directory.sh | 15 +-------------- 3 files changed, 11 insertions(+), 18 deletions(-) diff --git a/Documentation/config/safe.txt b/Documentation/config/safe.txt index c6ebd1674d..74627c5e7c 100644 --- a/Documentation/config/safe.txt +++ b/Documentation/config/safe.txt @@ -30,12 +30,13 @@ that you deem safe. As explained, Git only allows you to access repositories owned by yourself, i.e. the user who is running Git, by default. When Git is running as 'root' in a non Windows platform that provides sudo, - however, git checks the SUDO_UID environment variable that sudo creates -and will allow access to the uid recorded as its value instead. +however, git checks the SUDO_UID environment variable that sudo creates +and will allow access to the uid recorded as its value in addition to +the id from 'root'. This is to make it easy to perform a common sequence during installation "make && sudo make install". A git process running under 'sudo' runs as 'root' but the 'sudo' command exports the environment variable to record which id the original user has. If that is not what you would prefer and want git to only trust -repositories that are owned by root instead, then you must remove +repositories that are owned by root instead, then you can remove the `SUDO_UID` variable from root's environment before invoking git. diff --git a/git-compat-util.h b/git-compat-util.h index e7cbfa65c9..f505f817d5 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -447,7 +447,12 @@ static inline int is_path_owned_by_current_uid(const char *path) euid = geteuid(); if (euid == ROOT_UID) - extract_id_from_env("SUDO_UID", &euid); + { + if (st.st_uid == ROOT_UID) + return 1; + else + extract_id_from_env("SUDO_UID", &euid); + } return st.st_uid == euid; } diff --git a/t/t0034-root-safe-directory.sh b/t/t0034-root-safe-directory.sh index a621f1ea5e..ff31176128 100755 --- a/t/t0034-root-safe-directory.sh +++ b/t/t0034-root-safe-directory.sh @@ -68,7 +68,7 @@ test_expect_success 'can access if addressed explicitly' ' ) ' -test_expect_failure SUDO 'can access with sudo if root' ' +test_expect_success SUDO 'can access with sudo if root' ' ( cd root/p && sudo git status @@ -85,19 +85,6 @@ test_expect_success SUDO 'can access with sudo if root by removing SUDO_UID' ' ) ' -test_lazy_prereq SUDO_SUDO ' - sudo sudo id -u >u && - id -u root >r && - test_cmp u r -' - -test_expect_success SUDO_SUDO 'can access with sudo abusing SUDO_UID' ' - ( - cd root/p && - sudo sudo git status - ) -' - # this MUST be always the last test test_expect_success SUDO 'cleanup' ' sudo rm -rf root -- cgit v1.3 From 5b71c59bc3b9365075e2a175aa7b6f2b0c84ce44 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 17 Jun 2022 17:15:13 -0700 Subject: Git 2.37-rc1 Signed-off-by: Junio C Hamano --- Documentation/RelNotes/2.37.0.txt | 5 ++--- GIT-VERSION-GEN | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/Documentation/RelNotes/2.37.0.txt b/Documentation/RelNotes/2.37.0.txt index f1b93f3c59..99dc7e32f8 100644 --- a/Documentation/RelNotes/2.37.0.txt +++ b/Documentation/RelNotes/2.37.0.txt @@ -234,9 +234,8 @@ Fixes since v2.36 * With a recent update to refuse access to repositories of other people by default, "sudo make install" and "sudo git describe" - stopped working. This series intends to loosen it while keeping - the safety. - (merge b9063afda1 cb/path-owner-check-with-sudo later to maint). + stopped working, which has been corrected. + (merge 6b11e3d52e cb/path-owner-check-with-sudo-plus later to maint). * The tests that ensured merges stop when interfering local changes are present did not make sure that local changes are preserved; now diff --git a/GIT-VERSION-GEN b/GIT-VERSION-GEN index c0b5e722dd..22e76c2a59 100755 --- a/GIT-VERSION-GEN +++ b/GIT-VERSION-GEN @@ -1,7 +1,7 @@ #!/bin/sh GVF=GIT-VERSION-FILE -DEF_VER=v2.37.0-rc1 ;# not quite +DEF_VER=v2.37.0-rc1 LF=' ' -- cgit v1.3