From: Nico Williams <nico@cryptonector.com>
Date: Sat, 03 Oct 2026 22:19:35 GMT
Subject: Re: [RFC] git-brebase
Message-ID: <asF/d50VkzYSAPXn@ubby>
In-Reply-To: <asF0DkNtlnJ9-Sng@debian>

On Sat, Oct 03, 2026 at 11:38:29PM +0200, Alejandro Colomar wrote:
> > I'd have an option or sub-command of the main script that says "do the
> > callback thing", then when you run `git bisect run ...` put in the name
> > of this script as the command and the "do the callback thing" option
> > next.
> 
> I'd need to see some code.  I'm not seeing it.  :)

Warning: NOT TESTED.

Warning: I did not first adopt your other patch to support detached HEAD
mode.

Look, no temp file in sight:

	@@ -1,164 +1,164 @@
	 #!/bin/bash
	 # Copyright 2026, Alejandro Colomar <alx@kernel.org>
	 # SPDX-License-Identifier: GPL-3.0-or-later
	 
	 set -Eeufo pipefail;
	 shopt -s lastpipe;
	 
	 err()
	 {
	 	>&2 printf '%s\n' "$(basename "$0"): error: $*";
	 	exit 1;
	 }
	 
	 fp='';
	 other='';
	 pre='';
	 post='';
	+callback=false;
	 while test $# -ge 1; do
	 	case "$1" in
	 	--first-parent)
	 		fp='--first-parent';
	 		;;
	 	--pre-exec=*)
	 		echo "$1" \
	 		| sed 's/--pre-exec=//' \
	 		| read -r pre;
	 		;;
	 	--post-exec=*)
	 		echo "$1" \
	 		| sed 's/--post-exec=//' \
	 		| read -r post;
	 		;;
	+	--bisect-run-callback)
	+		callback=true
	+		break;;
	 	-*)
	 		other="$other $1";
	 		;;
	 	*)
	 		break;
	 		;;
	 	esac;
	 	shift;
	 done;
	-gbopts="$fp";
	-gropts="$other";
	 
	-if test $# -lt 1; then
	-	err 'Missing target commit.';
	-fi;
	-if test $# -gt 1; then
	-	err 'Too many arguments.';
	-fi;
	-git rev-list -1 "$1" \
	-| read -r tgt;
	-git rev-parse --abbrev-ref HEAD \
	-| read -r branch;
	-
	-# Set up the callback script for 'git rebase run'.
	-mktemp \
	-| read -r callback;
	-cat >"$callback" <<__EOF__
	-#!/bin/bash
	-
	-	set -Eeufo pipefail;
	-	shopt -s lastpipe;
	+if $callback; then
	+	# Positional arguments to the bisect run callback
	+	branch="$1"
	+	gropts="$2"
	+	pre="${3:-}"
	+	post="${4:-}"
	 
	 	git rev-list -1 HEAD \
	 	| read -r bisect_head;
	 
	-	if test -n '$pre'; then
	+	if test -n "$pre"; then
	 		printf '%s' 'Pre-rebase exec: ';
	-		pre='$pre';
	 		if
	-			\$pre;
	-			x="\$?";
	+			$pre;
	+			x="$?";
	 			true;
	 		then
	-			case "\$x" in
	+			case "$x" in
	 			0)
	 				echo 'success';
	 				;;
	 			125)
	 				echo 'skip';
	-				git checkout --detach "\$bisect_head" 2>/dev/null;
	+				git checkout --detach "$bisect_head" 2>/dev/null;
	 				exit 125;
	 				;;
	 			*)
	-				echo "failure (\$x)";
	-				git checkout --detach "\$bisect_head" 2>/dev/null;
	-				exit "\$x";
	+				echo "failure ($x)";
	+				git checkout --detach "$bisect_head" 2>/dev/null;
	+				exit "$x";
	 				;;
	 			esac;
	 		fi;
	 	fi;
	 
	-	git switch '$branch' >/dev/null 2>/dev/null;
	+	git switch "$branch" >/dev/null 2>/dev/null;
	 	git rev-list -1 HEAD \
	 	| read -r old_head;
	 	printf '%s' 'Rebase: ';
	-	if git rebase $gropts "\$bisect_head" >/dev/null 2>/dev/null; then
	+	if git rebase $gropts "$bisect_head" >/dev/null 2>/dev/null; then
	 		echo 'success';
	 	else
	 		echo 'conflict';
	 		git rebase --abort >/dev/null;
	-		git checkout --detach "\$bisect_head" 2>/dev/null;
	+		git checkout --detach "$bisect_head" 2>/dev/null;
	 		exit 1;
	 	fi;
	 
	-	if test -n '$post'; then
	+	if test -n "$post"; then
	 		printf '%s' 'Post-rebase exec: ';
	-		post='$post';
	 		if
	-			\$post;
	-			x="\$?";
	+			$post;
	+			x="$?";
	 			true;
	 		then
	-			case "\$x" in
	+			case "$x" in
	 			0)
	 				echo 'success';
	 				;;
	 			125)
	 				echo 'skip';
	-				git reset --hard "\$old_head";
	-				git checkout --detach "\$bisect_head" 2>/dev/null;
	+				git reset --hard "$old_head";
	+				git checkout --detach "$bisect_head" 2>/dev/null;
	 				exit 125;
	 				;;
	 			*)
	 				echo "failure (\$x)";
	-				git reset --hard "\$old_head";
	-				git checkout --detach "\$bisect_head" 2>/dev/null;
	-				exit "\$x";
	+				git reset --hard "$old_head";
	+				git checkout --detach "$bisect_head" 2>/dev/null;
	+				exit "$x";
	 				;;
	 			esac;
	 		fi;
	 	fi;
	-	git checkout --detach "\$bisect_head" 2>/dev/null;
	+	git checkout --detach "$bisect_head" 2>/dev/null;
	 	exit 0;
	-__EOF__
	-chmod +x "$callback";
	+fi
	+
	+gbopts="$fp";
	+gropts="$other";
	+
	+if test $# -lt 1; then
	+	err 'Missing target commit.';
	+fi;
	+if test $# -gt 1; then
	+	err 'Too many arguments.';
	+fi;
	+git rev-list -1 "$1" \
	+| read -r tgt;
	+git rev-parse --abbrev-ref HEAD \
	+| read -r branch;
	 
	 # Try the target first.
	 git checkout --detach "$tgt" 2>/dev/null;
	-if "$callback"; then
	+if "$callback" "$branch" "$gropts" "$pre" "$post"; then
	 	exit 0;
	 fi;
	 git status;
	 
	 # Bisect.
	 # shellcheck disable=SC2248  # gbopts may hold multiple options
	 git bisect start $gbopts >/dev/null;
	 git bisect bad "$tgt" >/dev/null;
	 git merge-base "$branch" "$tgt" \
	 | xargs -I{} git bisect good {};
	-git bisect run "$callback";
	+git bisect run "$0" --bisect-run-callback "$branch" "$gropts" "$pre" "$post";
	 git rev-list -1 bisect/bad \
	 | read -r bad;
	 git bisect reset >/dev/null 2>/dev/null;
	 
	 # Perform the conflicting rebase
	 git switch "$branch";
	 # shellcheck disable=SC2086  # gropts may hold multiple options
	 git rebase $gropts "$bad";
	 if test -v post; then
	 	echo 'Running post-rebase exec.';
	 	$post;
	 fi;

> > > > and use environment
> > > > variables to pass arguments to it.
> > > 
> > > The callback doesn't really need any arguments, since 'git bisect run'
> > > won't pass any arguments to it.
> > 
> > But you're embedding values into the temp executable script -- if you
> > don't have that any more you'll have to pass those in.
> 
> But why would we want to not have it?
> That would complicate the script, no?

Because I don't want it writing temp files unless absolutely necessary.
Even with a `trap` this can leave garbage behind.  Better to avoid it.

Plus I... just don't like that style of bash scripting, and sure, that's
just personal preference.

Nico
--