Git development
 help / color / mirror / Atom feed
* [PATCH 2/9] midx: check consistency of fanout table
From: Jeff King @ 2023-11-09  7:12 UTC (permalink / raw)
  To: git; +Cc: Taylor Blau
In-Reply-To: <20231109070310.GA2697602@coredump.intra.peff.net>

The commit-graph, midx, and pack idx on-disk formats all have oid fanout
tables which are fed to bsearch_hash(). If these tables do not increase
monotonically, then the binary search may not only produce bogus values,
it may cause out of bounds reads.

We fixed this for commit graphs in 4169d89645 (commit-graph: check
consistency of fanout table, 2023-10-09). That commit argued that we did
not need to do the same for midx and pack idx files, because they
already did this check. However, that is wrong. We _do_ check the fanout
table for pack idx files when we load them, but we only do so for midx
files when running "git multi-pack-index verify". So it is possible to
get an out-of-bounds read by running a normal command with a specially
crafted midx file.

Let's fix this using the same solution (and roughly the same test) we
did for the commit-graph in 4169d89645. This replaces the same check
from "multi-pack-index verify", because verify uses the same read
routines, we'd bail on reading the midx much sooner now. So let's make
sure to copy its verbose error message.

Signed-off-by: Jeff King <peff@peff.net>
---
 midx.c                      | 20 +++++++++++---------
 t/t5319-multi-pack-index.sh | 14 ++++++++++++++
 2 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/midx.c b/midx.c
index 2f3863c936..1d14661dad 100644
--- a/midx.c
+++ b/midx.c
@@ -64,13 +64,24 @@ void get_midx_rev_filename(struct strbuf *out, struct multi_pack_index *m)
 static int midx_read_oid_fanout(const unsigned char *chunk_start,
 				size_t chunk_size, void *data)
 {
+	int i;
 	struct multi_pack_index *m = data;
 	m->chunk_oid_fanout = (uint32_t *)chunk_start;
 
 	if (chunk_size != 4 * 256) {
 		error(_("multi-pack-index OID fanout is of the wrong size"));
 		return 1;
 	}
+	for (i = 0; i < 255; i++) {
+		uint32_t oid_fanout1 = ntohl(m->chunk_oid_fanout[i]);
+		uint32_t oid_fanout2 = ntohl(m->chunk_oid_fanout[i+1]);
+
+		if (oid_fanout1 > oid_fanout2) {
+			error(_("oid fanout out of order: fanout[%d] = %"PRIx32" > %"PRIx32" = fanout[%d]"),
+			      i, oid_fanout1, oid_fanout2, i + 1);
+			return 1;
+		}
+	}
 	m->num_objects = ntohl(m->chunk_oid_fanout[255]);
 	return 0;
 }
@@ -1782,15 +1793,6 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
 	}
 	stop_progress(&progress);
 
-	for (i = 0; i < 255; i++) {
-		uint32_t oid_fanout1 = ntohl(m->chunk_oid_fanout[i]);
-		uint32_t oid_fanout2 = ntohl(m->chunk_oid_fanout[i + 1]);
-
-		if (oid_fanout1 > oid_fanout2)
-			midx_report(_("oid fanout out of order: fanout[%d] = %"PRIx32" > %"PRIx32" = fanout[%d]"),
-				    i, oid_fanout1, oid_fanout2, i + 1);
-	}
-
 	if (m->num_objects == 0) {
 		midx_report(_("the midx contains no oid"));
 		/*
diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
index c4c6060cee..c20aafe99a 100755
--- a/t/t5319-multi-pack-index.sh
+++ b/t/t5319-multi-pack-index.sh
@@ -1157,4 +1157,18 @@ test_expect_success 'reader notices too-small revindex chunk' '
 	test_cmp expect.err err
 '
 
+test_expect_success 'reader notices out-of-bounds fanout' '
+	# This is similar to the out-of-bounds fanout test in t5318. The values
+	# in adjacent entries should be large but not identical (they
+	# are used as hi/lo starts for a binary search, which would then abort
+	# immediately).
+	corrupt_chunk OIDF 0 $(printf "%02x000000" $(test_seq 0 254)) &&
+	test_must_fail git log 2>err &&
+	cat >expect <<-\EOF &&
+	error: oid fanout out of order: fanout[254] = fe000000 > 5c = fanout[255]
+	fatal: multi-pack-index required OID fanout chunk missing or corrupted
+	EOF
+	test_cmp expect err
+'
+
 test_done
-- 
2.43.0.rc1.572.g273fc7bed6


^ permalink raw reply related

* [PATCH v3 3/3] t9164: fix inability to find basename(1) in Subversion hooks
From: Patrick Steinhardt @ 2023-11-09  7:10 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699513524.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 2201 bytes --]

Hooks executed by Subversion are spawned with an empty environment. By
default, not even variables like PATH will be propagated to them. In
order to ensure that we're still able to find required executables, we
thus write the current PATH variable into the hook script itself and
then re-export it in t9164.

This happens too late in the script though, as we already tried to
execute the basename(1) utility before exporting the PATH variable. This
tends to work on most platforms as the fallback value of PATH for Bash
(see `getconf PATH`) is likely to contain this binary. But on more
exotic platforms like NixOS this is not the case, and thus the test
fails.

While we could work around this issue by simply setting PATH earlier, it
feels fragile to inject a user-controlled value into the script and have
the shell interpret it. Instead, we can refactor the hook setup to write
a `hooks-env` file that configures PATH for us. Like this, Subversion
will know to set up the environment as expected for all hooks.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/t9164-git-svn-dcommit-concurrent.sh | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh
index c8e6c0733f4..d1dec89c3b7 100755
--- a/t/t9164-git-svn-dcommit-concurrent.sh
+++ b/t/t9164-git-svn-dcommit-concurrent.sh
@@ -46,6 +46,14 @@ setup_hook()
 			"passed to setup_hook" >&2 ; return 1; }
 	echo "cnt=$skip_revs" > "$hook_type-counter"
 	rm -f "$rawsvnrepo/hooks/"*-commit # drop previous hooks
+
+	# Subversion hooks run with an empty environment by default. We thus
+	# need to propagate PATH so that we can find executables.
+	cat >"$rawsvnrepo/conf/hooks-env" <<-EOF
+	[default]
+	PATH = ${PATH}
+	EOF
+
 	hook="$rawsvnrepo/hooks/$hook_type"
 	cat > "$hook" <<- 'EOF1'
 		#!/bin/sh
@@ -63,7 +71,6 @@ EOF1
 	if [ "$hook_type" = "pre-commit" ]; then
 		echo "echo 'commit disallowed' >&2; exit 1" >>"$hook"
 	else
-		echo "PATH=\"$PATH\"; export PATH" >>"$hook"
 		echo "svnconf=\"$svnconf\"" >>"$hook"
 		cat >>"$hook" <<- 'EOF2'
 			cd work-auto-commits.svn
-- 
2.42.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 2/3] t/lib-httpd: stop using legacy crypt(3) for authentication
From: Patrick Steinhardt @ 2023-11-09  7:09 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699513524.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

When setting up httpd for our tests, we also install a passwd and
proxy-passwd file that contain the test user's credentials. These
credentials currently use crypt(3) as the password encryption schema.

This schema can be considered deprecated nowadays as it is not safe
anymore. Quoting Apache httpd's documentation [1]:

> Unix only. Uses the traditional Unix crypt(3) function with a
> randomly-generated 32-bit salt (only 12 bits used) and the first 8
> characters of the password. Insecure.

This is starting to cause issues in modern Linux distributions. glibc
has deprecated its libcrypt library that used to provide crypt(3) in
favor of the libxcrypt library. This newer replacement provides a
compile time switch to disable insecure password encryption schemata,
which causes crypt(3) to always return `EINVAL`. The end result is that
httpd tests that exercise authentication will fail on distros that use
libxcrypt without these insecure encryption schematas.

Regenerate the passwd files to instead use the default password
encryption schema, which is md5. While it feels kind of funny that an
MD5-based encryption schema should be more secure than anything else, it
is the current default and supported by all platforms. Furthermore, it
really doesn't matter all that much given that these files are only used
for testing purposes anyway.

[1]: https://httpd.apache.org/docs/2.4/misc/password_encryptions.html

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/lib-httpd/passwd       | 2 +-
 t/lib-httpd/proxy-passwd | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/lib-httpd/passwd b/t/lib-httpd/passwd
index 99a34d64874..d9c122f3482 100644
--- a/t/lib-httpd/passwd
+++ b/t/lib-httpd/passwd
@@ -1 +1 @@
-user@host:xb4E8pqD81KQs
+user@host:$apr1$LGPmCZWj$9vxEwj5Z5GzQLBMxp3mCx1
diff --git a/t/lib-httpd/proxy-passwd b/t/lib-httpd/proxy-passwd
index 77c25138e07..2ad7705d9a3 100644
--- a/t/lib-httpd/proxy-passwd
+++ b/t/lib-httpd/proxy-passwd
@@ -1 +1 @@
-proxuser:2x7tAukjAED5M
+proxuser:$apr1$RxS6MLkD$DYsqQdflheq4GPNxzJpx5.
-- 
2.42.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 1/3] t/lib-httpd: dynamically detect httpd and modules path
From: Patrick Steinhardt @ 2023-11-09  7:09 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699513524.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 2159 bytes --]

In order to set up the Apache httpd server, we need to locate both the
httpd binary and its default module path. This is done with a hardcoded
list of locations that we scan. While this works okayish with distros
that more-or-less follow the Filesystem Hierarchy Standard, it falls
apart on others like NixOS that don't.

While it is possible to specify these paths via `LIB_HTTPD_PATH` and
`LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer
to figure out how to set those up. And in fact we can do better by
dynamically detecting both httpd and its module path at runtime:

    - The httpd binary can be located via PATH.

    - The module directory can (in many cases) be derived via the
      `HTTPD_ROOT` compile-time variable.

Amend the code to do so. The new runtime-detected paths will only be
used in case none of the hardcoded paths are usable.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 t/lib-httpd.sh | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/t/lib-httpd.sh b/t/lib-httpd.sh
index 5fe3c8ab69d..6ab8f273a3a 100644
--- a/t/lib-httpd.sh
+++ b/t/lib-httpd.sh
@@ -55,21 +55,30 @@ fi
 
 HTTPD_PARA=""
 
-for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \
+			  '/usr/sbin/apache2' \
+			  "$(command -v httpd)" \
+			  "$(command -v apache2)"
 do
-	if test -x "$DEFAULT_HTTPD_PATH"
+	if test -n "$DEFAULT_HTTPD_PATH" -a -x "$DEFAULT_HTTPD_PATH"
 	then
 		break
 	fi
 done
 
+if test -x "$DEFAULT_HTTPD_PATH"
+then
+	DETECTED_HTTPD_ROOT="$("$DEFAULT_HTTPD_PATH" -V | sed -n 's/^ -D HTTPD_ROOT="\(.*\)"$/\1/p')"
+fi
+
 for DEFAULT_HTTPD_MODULE_PATH in '/usr/libexec/apache2' \
 				 '/usr/lib/apache2/modules' \
 				 '/usr/lib64/httpd/modules' \
 				 '/usr/lib/httpd/modules' \
-				 '/usr/libexec/httpd'
+				 '/usr/libexec/httpd' \
+				 "${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}"
 do
-	if test -d "$DEFAULT_HTTPD_MODULE_PATH"
+	if test -n "$DEFAULT_HTTPD_MODULE_PATH" -a -d "$DEFAULT_HTTPD_MODULE_PATH"
 	then
 		break
 	fi
-- 
2.42.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply related

* [PATCH v3 0/3] t: improve compatibility with NixOS
From: Patrick Steinhardt @ 2023-11-09  7:09 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano
In-Reply-To: <cover.1699428122.git.ps@pks.im>

[-- Attachment #1: Type: text/plain, Size: 5864 bytes --]

Hi,

this is the third version of my patch series to improve compatibility of
our tests with NixOS.

Changes compared to v2:

    - Patch 1: We now check for both httpd and apache2 binaries via
      PATH.

    - Patch 1: We're a bit more defensive and will check whether
      variables are empty before passing them to either `test -d` or
      `test -x`.

    - Patch 3: Clarified why PATH is unset.

    - Patch 3: Instead of writing PATH into the hook we now write it
      into the `hook-env` configuration file read by Subversion.

Patrick

Patrick Steinhardt (3):
  t/lib-httpd: dynamically detect httpd and modules path
  t/lib-httpd: stop using legacy crypt(3) for authentication
  t9164: fix inability to find basename(1) in Subversion hooks

 t/lib-httpd.sh                        | 17 +++++++++++++----
 t/lib-httpd/passwd                    |  2 +-
 t/lib-httpd/proxy-passwd              |  2 +-
 t/t9164-git-svn-dcommit-concurrent.sh |  9 ++++++++-
 4 files changed, 23 insertions(+), 7 deletions(-)

Range-diff against v2:
1:  cee8fbebf84 ! 1:  e4c75c492dd t/lib-httpd: dynamically detect httpd and modules path
    @@ t/lib-httpd.sh: fi
      HTTPD_PARA=""
      
     -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
    -+for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' "$(command -v httpd)"
    ++for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' \
    ++			  '/usr/sbin/apache2' \
    ++			  "$(command -v httpd)" \
    ++			  "$(command -v apache2)"
      do
    - 	if test -x "$DEFAULT_HTTPD_PATH"
    +-	if test -x "$DEFAULT_HTTPD_PATH"
    ++	if test -n "$DEFAULT_HTTPD_PATH" -a -x "$DEFAULT_HTTPD_PATH"
      	then
    -@@ t/lib-httpd.sh: do
    + 		break
      	fi
      done
      
    @@ t/lib-httpd.sh: do
     +				 '/usr/libexec/httpd' \
     +				 "${DETECTED_HTTPD_ROOT:+${DETECTED_HTTPD_ROOT}/modules}"
      do
    - 	if test -d "$DEFAULT_HTTPD_MODULE_PATH"
    +-	if test -d "$DEFAULT_HTTPD_MODULE_PATH"
    ++	if test -n "$DEFAULT_HTTPD_MODULE_PATH" -a -d "$DEFAULT_HTTPD_MODULE_PATH"
      	then
    + 		break
    + 	fi
2:  f4c6c754f1e = 2:  3dce76da477 t/lib-httpd: stop using legacy crypt(3) for authentication
3:  361f1bd9c88 ! 3:  6891e254155 t9164: fix inability to find basename(1) in hooks
    @@ Metadata
     Author: Patrick Steinhardt <ps@pks.im>
     
      ## Commit message ##
    -    t9164: fix inability to find basename(1) in hooks
    +    t9164: fix inability to find basename(1) in Subversion hooks
     
    -    The post-commit hook in t9164 is executed with a default environment.
    -    To work around this issue we already write the current `PATH` value into
    -    the hook script. But this is done at a point where we already tried to
    -    execute non-built-in commands, namely basename(1). While this seems to
    -    work alright on most platforms, it fails on NixOS.
    +    Hooks executed by Subversion are spawned with an empty environment. By
    +    default, not even variables like PATH will be propagated to them. In
    +    order to ensure that we're still able to find required executables, we
    +    thus write the current PATH variable into the hook script itself and
    +    then re-export it in t9164.
     
    -    Set the `PATH` variable earlier to fix this issue. Note that we do this
    -    via two separate heredocs. This is done because in the first one we do
    -    want to expand variables, whereas in the second one we don't.
    +    This happens too late in the script though, as we already tried to
    +    execute the basename(1) utility before exporting the PATH variable. This
    +    tends to work on most platforms as the fallback value of PATH for Bash
    +    (see `getconf PATH`) is likely to contain this binary. But on more
    +    exotic platforms like NixOS this is not the case, and thus the test
    +    fails.
    +
    +    While we could work around this issue by simply setting PATH earlier, it
    +    feels fragile to inject a user-controlled value into the script and have
    +    the shell interpret it. Instead, we can refactor the hook setup to write
    +    a `hooks-env` file that configures PATH for us. Like this, Subversion
    +    will know to set up the environment as expected for all hooks.
     
         Signed-off-by: Patrick Steinhardt <ps@pks.im>
     
      ## t/t9164-git-svn-dcommit-concurrent.sh ##
     @@ t/t9164-git-svn-dcommit-concurrent.sh: setup_hook()
    + 			"passed to setup_hook" >&2 ; return 1; }
      	echo "cnt=$skip_revs" > "$hook_type-counter"
      	rm -f "$rawsvnrepo/hooks/"*-commit # drop previous hooks
    - 	hook="$rawsvnrepo/hooks/$hook_type"
    --	cat > "$hook" <<- 'EOF1'
    -+	cat >"$hook" <<-EOF
    - 		#!/bin/sh
    - 		set -e
    -+
    -+		PATH=\"$PATH\"
    -+		export PATH
    -+	EOF
     +
    -+	cat >>"$hook" <<-'EOF'
    - 		cd "$1/.."  # "$1" is repository location
    - 		exec >> svn-hook.log 2>&1
    - 		hook="$(basename "$0")"
    -@@ t/t9164-git-svn-dcommit-concurrent.sh: setup_hook()
    - 		cnt="$(($cnt - 1))"
    - 		echo "cnt=$cnt" > ./$hook-counter
    - 		[ "$cnt" = "0" ] || exit 0
    --EOF1
    ++	# Subversion hooks run with an empty environment by default. We thus
    ++	# need to propagate PATH so that we can find executables.
    ++	cat >"$rawsvnrepo/conf/hooks-env" <<-EOF
    ++	[default]
    ++	PATH = ${PATH}
     +	EOF
     +
    + 	hook="$rawsvnrepo/hooks/$hook_type"
    + 	cat > "$hook" <<- 'EOF1'
    + 		#!/bin/sh
    +@@ t/t9164-git-svn-dcommit-concurrent.sh: EOF1
      	if [ "$hook_type" = "pre-commit" ]; then
      		echo "echo 'commit disallowed' >&2; exit 1" >>"$hook"
      	else

base-commit: dadef801b365989099a9929e995589e455c51fed
-- 
2.42.0


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH 1/9] commit-graph: handle overflow in chunk_size checks
From: Jeff King @ 2023-11-09  7:09 UTC (permalink / raw)
  To: git; +Cc: Taylor Blau
In-Reply-To: <20231109070310.GA2697602@coredump.intra.peff.net>

We check the size of chunks with fixed records by multiplying the width
of each record by the number of commits in the file. Like:

  if (chunk_size != g->num_commits * GRAPH_DATA_WIDTH)

If this multiplication overflows, we may not notice a chunk is too small
(which could later lead to out-of-bound reads).

In the current code this is only possible for the CDAT chunk, but the
reasons are quite subtle. We compute g->num_commits by dividing the size
of the OIDL chunk by the hash length (since it consists of a bunch of
hashes). So we know that any size_t multiplication that uses a value
smaller than the hash length cannot overflow. And the CDAT records are
the only ones that are larger (the others are just 4-byte records). So
it's worth fixing all of these, to make it clear that they're not
subject to overflow (without having to reason about seemingly unrelated
code).

The obvious thing to do is add an st_mult(), like:

  if (chunk_size != st_mult(g->num_commits, GRAPH_DATA_WIDTH))

And that certainly works, but it has one downside: if we detect an
overflow, we'll immediately die(). But the commit graph is an optional
file; if we run into other problems loading it, we'll generally return
an error and fall back to accessing the full objects. Using st_mult()
means a malformed file will abort the whole process.

So instead, we can do a division like this:

  if (chunk_size / GRAPH_DATA_WIDTH != g->num_commits)

where there's no possibility of overflow. We do lose a little bit of
precision; due to integer division truncation we'd allow up to an extra
GRAPH_DATA_WIDTH-1 bytes of data in the chunk. That's OK. Our main goal
here is making sure we don't have too _few_ bytes, which would cause an
out-of-bounds read (we could actually replace our "!=" with "<", but I
think it's worth being a little pedantic, as a large mismatch could be a
sign of other problems).

I didn't add a test here. We'd need to generate a very large graph file
in order to get g->num_commits large enough to cause an overflow. And a
later patch in this series will use this same division technique in a
way that is much easier to trigger in the tests.

Signed-off-by: Jeff King <peff@peff.net>
---
There are still several st_mult() calls within commit-graph.c, unrelated
to my jk/chunk-bounds series. I suspect they're all redundant, as the
chunk-size checks give a stricter bound. But checking and removing them
is a separate topic.

I think the midx code has similar st_mult() problems, but it's quite
happy to die() on any error already. So I've left auditing that for
another day.

Another alternative to the division is introducing an st_mult() variant
like:

  if (!st_mult(&out, &a, &b))
        return error(...);

We may want to go there in the long run as part of lib-ification, but I
didn't want to get into it for this series.

 commit-graph.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/commit-graph.c b/commit-graph.c
index ee66098e07..5d7d7a89e5 100644
--- a/commit-graph.c
+++ b/commit-graph.c
@@ -344,7 +344,7 @@ static int graph_read_commit_data(const unsigned char *chunk_start,
 				  size_t chunk_size, void *data)
 {
 	struct commit_graph *g = data;
-	if (chunk_size != g->num_commits * GRAPH_DATA_WIDTH)
+	if (chunk_size / GRAPH_DATA_WIDTH != g->num_commits)
 		return error("commit-graph commit data chunk is wrong size");
 	g->chunk_commit_data = chunk_start;
 	return 0;
@@ -354,7 +354,7 @@ static int graph_read_generation_data(const unsigned char *chunk_start,
 				      size_t chunk_size, void *data)
 {
 	struct commit_graph *g = data;
-	if (chunk_size != g->num_commits * sizeof(uint32_t))
+	if (chunk_size / sizeof(uint32_t) != g->num_commits)
 		return error("commit-graph generations chunk is wrong size");
 	g->chunk_generation_data = chunk_start;
 	return 0;
@@ -364,7 +364,7 @@ static int graph_read_bloom_index(const unsigned char *chunk_start,
 				  size_t chunk_size, void *data)
 {
 	struct commit_graph *g = data;
-	if (chunk_size != g->num_commits * 4) {
+	if (chunk_size / 4 != g->num_commits) {
 		warning("commit-graph changed-path index chunk is too small");
 		return -1;
 	}
-- 
2.43.0.rc1.572.g273fc7bed6


^ permalink raw reply related

* [PATCH 0/9] some more chunk-file bounds-checks fixes
From: Jeff King @ 2023-11-09  7:03 UTC (permalink / raw)
  To: git; +Cc: Taylor Blau

This is a follow-up to the series from:

  https://lore.kernel.org/git/20231009205544.GA3281950@coredump.intra.peff.net/

which was merged to master as jk/chunk-bounds. There were a few issues
left open by that series and its review:

  1. the midx code didn't check fanout ordering

  2. whether we needed to sprinkle some more st_mult() on it

  3. improving some of the error messages (translations, some
     consistency, maybe more details)

  4. possible refactoring with a pair_chunk_expect() API (Taylor posted
     a series in that direction, which is currently in limbo)

The patches here fix the remaining correctness issues (points 1 and 2,
along with a few other small issues I found). There's some improvement
on 3, but I stopped short of adding lots more details. Partially because
the series was already getting big, and partially because going further
may depend on what we do with 4.

With regards to the pair_chunk_expect() thing, this series is
incompatible (textually, but not conceptually) with what Taylor posted
earlier, just because I'm moving some checks into the chunk-reader
callbacks. Because it's fixing user-visible bugs, I think we'd want to
do this first, and then (possibly) rebase Taylor's series on top. But I
also think some of the things I noticed around overflow (especially
patches 1 and 6) may inform how we'd want the pair_chunk_expect() API to
look.

This is a continuation of the jk/chunk-bounds topic, which is new in the
v2.43 cycle. But it should be OK to leave this until after the release.
Nothing here is fixing a regression in the 2.43 release candidates; it's
just a few bits that were incomplete. That said, I did try to float the
correctness bits to the first two patches just in case. ;)

  [1/9]: commit-graph: handle overflow in chunk_size checks
  [2/9]: midx: check consistency of fanout table
  [3/9]: commit-graph: drop redundant call to "lite" verification
  [4/9]: commit-graph: clarify missing-chunk error messages
  [5/9]: commit-graph: abort as soon as we see a bogus chunk
  [6/9]: commit-graph: use fanout value for graph size
  [7/9]: commit-graph: check order while reading fanout chunk
  [8/9]: commit-graph: drop verify_commit_graph_lite()
  [9/9]: commit-graph: mark chunk error messages for translation

 commit-graph.c              | 94 +++++++++++++------------------------
 midx.c                      | 20 ++++----
 t/t5318-commit-graph.sh     | 16 ++++---
 t/t5319-multi-pack-index.sh | 14 ++++++
 4 files changed, 67 insertions(+), 77 deletions(-)

-Peff

^ permalink raw reply

* Re: [PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks
From: Patrick Steinhardt @ 2023-11-09  7:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <ZUx8adNt3Ky-U09W@tanuki>

[-- Attachment #1: Type: text/plain, Size: 1128 bytes --]

On Thu, Nov 09, 2023 at 07:30:01AM +0100, Patrick Steinhardt wrote:
> On Thu, Nov 09, 2023 at 02:43:02AM +0900, Junio C Hamano wrote:
> > Jeff King <peff@peff.net> writes:
[snip]
> > By default, Subversion executes hook scripts with an empty
> > environment—that is, no environment variables are set at all, not even
> > $PATH (or %PATH%, under Windows). Because of this, many administrators
> > are baffled when their hook program runs fine by hand, but doesn't
> > work when invoked by Subversion. Administrators have historically
> > worked around this problem by manually setting all the environment
> > variables their hook scripts need in the scripts themselves.
> 
> So it's not an issue of the environment, but rather an implementation
> detail of how Subversion hooks work. It's surprising that this does not
> fail on other platforms -- maybe the shell has a default PATH there that
> allow us to locate basename(1)? I dunno.

Yup, that's in fact it. Bash falls back to a default PATH that can be
queried via getconf(1p), and this works alright on most platforms. Not
on NixOS though.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path
From: Patrick Steinhardt @ 2023-11-09  6:30 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Junio C Hamano
In-Reply-To: <20231108165426.GB1028115@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]

On Wed, Nov 08, 2023 at 11:54:26AM -0500, Jeff King wrote:
> On Wed, Nov 08, 2023 at 03:57:19PM +0100, Patrick Steinhardt wrote:
> 
> > While it is possible to specify these paths via `LIB_HTTPD_PATH` and
> > `LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer
> > to figure out how to set those up. And in fact we can do better by
> > dynamically detecting both httpd and its module path at runtime:
> > 
> >     - The httpd binary can be located via PATH.
> > 
> >     - The module directory can (in many cases) be derived via the
> >       `HTTPD_ROOT` compile-time variable.
> > 
> > Amend the code to do so. The new runtime-detected paths will only be
> > used in case none of the hardcoded paths are usable.
> 
> If these improve detection on your platform, I think that is a good
> thing and they are worth doing. But as a generic mechanism, I have two
> comments:
> 
> > -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
> > +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' "$(command -v httpd)"
> >  do
> >  	if test -x "$DEFAULT_HTTPD_PATH"
> >  	then
> 
> The binary goes under the name "httpd", but also "apache2". But the PATH
> search only looks for "httpd". Should we check "command -v apache2" as
> well?

Doesn't hurt to do it.

> This also means we may run "test -x" on an empty string, but that is
> probably OK in practice (we could guard it with "test -n", though).

Fair, will do.

> > +if test -x "$DEFAULT_HTTPD_PATH"
> > +then
> > +	DETECTED_HTTPD_ROOT="$("$DEFAULT_HTTPD_PATH" -V | sed -n 's/^ -D HTTPD_ROOT="\(.*\)"$/\1/p')"
> > +fi
> 
> I was really pleased to see this and hoped it could replace the
> kitchen-sink list of paths in the hunk below. But sadly I think it
> depends on having a configured apache setup. On my Debian system, for
> example, I have the "apache2-bin" package installed but not "apache"
> (because only the latter actually sets up a system apache daemon, which
> I don't want). And thus there is no config:
> 
>   $ /usr/sbin/apache2 -D HTTPD_ROOT
>   apache2: Could not open configuration file /etc/apache2/apache2.conf: No such file or directory
> 
> So without a system config file to act as a template for our custom
> config, I think we are stuck with guessing where the installer might
> have put them.

Indeed. Also, paths like `/usr/libexec/apache2` wouldn't be possible to
be discovered via `${HTTPD_ROOT}/modules` anyway. It's kind of a shame
that there is no way (or at least no way I know of) to portably discover
the default modules directory used by httpd.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks
From: Patrick Steinhardt @ 2023-11-09  6:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqqwmus3zvd.fsf@gitster.g>

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On Thu, Nov 09, 2023 at 02:43:02AM +0900, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
> 
> > ... But the more robust way to do it is:
> >
> >   # export ORIG_PATH, which presumably is not cleared the same way PATH
> >   # is, so that the hook can access it
> >   ORIG_PATH=$PATH &&
> >   export ORIG_PATH &&
> >
> >   cat >"$hook" <<EOF
> >   # pull the original path from the caller
> >   PATH=$ORIG_PATH
> >   export PATH
> >
> >   ...do other stuff...
> >   EOF
> >
> > That's assuming that environment variables make it intact to the hook at
> > all (it is not clear to me why the original $PATH doesn't).
> 
> Yeah, the parenthetical comment points at the crux of the issue.  I
> can tell from the patch what issue the platform throws at us we are
> trying to work around, but it is frustrating not to know why the
> platform does such unpleasant things in the first place.
> 
> Thanks.

Indeed, I should've described this better in the commit message. Quoting
[1]:

> By default, Subversion executes hook scripts with an empty
> environment—that is, no environment variables are set at all, not even
> $PATH (or %PATH%, under Windows). Because of this, many administrators
> are baffled when their hook program runs fine by hand, but doesn't
> work when invoked by Subversion. Administrators have historically
> worked around this problem by manually setting all the environment
> variables their hook scripts need in the scripts themselves.

So it's not an issue of the environment, but rather an implementation
detail of how Subversion hooks work. It's surprising that this does not
fail on other platforms -- maybe the shell has a default PATH there that
allow us to locate basename(1)? I dunno.

Will add to the commit message.

Patrick

[1]: https://svnbook.red-bean.com/en/1.8/svn.reposadmin.create.html#svn.reposadmin.create.hooks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Junio C Hamano @ 2023-11-09  1:32 UTC (permalink / raw)
  To: Victoria Dye; +Cc: Patrick Steinhardt, Victoria Dye via GitGitGadget, git
In-Reply-To: <xmqqfs1f3eji.fsf@gitster.g>

Junio C Hamano <gitster@pobox.com> writes:

> You have to feed it to "git cat-file tag" and parse the contents of
> the tag obbject yourself to manually peel further levels of onion.

Alternatively, you can drive "git show -s" with "--format" and you
probably can produce a machine parseable output.

But it does not change the argument fundamentally.  The point is
that "for-each-ref --format=%(*field)" that peels only the first
layer would not have helped all that much, if somebody really cares
about each levels of nested tags.  They would have been relying on
a solution to deal with the second and further layers anyway, and
that solution would have been working with the first layer, too.


^ permalink raw reply

* Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Junio C Hamano @ 2023-11-09  1:23 UTC (permalink / raw)
  To: Victoria Dye; +Cc: Patrick Steinhardt, Victoria Dye via GitGitGadget, git
In-Reply-To: <898d3850-b0ca-485e-9489-320eee3121e4@github.com>

Victoria Dye <vdye@github.com> writes:

> I'd certainly prefer that from a technical standpoint; it simplifies this
> patch if I can just replace 'get_tagged_oid' with 'peel_iterated_oid'. The
> two things that make me hesitate are:
>
> 1. There isn't a straightforward 1:1 substitute available for getting info
>    on the immediate target of a list of tags. 
> 2. The performance of a recursive peel can be worse than that of a single
>    tag dereference, since (unless the formatting is done in a ref_iterator
>    iteration *and* the tag is a packed ref) the dereferenced object needs to
>    be resolved to determine whether it's another tag or not.
>
> #1 may not be an issue in practice, but I don't have enough information on
> how applications use that formatting atom to say for sure. #2 is a bigger
> issue, IMO, since one of the goals of this series was to improve performance
> for some cases of 'for-each-ref' without hurting it in others.

In a repository without any tag-to-tag at tips of refs, would #2
above still be an issue?  My assumption when I raised "isn't this
simply a bug?" question was that the use of tag-to-tag is a mere
intellectual curiosity, there is no serious use case, and they are
not heavily used.  Hence I was envisioning that #1 below (i.e., a
mention in the Release Notes' backward compatibility notes section)
would be sufficient.

If it weren't the case, then I do not think any "transition" would
work, either.

And stepping back a bit, even though "peel only once" is how
for-each-ref works, I do not think anybody who really cares about
tag-to-tag and inspecting each level of peeled tag is helped by it
all that much.  Yes, you can get the result of single level peeling
via "git format-patch --format=%(*objectname)", but then what would
you do to dig further from that point?  You cannot ask rev-parse to
peel the result with "^{}", as that will peel all the way down.

You have to feed it to "git cat-file tag" and parse the contents of
the tag obbject yourself to manually peel further levels of onion.
Anybody who do care must already have such a machinery, and such a
machinery does not depend on "git for-each-ref --format='%(*field)'"
peeling just once, I would say.  They would most likely learn the
"%(objectname) %(objecttype) %(refname)" from the command, and for
those that are tags, they would manually peel the object with such a
machinery, because they have to do that for second and further
levels anyway.

And that is why I am not so worried about "breaking" existing users
in this particular case.  Our existing support with tag-to-tag is so
poor that those who truly need it would have invented necessary
support without relying on for-each-ref's peeling (if such people
did exist, that is).

But perhaps I am so overly optimistic against Hyrum's law.

> I can (and would like to) deprecate the "peel once" behavior and replace it
> with "peel all", but with how long it's been around and the potential
> performance impact, such a change should probably be clearly communicated.
> How that happens depends on how aggressively we want to cut over. We could:
>
> 1. Change the behavior of '*' from single dereference to recursive
>    dereference, make a note of it in the documentation.
> 2. Same as #1, but also add an option like '--no-recursive-dereference' or
>    something to use the old behavior. Remove the option after 1-2 release
>    cycles?
> 3. Add a new format specifier '^{}', note that '*' is deprecated in the
>    docs.
> 4. Same as #3, but also show a warning/advice if '*' is used.
> 5. Same as #3, but die() if '*' is used.
>
> I'm open to other options, those were just the first few I could think of. 

^ permalink raw reply

* Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Junio C Hamano @ 2023-11-09  1:22 UTC (permalink / raw)
  To: Victoria Dye; +Cc: Patrick Steinhardt, Victoria Dye via GitGitGadget, git
In-Reply-To: <898d3850-b0ca-485e-9489-320eee3121e4@github.com>

Victoria Dye <vdye@github.com> writes:

> I can (and would like to) deprecate the "peel once" behavior and replace it
> with "peel all", but with how long it's been around and the potential
> performance impact, such a change should probably be clearly communicated.

I've written a fairly detailed response on this about the reason why
I think that "leave a mention in the backward compatibility notes
section of the release notes" (your #1) is sufficient, but it seems
to have been lost in the ether.  I'll wait a bit and if the previous
response does not materialize, I may type it again.

But in addition to what I wrote there, there is this thread [*] from
2019 that indicates that our position is to mildly discourage
tag-to-tag in the first place.


[Reference]

* https://lore.kernel.org/git/20190404020226.GG4409@sigill.intra.peff.net/

^ permalink raw reply

* Re: [PATCH v2 1/3] t/lib-httpd: dynamically detect httpd and modules path
From: Junio C Hamano @ 2023-11-09  0:30 UTC (permalink / raw)
  To: Jeff King; +Cc: Patrick Steinhardt, git
In-Reply-To: <20231108165426.GB1028115@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> On Wed, Nov 08, 2023 at 03:57:19PM +0100, Patrick Steinhardt wrote:
>
>> While it is possible to specify these paths via `LIB_HTTPD_PATH` and
>> `LIB_HTTPD_MODULE_PATH`, it is not a nice experience for the developer
>> to figure out how to set those up. And in fact we can do better by
>> dynamically detecting both httpd and its module path at runtime:
>> 
>>     - The httpd binary can be located via PATH.
>> 
>>     - The module directory can (in many cases) be derived via the
>>       `HTTPD_ROOT` compile-time variable.
>> 
>> Amend the code to do so. The new runtime-detected paths will only be
>> used in case none of the hardcoded paths are usable.
>
> If these improve detection on your platform, I think that is a good
> thing and they are worth doing. But as a generic mechanism, I have two
> comments:
>
>> -for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2'
>> +for DEFAULT_HTTPD_PATH in '/usr/sbin/httpd' '/usr/sbin/apache2' "$(command -v httpd)"
>>  do
>>  	if test -x "$DEFAULT_HTTPD_PATH"
>>  	then
>
> The binary goes under the name "httpd", but also "apache2". But the PATH
> search only looks for "httpd". Should we check "command -v apache2" as
> well?

Interesting.  If $() were on the right hand side of an assignment,
the command that fails may also something to watch for, but I think
$? from $(command -v no-such-httpd) would be discarded, so it would
be fine in this case.  A trivia I found out today: dash exits with 127
and bash exits with 1 when doing 

    $shell -c 'command -v no-such-httpd'

> This also means we may run "test -x" on an empty string, but that is
> probably OK in practice (we could guard it with "test -n", though).

Good thinking.  We are not worried about exotic or misbehaving
shells in this thread, so hopefully shell built-in 'test' would say
"no" when asked if "" is executable (in other words, "" is not
mistaken as ".").  But an explicit "test -n" would save the readers
by removing the worry.

> ... So without a system config file to act as a template for our custom
> config, I think we are stuck with guessing where the installer might
> have put them.

Right.  Thanks for a careful analysis.

^ permalink raw reply

* [PATCH] merge-file: add --diff-algorithm option
From: Antonin Delpeuch via GitGitGadget @ 2023-11-08 21:54 UTC (permalink / raw)
  To: git; +Cc: Antonin Delpeuch, Antonin Delpeuch

From: Antonin Delpeuch <antonin@delpeuch.eu>

This makes it possible to use other diff algorithms than the 'myers'
default algorithm, when using the 'git merge-file' command.

Signed-off-by: Antonin Delpeuch <antonin@delpeuch.eu>
---
    merge-file: add --diff-algorithm option

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1606%2Fwetneb%2Fmerge_file_configurable_diff_algorithm-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1606/wetneb/merge_file_configurable_diff_algorithm-v1
Pull-Request: https://github.com/git/git/pull/1606

 Documentation/git-merge-file.txt |  5 +++++
 builtin/merge-file.c             | 28 ++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index 6a081eacb72..917535217c1 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -92,6 +92,11 @@ object store and the object ID of its blob is written to standard output.
 	Instead of leaving conflicts in the file, resolve conflicts
 	favouring our (or their or both) side of the lines.
 
+--diff-algorithm <algorithm>::
+	Use a different diff algorithm while merging, which can help
+	avoid mismerges that occur due to unimportant matching lines
+	(such as braces from distinct functions).  See also
+	linkgit:git-diff[1] `--diff-algorithm`.
 
 EXAMPLES
 --------
diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index 832c93d8d54..1f987334a31 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -1,5 +1,6 @@
 #include "builtin.h"
 #include "abspath.h"
+#include "diff.h"
 #include "hex.h"
 #include "object-name.h"
 #include "object-store.h"
@@ -28,6 +29,30 @@ static int label_cb(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+static int set_diff_algorithm(xpparam_t *xpp,
+			      const char *alg)
+{
+	long diff_algorithm = parse_algorithm_value(alg);
+	if (diff_algorithm < 0)
+		return -1;
+	xpp->flags = (xpp->flags & ~XDF_DIFF_ALGORITHM_MASK) | diff_algorithm;
+	return 0;
+}
+
+static int diff_algorithm_cb(const struct option *opt,
+				const char *arg, int unset)
+{
+	xpparam_t *xpp = opt->value;
+
+	BUG_ON_OPT_NEG(unset);
+
+	if (set_diff_algorithm(xpp, arg))
+		return error(_("option diff-algorithm accepts \"myers\", "
+			       "\"minimal\", \"patience\" and \"histogram\""));
+
+	return 0;
+}
+
 int cmd_merge_file(int argc, const char **argv, const char *prefix)
 {
 	const char *names[3] = { 0 };
@@ -48,6 +73,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 			    XDL_MERGE_FAVOR_THEIRS),
 		OPT_SET_INT(0, "union", &xmp.favor, N_("for conflicts, use a union version"),
 			    XDL_MERGE_FAVOR_UNION),
+		OPT_CALLBACK_F(0, "diff-algorithm", &xmp.xpp, N_("<algorithm>"),
+			     N_("choose a diff algorithm"),
+			     PARSE_OPT_NONEG, diff_algorithm_cb),
 		OPT_INTEGER(0, "marker-size", &xmp.marker_size,
 			    N_("for conflicts, use this marker size")),
 		OPT__QUIET(&quiet, N_("do not warn about conflicts")),

base-commit: 98009afd24e2304bf923a64750340423473809ff
-- 
gitgitgadget

^ permalink raw reply related

* Re: git-send-email: Send with mutt(1)
From: Jeff King @ 2023-11-08 21:27 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: git
In-Reply-To: <ZUv3gjjmuqvCaJEd@debian>

On Wed, Nov 08, 2023 at 10:02:52PM +0100, Alejandro Colomar wrote:

> > That seems like a reasonable feature. Probably it should be
> > --cc-from-trailer=signed-off-by, and then you could do the same with
> > other trailers.
> 
> That would work for me.  I only suggested the other one because it's
> aleady in send-email in that form.  But yeah, it might be useful to have
> finer control.  In my case, something that CCs every mail in the trailer
> would work (Reviewed-by, Suggested-by, ...  I want them all, always).

Oh, I didn't realize it existed in send-email. I am showing my ignorance
of it. ;)

> > # spool the message to a fake mbox; we need to add
> > # a "From" line to make it look legit
> > trap 'rm -f to-send' 0 &&
> > {
> >   echo "From whatever Mon Sep 17 00:00:00 2001" &&
> >   cat
> > } >to-send &&
> 
> Would a named pipe work?  Or maybe we could use $(mktemp)?

I suspect mutt wants it to be a real file. But yeah, mktemp would
definitely work. I actually started to write it that way but switched to
a static name for simplicity in demonstrating the idea. :)

One note, though. Later we need to pass this filename to mutt config:

> > mutt -p \
> >   -e 'set postponed=to-send' \

so it's a potential worry if "mktemp" might use a path with spaces or
funny characters (e.g., from $TMPDIR). Probably not much of a problem in
practice, though.

> Huh, this is magic sauce!  Works perfect for what I need.  This would
> need to be packaged to the masses.  :-)
> 
> I found a minor problem: If I ctrl+C within mutt(1), I expect it to
> cancel the last action, but this script intercepts the signal and exits.
> We would probably need to ignore SIGINT from mutt-as-mta.

Yeah, that might make sense, and can be done with trap.

> Would you mind adding this as part of git?  Or should we suggest the
> mutt project adding this script?

IMHO it is a little too weird and user-specific to really make sense in
either project. It's really glue-ing together two systems. And as it's
not something I use myself, I don't plan it moving it further along. But
you are welcome to take what I wrote and do what you will with it,
including submitting it to mutt.

-Peff

^ permalink raw reply

* Re: git-send-email: Send with mutt(1)
From: Alejandro Colomar @ 2023-11-08 21:02 UTC (permalink / raw)
  To: Jeff King; +Cc: git
In-Reply-To: <20231107201655.GA507701@coredump.intra.peff.net>

[-- Attachment #1: Type: text/plain, Size: 4632 bytes --]

Hi Jeff!

On Tue, Nov 07, 2023 at 03:16:55PM -0500, Jeff King wrote:
> > I had been thinking these days that it would be useful to have
> > format-patch generate those.  How about adding a --signed-off-by-cc to
> > format-patch?
> 
> That seems like a reasonable feature. Probably it should be
> --cc-from-trailer=signed-off-by, and then you could do the same with
> other trailers.

That would work for me.  I only suggested the other one because it's
aleady in send-email in that form.  But yeah, it might be useful to have
finer control.  In my case, something that CCs every mail in the trailer
would work (Reviewed-by, Suggested-by, ...  I want them all, always).

> 
> It feels like you could _almost_ do it with the existing
> --format='%(trailers)' functionality, but there's no way to say "do the
> regular --format=email output, but also stick this extra format in the
> headers section". Plus there are probably some niceties you'd get from
> Git knowing that you're adding headers (like de-duping addresses).
> 
> That feature might end up somewhat hairy, though, as then you get into
> questions of parsing address lists, etc. We do all that now in perl with
> send-email, where we can lean on some parsing libraries. So I dunno.

Hmmm.

> 
> > > If you're sending a long series, it's helpful to pre-populate various
> > > headers in the format-patch command with "--to", etc. I usually do so by
> > > sending the cover letter directly via mutt, and then using some perl
> > > hackery to convert those headers into format-patch args. The script I
> > 
> > Indeed, that hackery is what send-email already does, so how about
> > moving those features a bit upstream so that format-patch can do them
> > too?
> 
> Yeah, if they existed in format-patch I might be able to reuse them. I
> am hesitant, though, just because handling all the corner cases on
> parsing is going to be a bit of new C code.
> 
> > Although then, maybe it's simpler to teach send-email to learn to use
> > mutt(1) under the hood for the actual send.
> 
> I think you will find some corner cases in trying to make mutt act just
> like an mta accepting delivery. Two I can think of:
> 
>   1. It will take a body on stdin, but not a whole message. We can hack
>      around that with some postponed-folder magic, though.
> 
>   2. Bcc headers are stripped before sendmail sees the message (but
>      those addresses appear on the command-line). Converting that back
>      to bcc so that mutt can then re-strip them would be annoying but
>      possible. If you don't use bcc, it probably makes sense to just
>      punt on this.

Meh, I can live with no Bcc feature; I never used it before.  :)

> 
> So maybe a script like this:
> 
> -- >8 --
> #!/bin/sh
> 
> # ignore arguments; mutt will parse them itself
> # from to/cc headers. Note that we'll miss bcc this
> # way, but handling that would probably be kind of
> # tricky; we'd need to re-add those recipients as actual
> # bcc headers so that mutt knows how to handle them.
> 
> # spool the message to a fake mbox; we need to add
> # a "From" line to make it look legit
> trap 'rm -f to-send' 0 &&
> {
>   echo "From whatever Mon Sep 17 00:00:00 2001" &&
>   cat
> } >to-send &&

Would a named pipe work?  Or maybe we could use $(mktemp)?

> 
> # and then have mutt "resume" it. We have to redirect
> # stdin back from the terminal, since ours is a pipe
> # with the message contents.
> mutt -p \
>   -e 'set postponed=to-send' \
>   -e 'set edit_headers=yes' \
>   </dev/tty
> -- 8< --

Huh, this is magic sauce!  Works perfect for what I need.  This would
need to be packaged to the masses.  :-)

I found a minor problem: If I ctrl+C within mutt(1), I expect it to
cancel the last action, but this script intercepts the signal and exits.
We would probably need to ignore SIGINT from mutt-as-mta.

Other than that, this fulfills all of my needs.

Would you mind adding this as part of git?  Or should we suggest the
mutt project adding this script?


Thanks a lot!

Cheers,
Alex

> 
> and then in your git config:
> 
>   [sendemail]
>   sendmailcmd = /path/to/mutt-as-mta.sh
> 
> There are mutt-specific bits there that I don't think send-email should
> have to know about. Perhaps there are generic options that send-email
> could learn, but it really feels like you'd do better teaching mutt to
> be more ready to handle this (like taking a whole message on stdin,
> headers and all, rather than just a body).
> 
> -Peff

-- 
<https://www.alejandro-colomar.es/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: Regression: git send-email Message-Id: numbering doesn't start at 1 any more
From: Michael Strawbridge @ 2023-11-08 20:39 UTC (permalink / raw)
  To: Uwe Kleine-König, Junio C Hamano; +Cc: Douglas Anderson, git, entwicklung
In-Reply-To: <20231107070632.spe3cappk5b5jg3q@pengutronix.de>



On 11/7/23 02:06, Uwe Kleine-König wrote:
> Hello Junio,
> 
> On Tue, Nov 07, 2023 at 08:06:22AM +0900, Junio C Hamano wrote:
>> Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:
>>
>>> Hello,
>>>
>>> Since commit 3ece9bf0f9e24909b090cf348d89e8920bd4f82f I experience that
>>> the generated Message-Ids don't start at ....-1-... any more. I have:
>>>
>>> $ git send-email w/*
>>> ...
>>> Subject: [PATCH 0/5] watchdog: Drop platform_driver_probe() and convert to platform remove callback returning void (part II)
>>> Date: Mon,  6 Nov 2023 16:10:04 +0100
>>> Message-ID: <20231106151003.3844134-7-u.kleine-koenig@pengutronix.de>
>>> ...
>>>
>>> So the cover letter is sent with Message-Id: ...-7-...
>>
>> The above is consistent with the fact that a 5-patch series with a
>> cover letter consists of 6 messages.  Dry-run uses message numbers
>> 1-6 and forgets to reset the counter, so the next message becomes 7.
>> As you identified, the fix in 3ece9bf0 (send-email: clear the
>> $message_id after validation, 2023-05-17) for the fallout from an
>> even earlier change to process each message twice still had left an
>> observable side effect subject to the Hyrum's law, it seems.
>>
>>> +my ($message_id_stamp, $message_id_serial);
>>>  if ($validate) {
>>>  	# FIFOs can only be read once, exclude them from validation.
>>>  	my @real_files = ();
>>> @@ -821,6 +822,7 @@ sub is_format_patch_arg {
>>>  	}
>>>  	delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
>>>  	delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
>>> +	$message_id_serial = 0;
>>>  }
>>
>> This fix looks quite logical to me, but even with this, the side
>> effects of the earlier "read message twice" persists in end-user
>> observable form, don't they?  IIRC, when sending out an N message
>> series, we start from the timestamp as of N seconds ago and give
>> each message the Date: header that increments by 1 second, which
>> would mean the validator will see Date: that is different from what
>> will actually be sent out, and more importantly, the messages sent
>> out for real will have timestamps from the future, negating the
>> point of starting from N seconds ago in the first place.
> 
> The series I used as an example here was finally sent out with
> git-send-email patched with my suggested change.
> 
> The Message-Ids involved are:
> 
> 	20231106154807.3866712-1-u.kleine-koenig@pengutronix.de
> 	20231106154807.3866712-2-u.kleine-koenig@pengutronix.de
> 	20231106154807.3866712-3-u.kleine-koenig@pengutronix.de
> 	20231106154807.3866712-4-u.kleine-koenig@pengutronix.de
> 	20231106154807.3866712-5-u.kleine-koenig@pengutronix.de
> 	20231106154807.3866712-6-u.kleine-koenig@pengutronix.de
> 
> So the Ids are are identical apart from the number this report is about.
> 
>> Your script may not have been paying attention to it and only noticed
>> the difference in id_serial, but somebody else would complain the
>> difference coming from calling gen_header more than once for each
>> messages since a8022c5f (send-email: expose header information to
>> git-send-email's sendemail-validate hook, 2023-04-19).
>>
>> So, I dunno.  Michael, what do you think?  It appears to me that a
>> more fundamental fix to the fallout from a8022c5f might be needed
>> (e.g., we still let gen_header run while validating, but once
>> validation is done, save the headers that validator saw and use them
>> without calling gen_header again when we send the messages out, or
>> something), if we truly want to be regression free.
> 
> That sounds sane.

I agree that a more fundamental fix would be optimal.  Saving the header
information after generating it and using that in both validation and
sending would be great.  That would probably need to be paired with
cleaning up all the random code in between the function declarations
so that we can get a good look at all the variables.  Right now the
control flow is hard to follow.  I can't guarantee any specific timeline
on those patches.  It would likely be fairly large changes.  If anyone
else wants to take a crack at it, feel free.  Otherwise I may tinker
and post when I have a chance.
> 
>> By the way, out of curiosity, earlier you said your script looks at
>> the Message-IDs and counts the number of messages.  How does it do
>> that?  Does it read the output of send-email and pass the messages
>> to MTA for sending out for real?
> 
> The output of git send-email dumps the messages it sends out and then I
> pick the message-id of the last mail by cut-n-paste and call my script
> with that as a parameter. It then adds notes to the $commitcount topmost
> commits such that I have something like this on the sent out commits:
> 
> 	Notes:
> 	    Forwarded: id:20231106154807.3866712-2-u.kleine-koenig@pengutronix.de (v1)
> 
> This is very convenient to find the thread to ping if a patch doesn't
> make it into the next release.
> 
> (By the way, one difficulty in my script is that depending on the series
> having a cover letter or not I have to apply the id:....-1-... marker or
> not. Would be great if git send-email started with ...-0-... for a
> series with a cover letter. Detecting that reliably isn't trivial I
> guess.)
> 
> Best regards
> Uwe
> 

^ permalink raw reply

* [ANNOUNCE] Git for Windows 2.43.0-rc1
From: Johannes Schindelin @ 2023-11-08 19:53 UTC (permalink / raw)
  To: git-for-windows, git, git-packagers; +Cc: Johannes Schindelin

Dear Git users,

I hereby announce that Git for Windows 2.43.0-rc1 is available from:

    https://github.com/git-for-windows/git/releases/tag/v2.43.0-rc1.windows.1

Changes since Git for Windows v2.42.0(2) (August 30th 2023)

New Features

  * Comes with Git v2.43.0-rc1.
  * Comes with MinTTY v3.6.5.
  * Comes with MSYS2 runtime v3.4.9.
  * Comes with GNU TLS v3.8.1.
  * When installing into a Windows setup with Mandatory Address Space
    Layout Randomization (ASLR) enabled, which is incompatible with the
    MSYS2 runtime powering Git Bash, SSH and some other programs
    distributed with Git for Windows, the Git for Windows installer now
    offers to add exceptions that will allow those programs to work as
    expected.
  * Comes with OpenSSH v9.5.P1.
  * Comes with cURL v8.4.0.
  * Comes with OpenSSL v3.1.4.
  * Comes with Git Credential Manager v2.4.1.

Bug Fixes

  * Symbolic links whose target is an absolute path without the drive
    prefix accidentally had a drive prefix added when checked out,
    rendering them "eternally modified". This bug has been fixed.
  * Git for Windows's installer is no longer confused by global GIT_*
    environment variables.
  * The installer no longer claims that "fast-forward or merge" is the
    default git pull behavior: The default behavior has changed in Git
    a while ago, to "fast-forward only".

Git-2.43.0-rc1-64-bit.exe | 88ede07be3d3cee256f180130ed69e978e93271e835d31078e75ec8c0b13b77e
Git-2.43.0-rc1-32-bit.exe | cf66dda600bf0d1dbb7e4ae85c33cc81fd3a5aefc09ea43bad8737f2c7043644
PortableGit-2.43.0-rc1-64-bit.7z.exe | 5027018b1274ff5164f9ca1277838aea867c78c521a0a7d418b005dee651e00b
PortableGit-2.43.0-rc1-32-bit.7z.exe | 42a771402c47e0f0bc10100107fc5427951e95b309e5c3f53e48aed033165e98
MinGit-2.43.0-rc1-64-bit.zip | e3ca213c6f6f4acc08b249032770b5a061310473444c59628f0732bee02e15f1
MinGit-2.43.0-rc1-32-bit.zip | 375e8ec3c1d91536c3a2800db52c851d06e9273c0f0e13bc7cd54f32159e06d3
MinGit-2.43.0-rc1-busybox-64-bit.zip | bb4f22761110411d519f7c05bdd0655938db9d056dd929074d68603ff2f3713c
MinGit-2.43.0-rc1-busybox-32-bit.zip | 8eabb4f8b16018fe4124841957aebc1e2a9de9ed12c40b20b2bae59a69bfb088
Git-2.43.0-rc1-64-bit.tar.bz2 | fab6adea4a09a079a783aae395cf8c5963bca97095f16790f4c30f54ced8303c
Git-2.43.0-rc1-32-bit.tar.bz2 | e29cf087c9c5456092a53736b563d4c84c9ed4100552b532a7cf6abef232bef7

Ciao,
Johannes

^ permalink raw reply

* Re: first-class conflicts?
From: Martin von Zweigbergk @ 2023-11-08 18:22 UTC (permalink / raw)
  To: Elijah Newren; +Cc: phillip.wood, Sandra Snan, git, Randall S. Becker
In-Reply-To: <CABPp-BEcqSJ79b9WLm+KgKkcPwSwTv3o13meU_aXakQhV6iKDQ@mail.gmail.com>

Hi Elijah,


On Tue, Nov 7, 2023 at 11:31 PM Elijah Newren <newren@gmail.com> wrote:
>
> Hi Martin,
>
> On Tue, Nov 7, 2023 at 9:38 AM Martin von Zweigbergk
> <martinvonz@google.com> wrote:
> >
> [...]
> > > One thing to think about if we ever want to implement this is what other
> > > data we need to store along with the conflict trees to preserve the
> > > context in which the conflict was created. For example the files that
> > > are read by "git commit" when it commits a conflict resolution. For a
> > > single cherry-pick/revert it would probably be fairly straight forward
> > > to store CHERRY_PICK_HEAD/REVERT_HEAD and add it as a parent so it gets
> > > transferred along with the conflicts. For a sequence of cherry-picks or
> > > a rebase it is more complicated to preserve the context of the conflict.
> > > Even "git merge" can create several files in addition to MERGE_HEAD
> > > which are read when the conflict resolution is committed.
> >
> > Good point. We actually don't store any extra data in jj. The old
> > per-path conflict model was prepared for having some label associated
> > with each term of the conflict but we never actually used it.
> >
> > If we add such metadata, it would probably have to be something that
> > makes sense even after pushing the conflict to another repo, so it
> > probably shouldn't be commit ids, unless we made sure to also push
> > those commits. Also note that if you `jj restore --from <commit with
> > conflict>`, you can get a conflict into a commit that didn't have
> > conflicts previously. Or if you already had conflicts in the
> > destination commit, your root trees (the multiple root trees
> > constituting the conflict) will now have conflicts that potentially
> > were created by two completely unrelated operations, so you would kind
> > of need different labels for different paths.
> >
> > https://github.com/martinvonz/jj/issues/1176 has some more discussion
> > about this.
>
> Interesting link; thanks for sharing.
>
> I am curious more about the data you do store.  My fuzzy memory is
> that you store a commit header involving something of the form "A + B
> - C", where those are all commit IDs.  Is that correct?

We actually store it outside the Git repo (together with the "change
id"). We have avoided using commit headers because I wasn't sure how
well different tools deal with unexpected commit headers, and because
I wanted commits to be indistinguishable from commits created by a
regular Git binary. The latter argument doesn't apply to commits with
conflicts since those are clearly not from a regular Git binary
anyway, and we don't allow pushing them to a remote.

>  Is this in
> addition to a normal "tree" header as in Git, or are one of A or B
> found in the tree header?

It's in addition. For the tree, we actually write a tree object with
three subtrees:

.jjconflict-base-0: C
.jjconflict-side-0: A
.jjconflict-side-1: B

The tree is not authoritative - we use the Git-external storage for
that. The reason we write the trees is mostly to prevent them from
getting GC'd. Also, if a user does `git checkout <conflicted commit>`,
they'll see those subdirectories and will hopefully be reminded that
they did something odd (perhaps we should drop the leading `.` so `ls`
will show them...). They can also diff the directories in a diff tool
if they like.

>  I think you said there was also the
> possibility for more than three terms.  Are those for when a
> conflicted commit is merged with another branch that adds more
> conflicts, or are there other cases too?  (Octopus merges?)

Yes, they can happen in both of those cases you mention. More
generally, whenever you apply a diff between two trees onto another
tree, you might end up with a higher-arity conflict. So merging in
another branch can do that, or doing an octopus merge (which is the
same thing at the tree level, just different at the commit level), or
rebasing or reverting a commit.

We simplify conflicts algebraically, so rebasing a commit multiple
times does not increase the arity - the intermediate parents were both
added and removed and thus cancel out. These simple algorithms for
simplifying conflicts are encapsulated in
https://github.com/martinvonz/jj/blob/main/lib/src/merge.rs. Most of
them are independent of the type of values being merged; they can be
used for doing algebra on tree ids, content hunks, refs, etc. (in the
test cases, we mostly merge integers because integer literals are
compact).

> What about recursive merges, i.e. merges where the two sides do not
> have a unique merge base.  What is the form of those?  (Would "- C" be
> replaced by "- C1 - C2 - ... - Cn"?  Or would we create the virtual
> merge base V and then do a " - V"?  Or do we only have "A + B"?)

We do that by recursively creating a virtual tree just like Git does,
I think (https://github.com/martinvonz/jj/blob/084b99e1e2c42c40f2d52038cdc97687b76fed89/lib/src/rewrite.rs#L56-L71).
I think the main difference is that by modeling conflicts, we can
avoid recursive conflict markers (if that's what Git does), and we can
even automatically resolve some cases where the virtual tree has a
conflict.

> You previously mentioned that if someone goes to edit a commit with
> conflicts, and resolves the conflicts in just one file, then you can
> modify each of the trees A, B, and C such that a merging of those
> trees gives the partially resolved result.  How does one do that with
> special conflicts, such as:
>    * User modifies file D on both sides of history, in conflicting
> ways, and also renames D -> E on one side of history.  User checks out
> this conflicted commit and fixes the conflicts in E (but not other
> files) and does a "git add E".  When they go to commit, does the
> machinery need a mapping to figure out that it needs to adjust "D" in
> two of the trees while adjusting "E" in the other?
>    * Similar to the above, but the side that doesn't rename D renames
> olddir/ -> newdir/, and the side that renames D instead renames
> D->olddir/E.  For this case, the file will end up at newdir/E; do we
> need the backward mapping from newdir/E to both olddir/E and D?
>    * Slightly different than the above: User renames D -> E on one
> side of history, and D -> F on the other.  That's a rename/rename
> (1to2) conflict.  User checks out this conflicted commit and does a
> "git add F", marking it as okay, but leaving E conflicted.  How can
> one adjust the tree such that no conflict for F appears, but one still
> appears for E?
>    * Similar to above with an extra wrinkle: User renames D -> E on
> one side of history, and on the other side both renames D -> F and
> adds a slightly different file named E.  That's both a rename/rename
> (1to2) conflict for E & F, and an add/add conflict for E.  Users
> checks out this conflicted commit and resolves textual conflict in E
> (in favor of the "other side"), and does a "git add E", marking it as
> resolved.  When they go to commit, we not only need to worry about
> making sure a conflict for F appears, we also need to figure out how
> to adjust the tree such that the merge result gives you the expected
> value in E without affecting F.  How can that be done?
>
> On the first two bullet points, there's no such thing as a reverse
> mapping from conflicted files to original files from previous commits
> in current Git.  Creating one, if possible, would be a fair amount of
> work.  But, I'm not so sure it's even possible, due to the fact that
> conflicts and files do not always have one-to-one (or even one-to-many
> or many-to-one) relationships; many-to-many relationship can exist, as
> I've started alluding to in the last two bullet points (see also
> https://github.com/git/git/blob/98009afd24e2304bf923a64750340423473809ff/Documentation/git-merge-tree.txt#L266-L271).
> In fact, they can get even more complicated (e.g.
> https://github.com/git/git/blob/master/t/t6422-merge-rename-corner-cases.sh#L1017-L1022).

Great questions! We don't have support for renames, so we haven't had
to worry about these things. We have talked a little about divergent
renames and the need for recording that in the commit so we can tell
the user about it and maybe ask them which name they want to keep. I
had not considered the interaction with partial conflict resolution,
so thanks for bringing that up. I don't have any answers now, but
we'll probably need to start thinking about this soon.

> > > > But we'd also have to be careful and think through usecases, including
> > > > in the surrounding community.  People would probably want to ensure
> > > > that e.g. "Protected" or "Integration" branches don't get accept
> > > > fetches or pushes of conflicted commits,
> > >
> > > I think this is a really important point, while it can be useful to
> > > share conflicts so they can be collaboratively resolved we don't want to
> > > propagate them into "stable" or production branches. I wonder how 'jj'
> > > handles this.
> >
> > Agreed. `jj git push` refuses to push commits with conflicts, because
> > it's very unlikely that the remote will be able to make any sense of
> > it. Our commit backend at Google does support conflicts, so users can
> > check out each other's conflicted commits there (except that we
> > haven't even started dogfooding yet).
>
> I'm curious to hear what happens when you do start dogfooding, on
> projects with many developers and which are jj-only.  Do commits with
> conflicts accidentally end up in mainline branches, or are there good
> ways to make sure they don't hit anything considered stable?

That won't happen at Google because our source of truth for "merged
PRs" (in GitHub-speak) is in our existing VCS. We will necessarily
have to translate from jj's data model to its data model before a
commit can even be sent for review.

>
> > > > git status would probably
> > > > need some special warnings or notices, git checkout would probably
> > > > benefit from additional warnings/notices checks for those cases, git
> > > > log should probably display conflicted commits differently, we'd need
> > > > to add special handling for higher order conflicts (e.g. a merge with
> > > > conflicts is itself involved in a merge) probably similar to what jj
> > > > has done, and audit a lot of other code paths to see what would be
> > > > needed.
> > >
> > > As you point out there is a lot more to this than just being able to
> > > store the conflict data in a commit - in many ways I think that is the
> > > easiest part of the solution to sharing conflicts.
> >
> > Yes, I think it would be a very large project. Unlike jj, Git of
> > course has to worry about backwards compatibility. For example, you
> > would have to decide if your goal - even in the long term - is to make
> > `git rebase` etc. not get interrupted due to conflicts.
>
> ...and whether to copy jj's other feature in this area in some form:
> auto-rebasing any descendants when you checkout and amend an old
> commit (e.g. to resolve conflicts).  :-)

^ permalink raw reply

* Re: [PATCH 8/9] for-each-ref: add option to fully dereference tags
From: Victoria Dye @ 2023-11-08 18:02 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Patrick Steinhardt, Victoria Dye via GitGitGadget, git
In-Reply-To: <xmqq4jhx7x8l.fsf@gitster.g>

Junio C Hamano wrote:
> Victoria Dye <vdye@github.com> writes:
> 
>> I think `^{}fieldname` would be a good candidate, but it's *extremely*
> 
> Gaah.  Why?  fieldname^{} I may understand, but in the prefix form?

'fieldname^{}' seemed like more of a misuse of "^{}" than the prefixed form,
since we're not peeling "fieldname" but instead getting the value of
"fieldname" from the peeled tag. But then we're not dereferencing
"fieldname" in '*fieldname' either, so 'fieldname^{}' is no worse than what
already exists.

> 
> In any case, has anybody considered that we may be better off to
> declare that "*field" peeling a tag only once is a longstanding bug?
> 
> IOW, can we not add "fully peel" command line option or a new syntax
> and instead just "fix" the bug to fully peel when "*field" is asked
> for?

I'd certainly prefer that from a technical standpoint; it simplifies this
patch if I can just replace 'get_tagged_oid' with 'peel_iterated_oid'. The
two things that make me hesitate are:

1. There isn't a straightforward 1:1 substitute available for getting info
   on the immediate target of a list of tags. 
2. The performance of a recursive peel can be worse than that of a single
   tag dereference, since (unless the formatting is done in a ref_iterator
   iteration *and* the tag is a packed ref) the dereferenced object needs to
   be resolved to determine whether it's another tag or not.

#1 may not be an issue in practice, but I don't have enough information on
how applications use that formatting atom to say for sure. #2 is a bigger
issue, IMO, since one of the goals of this series was to improve performance
for some cases of 'for-each-ref' without hurting it in others.

> An application that cares about handling a chain of annotatetd tags
> would want to be able to say "this is the outermost tag's
> information; one level down, the tag was signed by this person;
> another level down, the tag was signed by this person, etc."  which
> would mean either
> 
>  * we have a syntax that shows the information from all levels
>    (e.g., "**taggername" may say "Victoria\nPatrick\nGitster")
> 
>  * we have a syntax that allows to specify how many levels to peel,
>    (e.g., "*0*taggername" may be the same as "taggername",
>    "*1*taggername" may be the same as "*taggername") plus some
>    programming construct like variables and loops.
> 
> but the repertoire being proposed that consists only of "peel only
> once" and "peel all levels" is way too insufficient.
> 
> Note that I do not advocate for allowing inspection of each levels
> separately.  Quite the contrary.  I would say that --format=<>
> placeholder should not be a programming language to satisify such a
> niche need.  And my conclusion from that stance is "peel once" plus
> "peel all" are already one level too many, and "peel once" was a
> very flawed implementation from day one, when 9f613ddd (Add
> git-for-each-ref: helper for language bindings, 2006-09-15)
> introduced it.

I can (and would like to) deprecate the "peel once" behavior and replace it
with "peel all", but with how long it's been around and the potential
performance impact, such a change should probably be clearly communicated.
How that happens depends on how aggressively we want to cut over. We could:

1. Change the behavior of '*' from single dereference to recursive
   dereference, make a note of it in the documentation.
2. Same as #1, but also add an option like '--no-recursive-dereference' or
   something to use the old behavior. Remove the option after 1-2 release
   cycles?
3. Add a new format specifier '^{}', note that '*' is deprecated in the
   docs.
4. Same as #3, but also show a warning/advice if '*' is used.
5. Same as #3, but die() if '*' is used.

I'm open to other options, those were just the first few I could think of. 


^ permalink raw reply

* Re: [PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks
From: Junio C Hamano @ 2023-11-08 17:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Patrick Steinhardt, git
In-Reply-To: <20231108172125.GD1028115@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> ... But the more robust way to do it is:
>
>   # export ORIG_PATH, which presumably is not cleared the same way PATH
>   # is, so that the hook can access it
>   ORIG_PATH=$PATH &&
>   export ORIG_PATH &&
>
>   cat >"$hook" <<EOF
>   # pull the original path from the caller
>   PATH=$ORIG_PATH
>   export PATH
>
>   ...do other stuff...
>   EOF
>
> That's assuming that environment variables make it intact to the hook at
> all (it is not clear to me why the original $PATH doesn't).

Yeah, the parenthetical comment points at the crux of the issue.  I
can tell from the patch what issue the platform throws at us we are
trying to work around, but it is frustrating not to know why the
platform does such unpleasant things in the first place.

Thanks.

^ permalink raw reply

* What's cooking in git.git (Nov 2023, #04; Thu, 9)
From: Junio C Hamano @ 2023-11-08 17:40 UTC (permalink / raw)
  To: git

Here are the topics that have been cooking in my tree.  Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release).  Commits prefixed with '-' are only in 'seen', and
aren't considered "accepted" at all and may be annotated with an URL
to a message that raises issues but they are no means exhaustive.  A
topic without enough support may be discarded after a long period of
no activity (of course they can be resubmit when new interests
arise).

The first candidate for the upcoming release, Git 2.43.0-rc0, has
been tagged.  As of now, 'next' is "empty" in the sense that its
tree is identical to that of 'master'.  There are tons of topic
waiting outside 'next' without sufficient support, which is sad, but
they are of lower priority now for a few weeks X-<.

Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors.  Some
repositories have only a subset of branches.

With maint, master, next, seen, todo:

	git://git.kernel.org/pub/scm/git/git.git/
	git://repo.or.cz/alt-git.git/
	https://kernel.googlesource.com/pub/scm/git/git/
	https://github.com/git/git/
	https://gitlab.com/git-vcs/git/

With all the integration branches and topics broken out:

	https://github.com/gitster/git/

Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):

	git://git.kernel.org/pub/scm/git/git-htmldocs.git/
	https://github.com/gitster/git-htmldocs.git/

Release tarballs are available at:

	https://www.kernel.org/pub/software/scm/git/

--------------------------------------------------
[Graduated to 'master']

* an/clang-format-typofix (2023-11-01) 1 commit
  (merged to 'next' on 2023-11-02 at 7f639690ab)
 + clang-format: fix typo in comment

 Typofix.
 source: <pull.1602.v2.git.git.1698759629166.gitgitgadget@gmail.com>


* bc/merge-file-object-input (2023-11-02) 2 commits
  (merged to 'next' on 2023-11-02 at ccbba9416c)
 + merge-file: add an option to process object IDs
 + git-merge-file doc: drop "-file" from argument placeholders

 "git merge-file" learns a mode to read three contents to be merged
 from blob objects.
 source: <20231101192419.794162-1-sandals@crustytoothpaste.net>


* jc/test-i18ngrep (2023-11-02) 2 commits
  (merged to 'next' on 2023-11-03 at 78406f8d94)
 + tests: teach callers of test_i18ngrep to use test_grep
 + test framework: further deprecate test_i18ngrep

 Another step to deprecate test_i18ngrep.
 source: <20231031052330.3762989-1-gitster@pobox.com>


* jk/chunk-bounds (2023-11-04) 1 commit
  (merged to 'next' on 2023-11-06 at ae9fbc1700)
 + t: avoid perl's pack/unpack "Q" specifier

 Test portability fix.
 source: <20231103162019.GB1470570@coredump.intra.peff.net>


* jk/tree-name-and-depth-limit (2023-11-02) 1 commit
  (merged to 'next' on 2023-11-06 at 041423344c)
 + max_tree_depth: lower it for MSVC to avoid stack overflows

 Further limit tree depth max to avoid Windows build running out of
 the stack space.
 source: <pull.1604.v2.git.1698843810814.gitgitgadget@gmail.com>


* js/ci-use-macos-13 (2023-11-03) 1 commit
  (merged to 'next' on 2023-11-06 at f7406347cd)
 + ci: upgrade to using macos-13

 Replace macos-12 used at GitHub CI with macos-13.
 source: <pull.1607.git.1698996455218.gitgitgadget@gmail.com>


* kn/rev-list-missing-fix (2023-11-01) 4 commits
  (merged to 'next' on 2023-11-02 at 2469dfc402)
 + rev-list: add commit object support in `--missing` option
 + rev-list: move `show_commit()` to the bottom
 + revision: rename bit to `do_not_die_on_missing_objects`
 + Merge branch 'ps/do-not-trust-commit-graph-blindly-for-existence' into kn/rev-list-missing-fix
 (this branch uses ps/do-not-trust-commit-graph-blindly-for-existence.)

 "git rev-list --missing" did not work for missing commit objects,
 which has been corrected.
 source: <20231026101109.43110-1-karthik.188@gmail.com>


* la/strvec-header-fix (2023-11-03) 1 commit
  (merged to 'next' on 2023-11-03 at db23d8a911)
 + strvec: drop unnecessary include of hex.h

 Code clean-up.
 source: <pull.1608.git.1698958277454.gitgitgadget@gmail.com>


* ps/do-not-trust-commit-graph-blindly-for-existence (2023-11-01) 2 commits
  (merged to 'next' on 2023-11-01 at 06037376ee)
 + commit: detect commits that exist in commit-graph but not in the ODB
 + commit-graph: introduce envvar to disable commit existence checks
 (this branch is used by kn/rev-list-missing-fix.)

 The codepath to traverse the commit-graph learned to notice that a
 commit is missing (e.g., corrupt repository lost an object), even
 though it knows something about the commit (like its parents) from
 what is in commit-graph.
 source: <cover.1698736363.git.ps@pks.im>


* ps/leakfixes (2023-11-07) 4 commits
  (merged to 'next' on 2023-11-08 at 1969726a2f)
 + setup: fix leaking repository format
 + setup: refactor `upgrade_repository_format()` to have common exit
 + shallow: fix memory leak when registering shallow roots
 + test-bloom: stop setting up Git directory twice

 Leakfix.
 source: <cover.1699267422.git.ps@pks.im>


* ps/show-ref (2023-11-01) 12 commits
  (merged to 'next' on 2023-11-02 at 987bb117f5)
 + t: use git-show-ref(1) to check for ref existence
 + builtin/show-ref: add new mode to check for reference existence
 + builtin/show-ref: explicitly spell out different modes in synopsis
 + builtin/show-ref: ensure mutual exclusiveness of subcommands
 + builtin/show-ref: refactor options for patterns subcommand
 + builtin/show-ref: stop using global vars for `show_one()`
 + builtin/show-ref: stop using global variable to count matches
 + builtin/show-ref: refactor `--exclude-existing` options
 + builtin/show-ref: fix dead code when passing patterns
 + builtin/show-ref: fix leaking string buffer
 + builtin/show-ref: split up different subcommands
 + builtin/show-ref: convert pattern to a local variable
 (this branch is used by ps/ref-tests-update.)

 Teach "git show-ref" a mode to check the existence of a ref.
 source: <cover.1698739941.git.ps@pks.im>


* tb/format-pack-doc-update (2023-11-01) 2 commits
  (merged to 'next' on 2023-11-02 at 538991fe9b)
 + Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
 + Documentation/gitformat-pack.txt: fix typo

 Doc update.
 source: <cover.1698780244.git.me@ttaylorr.com>


* tb/rev-list-unpacked-fix (2023-11-07) 2 commits
  (merged to 'next' on 2023-11-08 at 4b73bc0256)
 + pack-bitmap: drop --unpacked non-commit objects from results
 + list-objects: drop --unpacked non-commit objects from results

 "git rev-list --unpacked --objects" failed to exclude packed
 non-commit objects, which has been corrected.
 source: <cover.1699311386.git.me@ttaylorr.com>

--------------------------------------------------
[Stalled]

* pw/rebase-sigint (2023-09-07) 1 commit
 - rebase -i: ignore signals when forking subprocesses

 If the commit log editor or other external programs (spawned via
 "exec" insn in the todo list) receive internactive signal during
 "git rebase -i", it caused not just the spawned program but the
 "Git" process that spawned them, which is often not what the end
 user intended.  "git" learned to ignore SIGINT and SIGQUIT while
 waiting for these subprocesses.

 Expecting a reroll.
 cf. <12c956ea-330d-4441-937f-7885ab519e26@gmail.com>
 source: <pull.1581.git.1694080982621.gitgitgadget@gmail.com>


* tk/cherry-pick-sequence-requires-clean-worktree (2023-06-01) 1 commit
 - cherry-pick: refuse cherry-pick sequence if index is dirty

 "git cherry-pick A" that replays a single commit stopped before
 clobbering local modification, but "git cherry-pick A..B" did not,
 which has been corrected.

 Expecting a reroll.
 cf. <999f12b2-38d6-f446-e763-4985116ad37d@gmail.com>
 source: <pull.1535.v2.git.1685264889088.gitgitgadget@gmail.com>


* jc/diff-cached-fsmonitor-fix (2023-09-15) 3 commits
 - diff-lib: fix check_removed() when fsmonitor is active
 - Merge branch 'jc/fake-lstat' into jc/diff-cached-fsmonitor-fix
 - Merge branch 'js/diff-cached-fsmonitor-fix' into jc/diff-cached-fsmonitor-fix
 (this branch uses jc/fake-lstat.)

 The optimization based on fsmonitor in the "diff --cached"
 codepath is resurrected with the "fake-lstat" introduced earlier.

 It is unknown if the optimization is worth resurrecting, but in case...
 source: <xmqqr0n0h0tw.fsf@gitster.g>

--------------------------------------------------
[Cooking]

* vd/for-each-ref-unsorted-optimization (2023-11-07) 9 commits
 - t/perf: add perf tests for for-each-ref
 - for-each-ref: add option to fully dereference tags
 - ref-filter.c: filter & format refs in the same callback
 - ref-filter.c: refactor to create common helper functions
 - ref-filter.h: add functions for filter/format & format-only
 - ref-filter.h: move contains caches into filter
 - ref-filter.h: add max_count and omit_empty to ref_format
 - for-each-ref: clarify interaction of --omit-empty & --count
 - ref-filter.c: really don't sort when using --no-sort

 "git for-each-ref --no-sort" still sorted the refs alphabetically
 which paid non-trivial cost.  It has been redefined to show output
 in an unspecified order, to allow certain optimizations to take
 advantage of.

 Expecting a reroll.
 cf. <dbcbcf0e-aeee-4bb9-9e39-e2e85194d083@github.com>
 source: <pull.1609.git.1699320361.gitgitgadget@gmail.com>


* jw/git-add-attr-pathspec (2023-11-04) 1 commit
 - attr: enable attr pathspec magic for git-add and git-stash

 "git add" and "git stash" learned to support the ":(attr:...)"
 magic pathspec.

 Will merge to 'next'?
 source: <20231103163449.1578841-1-jojwang@google.com>


* jc/strbuf-comment-line-char (2023-11-01) 4 commits
 - strbuf: move env-using functions to environment.c
 - strbuf: make add_lines() public
 - strbuf_add_commented_lines(): drop the comment_line_char parameter
 - strbuf_commented_addf(): drop the comment_line_char parameter

 Code simplification.
 source: <cover.1698791220.git.jonathantanmy@google.com>


* ps/ci-gitlab (2023-11-02) 8 commits
 - ci: add support for GitLab CI
 - ci: install test dependencies for linux-musl
 - ci: squelch warnings when testing with unusable Git repo
 - ci: unify setup of some environment variables
 - ci: split out logic to set up failed test artifacts
 - ci: group installation of Docker dependencies
 - ci: make grouping setup more generic
 - ci: reorder definitions for grouping functions

 Add support for GitLab CI.

 Comments?
 source: <cover.1698843660.git.ps@pks.im>


* ps/ref-tests-update (2023-11-03) 10 commits
 - t: mark several tests that assume the files backend with REFFILES
 - t7900: assert the absence of refs via git-for-each-ref(1)
 - t7300: assert exact states of repo
 - t4207: delete replace references via git-update-ref(1)
 - t1450: convert tests to remove worktrees via git-worktree(1)
 - t: convert tests to not access reflog via the filesystem
 - t: convert tests to not access symrefs via the filesystem
 - t: convert tests to not write references via the filesystem
 - t: allow skipping expected object ID in `ref-store update-ref`
 - Merge branch 'ps/show-ref' into ps/ref-tests-update

 Update ref-related tests.

 Comments?
 source: <cover.1698914571.git.ps@pks.im>


* jx/fetch-atomic-error-message-fix (2023-10-19) 2 commits
 - fetch: no redundant error message for atomic fetch
 - t5574: test porcelain output of atomic fetch

 "git fetch --atomic" issued an unnecessary empty error message,
 which has been corrected.

 Needs review.
 source: <ced46baeb1c18b416b4b4cc947f498bea2910b1b.1697725898.git.zhiyou.jx@alibaba-inc.com>


* js/bugreport-in-the-same-minute (2023-10-16) 1 commit
 - bugreport: include +i in outfile suffix as needed

 Instead of auto-generating a filename that is already in use for
 output and fail the command, `git bugreport` learned to fuzz the
 filename to avoid collisions with existing files.

 Expecting a reroll.
 cf. <ZTtZ5CbIGETy1ucV.jacob@initialcommit.io>
 source: <20231016214045.146862-2-jacob@initialcommit.io>


* kh/t7900-cleanup (2023-10-17) 9 commits
 - t7900: fix register dependency
 - t7900: factor out packfile dependency
 - t7900: fix `print-args` dependency
 - t7900: fix `pfx` dependency
 - t7900: factor out common schedule setup
 - t7900: factor out inheritance test dependency
 - t7900: create commit so that branch is born
 - t7900: setup and tear down clones
 - t7900: remove register dependency

 Test clean-up.

 Perhaps discard?
 cf. <655ca147-c214-41be-919d-023c1b27b311@app.fastmail.com>
 source: <cover.1697319294.git.code@khaugsbakk.name>


* tb/merge-tree-write-pack (2023-10-23) 5 commits
 - builtin/merge-tree.c: implement support for `--write-pack`
 - bulk-checkin: introduce `index_tree_bulk_checkin_incore()`
 - bulk-checkin: introduce `index_blob_bulk_checkin_incore()`
 - bulk-checkin: generify `stream_blob_to_pack()` for arbitrary types
 - bulk-checkin: extract abstract `bulk_checkin_source`

 "git merge-tree" learned "--write-pack" to record its result
 without creating loose objects.

 Comments?
 source: <cover.1698101088.git.me@ttaylorr.com>


* tb/pair-chunk-expect-size (2023-10-14) 8 commits
 - midx: read `OOFF` chunk with `pair_chunk_expect()`
 - midx: read `OIDL` chunk with `pair_chunk_expect()`
 - midx: read `OIDF` chunk with `pair_chunk_expect()`
 - commit-graph: read `BIDX` chunk with `pair_chunk_expect()`
 - commit-graph: read `GDAT` chunk with `pair_chunk_expect()`
 - commit-graph: read `CDAT` chunk with `pair_chunk_expect()`
 - commit-graph: read `OIDF` chunk with `pair_chunk_expect()`
 - chunk-format: introduce `pair_chunk_expect()` helper

 Code clean-up for jk/chunk-bounds topic.

 Comments?
 source: <45cac29403e63483951f7766c6da3c022c68d9f0.1697225110.git.me@ttaylorr.com>
 source: <cover.1697225110.git.me@ttaylorr.com>


* tb/path-filter-fix (2023-10-18) 17 commits
 - bloom: introduce `deinit_bloom_filters()`
 - commit-graph: reuse existing Bloom filters where possible
 - object.h: fix mis-aligned flag bits table
 - commit-graph: drop unnecessary `graph_read_bloom_data_context`
 - commit-graph.c: unconditionally load Bloom filters
 - bloom: prepare to discard incompatible Bloom filters
 - bloom: annotate filters with hash version
 - commit-graph: new filter ver. that fixes murmur3
 - repo-settings: introduce commitgraph.changedPathsVersion
 - t4216: test changed path filters with high bit paths
 - t/helper/test-read-graph: implement `bloom-filters` mode
 - bloom.h: make `load_bloom_filter_from_graph()` public
 - t/helper/test-read-graph.c: extract `dump_graph_info()`
 - gitformat-commit-graph: describe version 2 of BDAT
 - commit-graph: ensure Bloom filters are read with consistent settings
 - revision.c: consult Bloom filters for root commits
 - t/t4216-log-bloom.sh: harden `test_bloom_filters_not_used()`

 The Bloom filter used for path limited history traversal was broken
 on systems whose "char" is unsigned; update the implementation and
 bump the format version to 2.

 Needs (hopefully final and quick) review.
 source: <cover.1697653929.git.me@ttaylorr.com>


* cc/git-replay (2023-11-03) 14 commits
 - replay: stop assuming replayed branches do not diverge
 - replay: add --contained to rebase contained branches
 - replay: add --advance or 'cherry-pick' mode
 - replay: use standard revision ranges
 - replay: make it a minimal server side command
 - replay: remove HEAD related sanity check
 - replay: remove progress and info output
 - replay: add an important FIXME comment about gpg signing
 - replay: change rev walking options
 - replay: introduce pick_regular_commit()
 - replay: die() instead of failing assert()
 - replay: start using parse_options API
 - replay: introduce new builtin
 - t6429: remove switching aspects of fast-rebase

 Introduce "git replay", a tool meant on the server side without
 working tree to recreate a history.

 Comments?
 source: <20231102135151.843758-1-christian.couder@gmail.com>


* ak/color-decorate-symbols (2023-10-23) 7 commits
 - log: add color.decorate.pseudoref config variable
 - refs: exempt pseudorefs from pattern prefixing
 - refs: add pseudorefs array and iteration functions
 - log: add color.decorate.ref config variable
 - log: add color.decorate.symbol config variable
 - log: use designated inits for decoration_colors
 - config: restructure color.decorate documentation

 A new config for coloring.

 Needs review.
 source: <20231023221143.72489-1-andy.koppe@gmail.com>


* js/update-urls-in-doc-and-comment (2023-09-26) 4 commits
 - doc: refer to internet archive
 - doc: update links for andre-simon.de
 - doc: update links to current pages
 - doc: switch links to https

 Stale URLs have been updated to their current counterparts (or
 archive.org) and HTTP links are replaced with working HTTPS links.

 Needs review.
 source: <pull.1589.v2.git.1695553041.gitgitgadget@gmail.com>


* la/trailer-cleanups (2023-10-20) 3 commits
 - trailer: use offsets for trailer_start/trailer_end
 - trailer: find the end of the log message
 - commit: ignore_non_trailer computes number of bytes to ignore

 Code clean-up.

 Comments?
 source: <pull.1563.v5.git.1697828495.gitgitgadget@gmail.com>


* eb/hash-transition (2023-10-02) 30 commits
 - t1016-compatObjectFormat: add tests to verify the conversion between objects
 - t1006: test oid compatibility with cat-file
 - t1006: rename sha1 to oid
 - test-lib: compute the compatibility hash so tests may use it
 - builtin/ls-tree: let the oid determine the output algorithm
 - object-file: handle compat objects in check_object_signature
 - tree-walk: init_tree_desc take an oid to get the hash algorithm
 - builtin/cat-file: let the oid determine the output algorithm
 - rev-parse: add an --output-object-format parameter
 - repository: implement extensions.compatObjectFormat
 - object-file: update object_info_extended to reencode objects
 - object-file-convert: convert commits that embed signed tags
 - object-file-convert: convert commit objects when writing
 - object-file-convert: don't leak when converting tag objects
 - object-file-convert: convert tag objects when writing
 - object-file-convert: add a function to convert trees between algorithms
 - object: factor out parse_mode out of fast-import and tree-walk into in object.h
 - cache: add a function to read an OID of a specific algorithm
 - tag: sign both hashes
 - commit: export add_header_signature to support handling signatures on tags
 - commit: convert mergetag before computing the signature of a commit
 - commit: write commits for both hashes
 - object-file: add a compat_oid_in parameter to write_object_file_flags
 - object-file: update the loose object map when writing loose objects
 - loose: compatibilty short name support
 - loose: add a mapping between SHA-1 and SHA-256 for loose objects
 - repository: add a compatibility hash algorithm
 - object-names: support input of oids in any supported hash
 - oid-array: teach oid-array to handle multiple kinds of oids
 - object-file-convert: stubs for converting from one object format to another

 Teach a repository to work with both SHA-1 and SHA-256 hash algorithms.

 Needs review.
 source: <878r8l929e.fsf@gmail.froward.int.ebiederm.org>


* jx/remote-archive-over-smart-http (2023-10-04) 4 commits
 - archive: support remote archive from stateless transport
 - transport-helper: call do_take_over() in connect_helper
 - transport-helper: call do_take_over() in process_connect
 - transport-helper: no connection restriction in connect_helper

 "git archive --remote=<remote>" learned to talk over the smart
 http (aka stateless) transport.

 Needs review.
 source: <cover.1696432593.git.zhiyou.jx@alibaba-inc.com>


* jx/sideband-chomp-newline-fix (2023-10-04) 3 commits
 - pkt-line: do not chomp newlines for sideband messages
 - pkt-line: memorize sideband fragment in reader
 - test-pkt-line: add option parser for unpack-sideband

 Sideband demultiplexer fixes.

 Needs review.
 source: <cover.1696425168.git.zhiyou.jx@alibaba-inc.com>


* js/config-parse (2023-09-21) 5 commits
 - config-parse: split library out of config.[c|h]
 - config.c: accept config_parse_options in git_config_from_stdin
 - config: report config parse errors using cb
 - config: split do_event() into start and flush operations
 - config: split out config_parse_options

 The parsing routines for the configuration files have been split
 into a separate file.

 Needs review.
 source: <cover.1695330852.git.steadmon@google.com>


* jc/fake-lstat (2023-09-15) 1 commit
 - cache: add fake_lstat()
 (this branch is used by jc/diff-cached-fsmonitor-fix.)

 A new helper to let us pretend that we called lstat() when we know
 our cache_entry is up-to-date via fsmonitor.

 Needs review.
 source: <xmqqcyykig1l.fsf@gitster.g>


* js/doc-unit-tests (2023-11-02) 3 commits
 - ci: run unit tests in CI
 - unit tests: add TAP unit test framework
 - unit tests: add a project plan document
 (this branch is used by js/doc-unit-tests-with-cmake.)

 Process to add some form of low-level unit tests has started.

 Will merge to 'next'?
 source: <cover.1698881249.git.steadmon@google.com>


* js/doc-unit-tests-with-cmake (2023-11-02) 7 commits
 - cmake: handle also unit tests
 - cmake: use test names instead of full paths
 - cmake: fix typo in variable name
 - artifacts-tar: when including `.dll` files, don't forget the unit-tests
 - unit-tests: do show relative file paths
 - unit-tests: do not mistake `.pdb` files for being executable
 - cmake: also build unit tests
 (this branch uses js/doc-unit-tests.)

 Update the base topic to work with CMake builds.

 Will merge to 'next'?
 source: <pull.1579.v3.git.1695640836.gitgitgadget@gmail.com>


* jc/rerere-cleanup (2023-08-25) 4 commits
 - rerere: modernize use of empty strbuf
 - rerere: try_merge() should use LL_MERGE_ERROR when it means an error
 - rerere: fix comment on handle_file() helper
 - rerere: simplify check_one_conflict() helper function

 Code clean-up.

 Not ready to be reviewed yet.
 source: <20230824205456.1231371-1-gitster@pobox.com>


* rj/status-bisect-while-rebase (2023-10-16) 1 commit
 - status: fix branch shown when not only bisecting

 "git status" is taught to show both the branch being bisected and
 being rebased when both are in effect at the same time.

 Needs review.
 source: <2e24ca9b-9c5f-f4df-b9f8-6574a714dfb2@gmail.com>

^ permalink raw reply

* [ANNOUNCE] Git v2.43.0-rc1
From: Junio C Hamano @ 2023-11-08 17:33 UTC (permalink / raw)
  To: git; +Cc: Linux Kernel, git-packagers

A release candidate Git v2.43.0-rc1 is now available for testing at
the usual places.  It is comprised of 449 non-merge commits since
v2.42.0, contributed by 71 people, 17 of which are new faces [*].

The tarballs are found at:

    https://www.kernel.org/pub/software/scm/git/testing/

The following public repositories all have a copy of the
'v2.43.0-rc1' tag and the 'master' branch that the tag points at:

  url = https://git.kernel.org/pub/scm/git/git
  url = https://kernel.googlesource.com/pub/scm/git/git
  url = git://repo.or.cz/alt-git.git
  url = https://github.com/gitster/git

New contributors whose contributions weren't in v2.42.0 are as follows.
Welcome to the Git development community!

  Aditya Neelamraju, Alyssa Ross, Caleb Hill, Dorcas AnonoLitunya,
  Dragan Simic, Isoken June Ibizugbe, Jan Alexander Steffens
  (heftig), Javier Mora, ks1322 ks1322, Mark Ruvald Pedersen,
  Matthew McClain, Naomi Ibe, Romain Chossart, Tang Yuyi, Vipul
  Kumar, 王常新, and 谢致邦 (XIE Zhibang).

Returning contributors who helped this release are as follows.
Thanks for your continued support.

  Ævar Arnfjörð Bjarmason, Andrei Rybak, Andy Koppe, Bagas
  Sanjaya, Beat Bolli, brian m. carlson, Calvin Wan, Christian
  Couder, Christian Hesse, Derrick Stolee, Drew DeVault, Elijah
  Newren, Emily Shaffer, Eric W. Biederman, Eric Wong, Evan
  Gates, Han Young, Hariom Verma, Jacob Abel, Jacob Stopak,
  Jason Hatton, Jeff King, Johannes Schindelin, John Cai,
  Josh Soref, Josip Sokcevic, Junio C Hamano, Karthik Nayak,
  Kousik Sanagavarapu, Kristoffer Haugsbakk, Linus Arver, Mark
  Levedahl, Martin Ågren, Martin Storsjö, M Hickford, Michael
  Strawbridge, Michal Suchanek, Oswald Buddenhagen, Patrick
  Steinhardt, Philippe Blain, Phillip Wood, Randall S. Becker,
  René Scharfe, Robert Coup, Rubén Justo, Sergey Organov, Shuqi
  Liang, Stefan Haller, Štěpán Němec, Taylor Blau, Teng Long,
  Todd Zullinger, Victoria Dye, and Wesley Schwengle.

[*] We are counting not just the authorship contribution but issue
    reporting, mentoring, helping and reviewing that are recorded in
    the commit trailers.

----------------------------------------------------------------

Git v2.43 Release Notes (draft)
===============================

Backward Compatibility Notes

 * The "--rfc" option of "git format-patch" used to be a valid way to
   override an earlier "--subject-prefix=<something>" on the command
   line and replace it with "[RFC PATCH]", but from this release, it
   merely prefixes the string "RFC " in front of the given subject
   prefix.  If you are negatively affected by this change, please use
   "--subject-prefix=PATCH --rfc" as a replacement.

 * "git rev-list --stdin" learned to take non-revisions (like "--not")
   recently from the standard input, but the way such a "--not" was
   handled was quite confusing, which has been rethought.  The updated
   rule is that "--not" given from the command line only affects revs
   given from the command line that comes but not revs read from the
   standard input, and "--not" read from the standard input affects
   revs given from the standard input and not revs given from the
   command line.

UI, Workflows & Features

 * A message written in olden time prevented a branch from getting
   checked out saying it is already checked out elsewhere, but these
   days, we treat a branch that is being bisected or rebased just like
   a branch that is checked out and protect it.  Rephrase the message
   to say that the branch is in use.

 * Hourly and other schedule of "git maintenance" jobs are randomly
   distributed now.

 * "git cmd -h" learned to signal which options can be negated by
   listing such options like "--[no-]opt".

 * The way authentication related data other than passwords (e.g.
   oath token and password expiration data) are stored in libsecret
   keyrings has been rethought.

 * Update the libsecret and wincred credential helpers to correctly
   match which credential to erase; they erased the wrong entry in
   some cases.

 * Git GUI updates.

 * "git format-patch" learns a way to feed cover letter description,
   that (1) can be used on detached HEAD where there is no branch
   description available, and (2) also can override the branch
   description if there is one.

 * Use of --max-pack-size to allow multiple packfiles to be created is
   now supported even when we are sending unreachable objects to cruft
   packs.

 * "git format-patch --rfc --subject-prefix=<foo>" used to ignore the
   "--subject-prefix" option and used "[RFC PATCH]"; now we will add
   "RFC" prefix to whatever subject prefix is specified.

 * "git log --format" has been taught the %(decorate) placeholder.

 * The default log message created by "git revert", when reverting a
   commit that records a revert, has been tweaked, to encourage people
   describe complex "revert of revert of revert" situation better in
   their own words.

 * The command-line completion support (in contrib/) learned to
   complete "git commit --trailer=" for possible trailer keys.

 * "git update-index" learns "--show-index-version" to inspect
   the index format version used by the on-disk index file.

 * "git diff" learned diff.statNameWidth configuration variable, to
   give the default width for the name part in the "--stat" output.

 * "git range-diff --notes=foo" compared "log --notes=foo --notes" of
   the two ranges, instead of using just the specified notes tree.

 * The command line completion script (in contrib/) can be told to
   complete aliases by including ": git <cmd> ;" in the alias to tell
   it that the alias should be completed similar to how "git <cmd>" is
   completed.  The parsing code for the alias as been loosened to
   allow ';' without an extra space before it.

 * "git for-each-ref" and friends learned to apply mailmap to
   authorname and other fields.

 * "git repack" machinery learns to pay attention to the "--filter="
   option.

 * "git repack" learned "--max-cruft-size" to prevent cruft packs from
   growing without bounds.

 * "git merge-tree" learned to take strategy backend specific options
   via the "-X" option, like "git merge" does.

 * "git log" and friends learned "--dd" that is a short-hand for
   "--diff-merges=first-parent -p".

 * The attribute subsystem learned to honor `attr.tree` configuration
   that specifies which tree to read the .gitattributes files from.

 * "git merge-file" learns a mode to read three contents to be merged
   from blob objects.


Performance, Internal Implementation, Development Support etc.

 * "git check-attr" has been taught to work better with sparse-index.

 * It may be tempting to leave the help text NULL for a command line
   option that is either hidden or too obvious, but "git subcmd -h"
   and "git subcmd --help-all" would have segfaulted if done so.  Now
   the help text is optional.

 * Tests that are known to pass with LSan are now marked as such.

 * Flaky "git p4" tests, as well as "git svn" tests, are now skipped
   in the (rather expensive) sanitizer CI job.

 * Tests with LSan from time to time seem to emit harmless message
   that makes our tests unnecessarily flaky; we work it around by
   filtering the uninteresting output.

 * Unused parameters to functions are marked as such, and/or removed,
   in order to bring us closer to -Wunused-parameter clean.

 * The code to keep track of existing packs in the repository while
   repacking has been refactored.

 * The "streaming" interface used for bulk-checkin codepath has been
   narrowed to take only blob objects for now, with no real loss of
   functionality.

 * GitHub CI workflow has learned to trigger Coverity check.

 * Test coverage for trailers has been improved.

 * The code to iterate over loose references have been optimized to
   reduce the number of lstat() system calls.

 * The codepaths that read "chunk" formatted files have been corrected
   to pay attention to the chunk size and notice broken files.

 * Replace macos-12 used at GitHub CI with macos-13.
   (merge 682a868f67 js/ci-use-macos-13 later to maint).


Fixes since v2.42
-----------------

 * Overly long label names used in the sequencer machinery are now
   chopped to fit under filesystem limitation.

 * Scalar updates.

 * Tweak GitHub Actions CI so that pushing the same commit to multiple
   branch tips at the same time will not waste building and testing
   the same thing twice.

 * The commit-graph verification code that detects mixture of zero and
   non-zero generation numbers has been updated.

 * "git diff -w --exit-code" with various options did not work
   correctly, which is being addressed.

 * transfer.unpackLimit ought to be used as a fallback, but overrode
   fetch.unpackLimit and receive.unpackLimit instead.

 * The use of API between two calls to require_clean_work_tree() from
   the sequencer code has been cleaned up for consistency.

 * "git diff --no-such-option" and other corner cases around the exit
   status of the "diff" command has been corrected.

 * "git for-each-ref --sort='contents:size'" sorts the refs according
   to size numerically, giving a ref that points at a blob twelve-byte
   (12) long before showing a blob hundred-byte (100) long.

 * We now limit depth of the tree objects and maximum length of
   pathnames recorded in tree objects.
   (merge 4d5693ba05 jk/tree-name-and-depth-limit later to maint).

 * Various fixes to the behavior of "rebase -i" when the command got
   interrupted by conflicting changes.

 * References from description of the `--patch` option in various
   manual pages have been simplified and improved.

 * "git grep -e A --no-or -e B" is accepted, even though the negation
   of "or" did not mean anything, which has been tightened.

 * The completion script (in contrib/) has been taught to treat the
   "-t" option to "git checkout" and "git switch" just like the
   "--track" option, to complete remote-tracking branches.

 * "git diff --no-index -R <(one) <(two)" did not work correctly,
   which has been corrected.

 * Update "git maintenance" timers' implementation based on systemd
   timers to work with WSL.

 * "git diff --cached" codepath did not fill the necessary stat
   information for a file when fsmonitor knows it is clean and ended
   up behaving as if it is not clean, which has been corrected.

 * Clarify how "alias.foo = : git cmd ; aliased-command-string" should
   be spelled with necessary whitespaces around punctuation marks to
   work.

 * HTTP Header redaction code has been adjusted for a newer version of
   cURL library that shows its traces differently from earlier
   versions.

 * An error message given by "git send-email" when given a malformed
   address did not give correct information, which has been corrected.

 * UBSan options were not propagated through the test framework to git
   run via the httpd, unlike ASan options, which has been corrected.

 * "checkout --merge -- path" and "update-index --unresolve path" did
   not resurrect conflicted state that was resolved to remove path,
   but now they do.
   (merge 5bdedac3c7 jc/unresolve-removal later to maint).

 * The display width table for unicode characters has been updated for
   Unicode 15.1
   (merge 872976c37e bb/unicode-width-table-15 later to maint).

 * Update mailmap entry for Derrick.
   (merge 6e5457d8c7 ds/mailmap-entry-update later to maint).

 * In .gitmodules files, submodules are keyed by their names, and the
   path to the submodule whose name is $name is specified by the
   submodule.$name.path variable.  There were a few codepaths that
   mixed the name and path up when consulting the submodule database,
   which have been corrected.  It took long for these bugs to be found
   as the name of a submodule initially is the same as its path, and
   the problem does not surface until it is moved to a different path,
   which apparently happens very rarely.

 * "git diff --merge-base X other args..." insisted that X must be a
   commit and errored out when given an annotated tag that peels to a
   commit, but we only need it to be a committish.  This has been
   corrected.
   (merge 4adceb5a29 ar/diff-index-merge-base-fix later to maint).

 * Fix "git merge-tree" to stop segfaulting when the --attr-source
   option is used.
   (merge e95bafc52f jc/merge-ort-attr-index-fix later to maint).

 * Unlike "git log --pretty=%D", "git log --pretty="%(decorate)" did
   not auto-initialize the decoration subsystem, which has been
   corrected.

 * Feeding "git stash store" with a random commit that was not created
   by "git stash create" now errors out.
   (merge d9b6634589 jc/fail-stash-to-store-non-stash later to maint).

 * The index file has room only for lower 32-bit of the file size in
   the cached stat information, which means cached stat information
   will have 0 in its sd_size member for a file whose size is multiple
   of 4GiB.  This is mistaken for a racily clean path.  Avoid it by
   storing a bogus sd_size value instead for such files.
   (merge 5143ac07b1 bc/racy-4gb-files later to maint).

 * "git p4" tried to store symlinks to LFS when told, but has been
   fixed not to do so, because it does not make sense.
   (merge 10c89a02b0 mm/p4-symlink-with-lfs later to maint).

 * The codepath to handle recipient addresses `git send-email
   --compose` learns from the user was completely broken, which has
   been corrected.
   (merge 3ec6167567 jk/send-email-fix-addresses-from-composed-messages later to maint).

 * "cd sub && git grep -f patterns" tried to read "patterns" file at
   the top level of the working tree; it has been corrected to read
   "sub/patterns" instead.


 * "git reflog expire --single-worktree" has been broken for the past
   20 months or so, which has been corrected.

 * "git send-email" did not have certain pieces of data computed yet
   when it tried to validate the outging messages and its recipient
   addresses, which has been sorted out.

 * "git bugreport" learned to complain when it received a command line
   argument that it will not use.

 * The codepath to traverse the commit-graph learned to notice that a
   commit is missing (e.g., corrupt repository lost an object), even
   though it knows something about the commit (like its parents) from
   what is in commit-graph.
   (merge 7a5d604443 ps/do-not-trust-commit-graph-blindly-for-existence later to maint).

 * "git rev-list --missing" did not work for missing commit objects,
   which has been corrected.

 * "git rev-list --unpacked --objects" failed to exclude packed
   non-commit objects, which has been corrected.
   (merge 7b3c8e9f38 tb/rev-list-unpacked-fix later to maint).

 * Other code cleanup, docfix, build fix, etc.
   (merge c2c349a15c xz/commit-title-soft-limit-doc later to maint).
   (merge 1bd809938a tb/format-pack-doc-update later to maint).
   (merge 8f81532599 an/clang-format-typofix later to maint).
   (merge 3ca86adc2d la/strvec-header-fix later to maint).
   (merge 6789275d37 jc/test-i18ngrep later to maint).
   (merge 9972cd6004 ps/leakfixes later to maint).

----------------------------------------------------------------

Changes since v2.42.0 are as follows:

Aditya Neelamraju (1):
      clang-format: fix typo in comment

Alyssa Ross (1):
      diff: fix --merge-base with annotated tags

Andrei Rybak (1):
      SubmittingPatches: call gitk's command "Copy commit reference"

Andy Koppe (8):
      pretty-formats: enclose options in angle brackets
      decorate: refactor format_decorations()
      decorate: avoid some unnecessary color overhead
      decorate: color each token separately
      pretty: add %(decorate[:<options>]) format
      pretty: add pointer and tag options to %(decorate)
      decorate: use commit color for HEAD arrow
      pretty: fix ref filtering for %(decorate) formats

Beat Bolli (1):
      unicode: update the width tables to Unicode 15.1

Caleb Hill (1):
      git-clean doc: fix "without do cleaning" typo

Calvin Wan (4):
      hex-ll: separate out non-hash-algo functions
      wrapper: reduce scope of remove_or_warn()
      config: correct bad boolean env value error message
      parse: separate out parsing functions from config.h

Christian Couder (9):
      pack-objects: allow `--filter` without `--stdout`
      t/helper: add 'find-pack' test-tool
      repack: refactor finishing pack-objects command
      repack: refactor finding pack prefix
      pack-bitmap-write: rebuild using new bitmap when remapping
      repack: add `--filter=<filter-spec>` option
      gc: add `gc.repackFilter` config option
      repack: implement `--filter-to` for storing filtered out objects
      gc: add `gc.repackFilterTo` config option

Christian Hesse (2):
      t/lib-gpg: forcibly run a trustdb update
      t/t6300: drop magic filtering

Derrick Stolee (13):
      upload-pack: fix race condition in error messages
      maintenance: add get_random_minute()
      maintenance: use random minute in launchctl scheduler
      maintenance: use random minute in Windows scheduler
      maintenance: use random minute in cron scheduler
      maintenance: swap method locations
      maintenance: use random minute in systemd scheduler
      maintenance: fix systemd schedule overlaps
      maintenance: update schedule before config
      scalar: add --[no-]src option
      setup: add discover_git_directory_reason()
      scalar reconfigure: help users remove buggy repos
      mailmap: change primary address for Derrick Stolee

Dorcas AnonoLitunya (1):
      t7601: use "test_path_is_file" etc. instead of "test -f"

Dragan Simic (2):
      diff --stat: add config option to limit filename width
      diff --stat: set the width defaults in a helper function

Drew DeVault (1):
      format-patch: --rfc honors what --subject-prefix sets

Elijah Newren (25):
      documentation: wording improvements
      documentation: fix small error
      documentation: fix typos
      documentation: fix apostrophe usage
      documentation: add missing words
      documentation: remove extraneous words
      documentation: fix subject/verb agreement
      documentation: employ consistent verb tense for a list
      documentation: fix verb tense
      documentation: fix adjective vs. noun
      documentation: fix verb vs. noun
      documentation: fix singular vs. plural
      documentation: whitespace is already generally plural
      documentation: fix choice of article
      documentation: add missing article
      documentation: remove unnecessary hyphens
      documentation: add missing hyphens
      documentation: use clearer prepositions
      documentation: fix punctuation
      documentation: fix capitalization
      documentation: fix whitespace issues
      documentation: add some commas where they are helpful
      documentation: add missing fullstops
      documentation: add missing quotes
      documentation: add missing parenthesis

Emily Shaffer (2):
      t0091-bugreport: stop using i18ngrep
      bugreport: reject positional arguments

Eric W. Biederman (1):
      bulk-checkin: only support blobs in index_bulk_checkin

Eric Wong (1):
      treewide: fix various bugs w/ OpenSSL 3+ EVP API

Evan Gates (1):
      git-config: fix misworded --type=path explanation

Han Young (1):
      show doc: redirect user to git log manual instead of git diff-tree

Isoken June Ibizugbe (1):
      builtin/branch.c: adjust error messages to coding guidelines

Jacob Abel (1):
      builtin/worktree.c: fix typo in "forgot fetch" msg

Jacob Stopak (1):
      Include gettext.h in MyFirstContribution tutorial

Jan Alexander Steffens (heftig) (6):
      submodule--helper: use submodule_from_path in set-{url,branch}
      submodule--helper: return error from set-url when modifying failed
      t7419: actually test the branch switching
      t7419, t7420: use test_cmp_config instead of grepping .gitmodules
      t7419: test that we correctly handle renamed submodules
      t7420: test that we correctly handle renamed submodules

Jason Hatton (1):
      Prevent git from rehashing 4GiB files

Javier Mora (2):
      git-status.txt: fix minor asciidoc format issue
      doc/git-bisect: clarify `git bisect run` syntax

Jeff King (114):
      hashmap: use expected signatures for comparison functions
      diff-files: avoid negative exit value
      diff: show usage for unknown builtin_diff_files() options
      diff: die when failing to read index in git-diff builtin
      diff: drop useless return from run_diff_{files,index} functions
      diff: drop useless return values in git-diff helpers
      diff: drop useless "status" parameter from diff_result_code()
      commit-graph: verify swapped zero/non-zero generation cases
      test-lib: ignore uninteresting LSan output
      sequencer: use repository parameter in short_commit_name()
      sequencer: mark repository argument as unused
      ref-filter: mark unused parameters in parser callbacks
      pack-bitmap: mark unused parameters in show_object callback
      worktree: mark unused parameters in each_ref_fn callback
      commit-graph: mark unused data parameters in generation callbacks
      ls-tree: mark unused parameter in callback
      stash: mark unused parameter in diff callback
      trace2: mark unused us_elapsed_absolute parameters
      trace2: mark unused config callback parameter
      test-trace2: mark unused argv/argc parameters
      grep: mark unused parameter in output function
      add-interactive: mark unused callback parameters
      negotiator/noop: mark unused callback parameters
      worktree: mark unused parameters in noop repair callback
      imap-send: mark unused parameters with NO_OPENSSL
      grep: mark unused parmaeters in pcre fallbacks
      credential: mark unused parameter in urlmatch callback
      fetch: mark unused parameter in ref_transaction callback
      bundle-uri: mark unused parameters in callbacks
      gc: mark unused descriptors in scheduler callbacks
      update-ref: mark unused parameter in parser callbacks
      ci: allow branch selection through "vars"
      ci: deprecate ci/config/allow-ref script
      merge: make xopts a strvec
      merge: simplify parsing of "-n" option
      format-patch: use OPT_STRING_LIST for to/cc options
      tree-walk: reduce stack size for recursive functions
      tree-walk: drop MAX_TRAVERSE_TREES macro
      tree-walk: rename "error" variable
      fsck: detect very large tree pathnames
      add core.maxTreeDepth config
      traverse_trees(): respect max_allowed_tree_depth
      read_tree(): respect max_allowed_tree_depth
      list-objects: respect max_allowed_tree_depth
      tree-diff: respect max_allowed_tree_depth
      lower core.maxTreeDepth default to 2048
      checkout-index: delay automatic setting of to_tempfile
      parse-options: prefer opt->value to globals in callbacks
      parse-options: mark unused "opt" parameter in callbacks
      merge: do not pass unused opt->value parameter
      parse-options: add more BUG_ON() annotations
      interpret-trailers: mark unused "unset" parameters in option callbacks
      parse-options: mark unused parameters in noop callback
      merge-ort: drop custom err() function
      merge-ort: stop passing "opt" to read_oid_strbuf()
      merge-ort: drop unused parameters from detect_and_process_renames()
      merge-ort: drop unused "opt" parameter from merge_check_renames_reusable()
      http: factor out matching of curl http/2 trace lines
      http: update curl http/2 info matching for curl 8.3.0
      merge-ort: lowercase a few error messages
      fsmonitor: prefer repo_git_path() to git_pathdup()
      fsmonitor/win32: drop unused parameters
      fsmonitor: mark some maybe-unused parameters
      fsmonitor/win32: mark unused parameter in fsm_os__incompatible()
      fsmonitor: mark unused parameters in stub functions
      fsmonitor/darwin: mark unused parameters in system callback
      fsmonitor: mark unused hashmap callback parameters
      run-command: mark unused parameters in start_bg_wait callbacks
      test-lib: set UBSAN_OPTIONS to match ASan
      commit-graph: factor out chain opening function
      commit-graph: check mixed generation validation when loading chain file
      t5324: harmonize sha1/sha256 graph chain corruption
      commit-graph: detect read errors when verifying graph chain
      commit-graph: tighten chain size check
      commit-graph: report incomplete chains during verification
      t6700: mark test as leak-free
      commit-reach: free temporary list in get_octopus_merge_bases()
      merge: free result of repo_get_merge_bases()
      commit-graph: move slab-clearing to close_commit_graph()
      commit-graph: free all elements of graph chain
      commit-graph: delay base_graph assignment in add_graph_to_chain()
      commit-graph: free graph struct that was not added to chain
      commit-graph: free write-context entries before overwriting
      commit-graph: free write-context base_graph_name during cleanup
      commit-graph: clear oidset after finishing write
      decorate: add clear_decoration() function
      revision: clear decoration structs during release_revisions()
      daemon: free listen_addr before returning
      repack: free existing_cruft array after use
      chunk-format: note that pair_chunk() is unsafe
      t: add library for munging chunk-format files
      midx: stop ignoring malformed oid fanout chunk
      commit-graph: check size of oid fanout chunk
      midx: check size of oid lookup chunk
      commit-graph: check consistency of fanout table
      midx: check size of pack names chunk
      midx: enforce chunk alignment on reading
      midx: check size of object offset chunk
      midx: bounds-check large offset chunk
      midx: check size of revindex chunk
      commit-graph: check size of commit data chunk
      commit-graph: detect out-of-bounds extra-edges pointers
      commit-graph: bounds-check base graphs chunk
      commit-graph: check size of generations chunk
      commit-graph: bounds-check generation overflow chunk
      commit-graph: check bounds when accessing BDAT chunk
      commit-graph: check bounds when accessing BIDX chunk
      commit-graph: detect out-of-order BIDX offsets
      chunk-format: drop pair_chunk_unsafe()
      t5319: make corrupted large-offset test more robust
      doc/send-email: mention handling of "reply-to" with --compose
      Revert "send-email: extract email-parsing code into a subroutine"
      send-email: handle to/cc/bcc from --compose message
      t: avoid perl's pack/unpack "Q" specifier

Johannes Schindelin (19):
      windows: ignore empty `PATH` elements
      is_Cygwin: avoid `exec`ing anything
      Move is_<platform> functions to the beginning
      Move the `_which` function (almost) to the top
      Work around Tcl's default `PATH` lookup
      rebase: allow overriding the maximal length of the generated labels
      ci: avoid building from the same commit in parallel
      ci(linux-asan-ubsan): let's save some time
      var: avoid a segmentation fault when `HOME` is unset
      completion(switch/checkout): treat --track and -t the same
      maintenance(systemd): support the Windows Subsystem for Linux
      ci: add a GitHub workflow to submit Coverity scans
      coverity: cache the Coverity Build Tool
      coverity: allow overriding the Coverity project
      coverity: support building on Windows
      coverity: allow running on macOS
      coverity: detect and report when the token or project is incorrect
      max_tree_depth: lower it for MSVC to avoid stack overflows
      ci: upgrade to using macos-13

John Cai (3):
      merge-ort: initialize repo in index state
      attr: read attributes from HEAD when bare repo
      attr: add attr.tree for setting the treeish to read attributes from

Josh Soref (1):
      Documentation/git-status: add missing line breaks

Josip Sokcevic (1):
      diff-lib: fix check_removed when fsmonitor is on

Junio C Hamano (55):
      update-index: do not read HEAD and MERGE_HEAD unconditionally
      resolve-undo: allow resurrecting conflicted state that resolved to deletion
      update-index: use unmerge_index_entry() to support removal
      update-index: remove stale fallback code for "--unresolve"
      checkout/restore: refuse unmerging paths unless checking out of the index
      checkout/restore: add basic tests for --merge
      checkout: allow "checkout -m path" to unmerge removed paths
      mv: fix error for moving directory to another
      diff: move the fallback "--exit-code" code down
      diff: mode-only change should be noticed by "--patch -w --exit-code"
      diff: teach "--stat -w --exit-code" to notice differences
      t4040: remove test that succeeded for a wrong reason
      pretty-formats: define "literal formatting code"
      diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()
      diff: the -w option breaks --exit-code for --raw and other output modes
      transfer.unpackLimit: fetch/receive.unpackLimit takes precedence
      Start the 2.43 cycle
      The second batch for 2.43
      The extra batch to update credenthal helpers
      The third batch
      The fourth batch
      The fifth batch
      The sixth batch
      The seventh batch
      update-index doc: v4 is OK with JGit and libgit2
      update-index: add --show-index-version
      test-tool: retire "index-version"
      The eighth batch
      The ninth batch
      The tenth batch
      The eleventh batch
      completion: loosen and document the requirement around completing alias
      The twelfth batch
      The thirteenth batch
      The fourteenth batch
      The fifteenth batch
      doc: update list archive reference to use lore.kernel.org
      The sixteenth batch
      merge: introduce {copy|clear}_merge_options()
      stash: be careful what we store
      grep: -f <path> is relative to $cwd
      The seventeenth batch
      The eighteenth batch
      commit: do not use cryptic "new_index" in end-user facing messages
      The nineteenth batch
      am: align placeholder for --whitespace option with apply
      The twentieth batch
      The twenty-first batch
      The twenty-second batch
      test framework: further deprecate test_i18ngrep
      Git 2.42.1
      tests: teach callers of test_i18ngrep to use test_grep
      A bit more before -rc1
      Prepare for -rc1
      Git 2.43-rc1

Karthik Nayak (3):
      revision: rename bit to `do_not_die_on_missing_objects`
      rev-list: move `show_commit()` to the bottom
      rev-list: add commit object support in `--missing` option

Kousik Sanagavarapu (4):
      ref-filter: sort numerically when ":size" is used
      t/t6300: cleanup test_atom
      t/t6300: introduce test_bad_atom
      ref-filter: add mailmap support

Kristoffer Haugsbakk (2):
      range-diff: treat notes like `log`
      grep: die gracefully when outside repository

Linus Arver (17):
      trailer tests: make test cases self-contained
      trailer test description: this tests --where=after, not --where=before
      trailer: add tests to check defaulting behavior with --no-* flags
      trailer doc: narrow down scope of --where and related flags
      trailer: trailer location is a place, not an action
      trailer --no-divider help: describe usual "---" meaning
      trailer --parse help: expose aliased options
      trailer --only-input: prefer "configuration variables" over "rules"
      trailer --parse docs: add explanation for its usefulness
      trailer --unfold help: prefer "reformat" over "join"
      trailer doc: emphasize the effect of configuration variables
      trailer doc: separator within key suppresses default separator
      trailer doc: <token> is a <key> or <keyAlias>, not both
      trailer: separate public from internal portion of trailer_iterator
      trailer: split process_input_file into separate pieces
      trailer: split process_command_line_args into separate functions
      strvec: drop unnecessary include of hex.h

M Hickford (3):
      credential/libsecret: store new attributes
      credential/libsecret: erase matching creds only
      credential/wincred: erase matching creds only

Mark Levedahl (6):
      git gui Makefile - remove Cygwin modifications
      git-gui - remove obsolete Cygwin specific code
      git-gui - use cygstart to browse on Cygwin
      git-gui - use mkshortcut on Cygwin
      git-gui - re-enable use of hook scripts
      git-gui - use git-hook, honor core.hooksPath

Mark Ruvald Pedersen (1):
      sequencer: truncate labels to accommodate loose refs

Martin Ågren (1):
      git-merge-file doc: drop "-file" from argument placeholders

Matthew McClain (1):
      git-p4 shouldn't attempt to store symlinks in LFS

Michael Strawbridge (1):
      send-email: move validation code below process_address_list

Michal Suchanek (1):
      git-push doc: more visibility for -q option

Naomi Ibe (1):
      builtin/add.c: clean up die() messages

Oswald Buddenhagen (16):
      t/lib-rebase: set_fake_editor(): fix recognition of reset's short command
      t/lib-rebase: set_fake_editor(): handle FAKE_LINES more consistently
      sequencer: simplify allocation of result array in todo_list_rearrange_squash()
      t/lib-rebase: improve documentation of set_fake_editor()
      t9001: fix indentation in test_no_confirm()
      format-patch: add --description-file option
      sequencer: rectify empty hint in call of require_clean_work_tree()
      sequencer: beautify subject of reverts of reverts
      git-revert.txt: add discussion
      sequencer: fix error message on failure to copy SQUASH_MSG
      t3404-rebase-interactive.sh: fix typos in title of a rewording test
      sequencer: remove unreachable exit condition in pick_commits()
      am: fix error message in parse_opt_show_current_patch()
      rebase: simplify code related to imply_merge()
      rebase: handle --strategy via imply_merge() as well
      rebase: move parse_opt_keep_empty() down

Patrick Steinhardt (23):
      upload-pack: fix exit code when denying fetch of unreachable object ID
      revision: make pseudo-opt flags read via stdin behave consistently
      doc/git-worktree: mention "refs/rewritten" as per-worktree refs
      doc/git-repack: fix syntax for `-g` shorthand option
      doc/git-repack: don't mention nonexistent "--unpacked" option
      commit-graph: introduce envvar to disable commit existence checks
      commit: detect commits that exist in commit-graph but not in the ODB
      builtin/show-ref: convert pattern to a local variable
      builtin/show-ref: split up different subcommands
      builtin/show-ref: fix leaking string buffer
      builtin/show-ref: fix dead code when passing patterns
      builtin/show-ref: refactor `--exclude-existing` options
      builtin/show-ref: stop using global variable to count matches
      builtin/show-ref: stop using global vars for `show_one()`
      builtin/show-ref: refactor options for patterns subcommand
      builtin/show-ref: ensure mutual exclusiveness of subcommands
      builtin/show-ref: explicitly spell out different modes in synopsis
      builtin/show-ref: add new mode to check for reference existence
      t: use git-show-ref(1) to check for ref existence
      test-bloom: stop setting up Git directory twice
      shallow: fix memory leak when registering shallow roots
      setup: refactor `upgrade_repository_format()` to have common exit
      setup: fix leaking repository format

Philippe Blain (3):
      completion: commit: complete configured trailer tokens
      completion: commit: complete trailers tokens more robustly
      completion: improve doc for complex aliases

Phillip Wood (7):
      rebase -i: move unlink() calls
      rebase -i: remove patch file after conflict resolution
      sequencer: use rebase_path_message()
      sequencer: factor out part of pick_commits()
      rebase: fix rewritten list for failed pick
      rebase --continue: refuse to commit after failed command
      rebase -i: fix adding failed command to the todo list

René Scharfe (18):
      subtree: disallow --no-{help,quiet,debug,branch,message}
      t1502, docs: disallow --no-help
      t1502: move optionspec help output to a file
      t1502: test option negation
      parse-options: show negatability of options in short help
      parse-options: factor out usage_indent() and usage_padding()
      parse-options: no --[no-]no-...
      parse-options: simplify usage_padding()
      parse-options: allow omitting option help text
      name-rev: use OPT_HIDDEN_BOOL for --peel-tag
      grep: use OPT_INTEGER_F for --max-depth
      grep: reject --no-or
      diff --no-index: fix -R with stdin
      parse-options: drop unused parse_opt_ctx_t member
      parse-options: make CMDMODE errors more precise
      am: simplify --show-current-patch handling
      am, rebase: fix arghelp syntax of --empty
      reflog: fix expire --single-worktree

Robert Coup (1):
      upload-pack: add tracing for fetches

Rubén Justo (2):
      branch: error message deleting a branch in use
      branch: error message checking out a branch in use

Sergey Organov (4):
      doc/diff-options: fix link to generating patch section
      diff-merges: improve --diff-merges documentation
      diff-merges: introduce '--dd' option
      completion: complete '--dd'

Shuqi Liang (3):
      t1092: add tests for 'git check-attr'
      attr.c: read attributes in a sparse directory
      check-attr: integrate with sparse-index

Tang Yuyi (1):
      merge-tree: add -X strategy option

Taylor Blau (28):
      repack: move `pack_geometry` struct to the stack
      commit-graph: introduce `commit_graph_generation_from_graph()`
      t/t5318-commit-graph.sh: test generation zero transitions during fsck
      commit-graph: avoid repeated mixed generation number warnings
      leak tests: mark a handful of tests as leak-free
      leak tests: mark t3321-notes-stripspace.sh as leak-free
      leak tests: mark t5583-push-branches.sh as leak-free
      builtin/pack-objects.c: remove unnecessary strbuf_reset()
      builtin/pack-objects.c: support `--max-pack-size` with `--cruft`
      Documentation/gitformat-pack.txt: remove multi-cruft packs alternative
      Documentation/gitformat-pack.txt: drop mixed version section
      builtin/repack.c: extract structure to store existing packs
      builtin/repack.c: extract marking packs for deletion
      builtin/repack.c: extract redundant pack cleanup for --geometric
      builtin/repack.c: extract redundant pack cleanup for existing packs
      builtin/repack.c: extract `has_existing_non_kept_packs()`
      builtin/repack.c: store existing cruft packs separately
      builtin/repack.c: avoid directly inspecting "util"
      builtin/repack.c: extract common cruft pack loop
      git-send-email.perl: avoid printing undef when validating addresses
      t7700: split cruft-related tests to t7704
      builtin/repack.c: parse `--max-pack-size` with OPT_MAGNITUDE
      builtin/repack.c: implement support for `--max-cruft-size`
      builtin/repack.c: avoid making cruft packs preferred
      Documentation/gitformat-pack.txt: fix typo
      Documentation/gitformat-pack.txt: fix incorrect MIDX documentation
      list-objects: drop --unpacked non-commit objects from results
      pack-bitmap: drop --unpacked non-commit objects from results

Todd Zullinger (2):
      RelNotes: minor typo fixes in 2.43.0 draft
      RelNotes: improve wording of credential helper notes

Victoria Dye (4):
      ref-cache.c: fix prefix matching in ref iteration
      dir.[ch]: expose 'get_dtype'
      dir.[ch]: add 'follow_symlink' arg to 'get_dtype'
      files-backend.c: avoid stat in 'loose_fill_ref_dir'

Vipul Kumar (1):
      git-gui: Fix a typo in README

Wesley Schwengle (2):
      git-push.txt: fix grammar
      git-svn: drop FakeTerm hack

brian m. carlson (2):
      t: add a test helper to truncate files
      merge-file: add an option to process object IDs

Ævar Arnfjörð Bjarmason (1):
      Makefiles: change search through $(MAKEFLAGS) for GNU make 4.4

Štěpán Němec (6):
      doc: fix some typos, grammar and wording issues
      doc/diff-options: improve wording of the log.diffMerges mention
      git-jump: admit to passing merge mode args to ls-files
      doc/gitk: s/sticked/stuck/
      t/README: fix multi-prerequisite example
      doc/cat-file: make synopsis and description less confusing

王常新 (1):
      merge-ort.c: fix typo 'neeed' to 'needed'

谢致邦 (XIE Zhibang) (2):
      doc: correct the 50 characters soft limit
      doc: correct the 50 characters soft limit (+)


^ permalink raw reply

* Re: [PATCH v2 3/3] t9164: fix inability to find basename(1) in hooks
From: Jeff King @ 2023-11-08 17:21 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, Junio C Hamano
In-Reply-To: <361f1bd9c88e3e6b7b135ba67b39d3bf4d70e322.1699455383.git.ps@pks.im>

On Wed, Nov 08, 2023 at 03:57:27PM +0100, Patrick Steinhardt wrote:

> diff --git a/t/t9164-git-svn-dcommit-concurrent.sh b/t/t9164-git-svn-dcommit-concurrent.sh
> index c8e6c0733f4..3b64022dc57 100755
> --- a/t/t9164-git-svn-dcommit-concurrent.sh
> +++ b/t/t9164-git-svn-dcommit-concurrent.sh
> @@ -47,9 +47,15 @@ setup_hook()
>  	echo "cnt=$skip_revs" > "$hook_type-counter"
>  	rm -f "$rawsvnrepo/hooks/"*-commit # drop previous hooks
>  	hook="$rawsvnrepo/hooks/$hook_type"
> -	cat > "$hook" <<- 'EOF1'
> +	cat >"$hook" <<-EOF
>  		#!/bin/sh
>  		set -e
> +
> +		PATH=\"$PATH\"
> +		export PATH
> +	EOF

The quoting here is weird. The original escaped the double-quotes
because it was echo-ing and interpolated string:

> -		echo "PATH=\"$PATH\"; export PATH" >>"$hook"

But the here-doc interpolation does not treat double-quotes as special,
and you end up with actual double-quote characters at the beginning and
end of your $PATH variable, wrecking those entries (but I guess it works
some of the time depending on whether those parts of your $PATH are
important).

It also interpolates PATH while making the here-doc. That's what you
want to convince the hook's inner shell to use the same PATH as the
outer shell, but it also means that the inner shell will parse and
evaluate it. I don't think anyone is going to inject '"; rm -rf /' into
their own test runs, but it does mean we'd do the wrong thing if their
$PATH contains double-quotes or backslashes.

Certainly the original suffered from the same issue, so it may not be
worth worrying about. But the more robust way to do it is:

  # export ORIG_PATH, which presumably is not cleared the same way PATH
  # is, so that the hook can access it
  ORIG_PATH=$PATH &&
  export ORIG_PATH &&

  cat >"$hook" <<EOF
  # pull the original path from the caller
  PATH=$ORIG_PATH
  export PATH

  ...do other stuff...
  EOF

That's assuming that environment variables make it intact to the hook at
all (it is not clear to me why the original $PATH doesn't). If they
don't, you'd probably have to stash it in a file like:

  echo "$PATH" >orig-path
  cat >"$hook" <<EOF
  PATH="$(cat orig-path)"
  export PATH
  EOF

-Peff

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox