All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nico Williams <nico@cryptonector.com>
To: Alejandro Colomar <alx@kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org, Ben Boeckel <mathstuf@gmail.com>,
	Viktor Dukhovni <viktor@openssl.org>
Subject: Re: [RFC] git-brebase
Date: Sat, 3 Oct 2026 17:19:35 -0500	[thread overview]
Message-ID: <asF/d50VkzYSAPXn@ubby> (raw)
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
-- 

  reply	other threads:[~2026-10-03 22:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-03 20:11 [RFC] git-brebase Alejandro Colomar
2026-10-03 20:13 ` Alejandro Colomar
2026-10-03 20:39 ` Nico Williams
2026-10-03 20:48   ` Nico Williams
2026-10-03 21:07     ` Alejandro Colomar
2026-10-03 21:17       ` Nico Williams
2026-10-03 21:29         ` Alejandro Colomar
2026-10-03 21:50           ` Nico Williams
2026-10-03 20:56   ` Alejandro Colomar
2026-10-03 21:13     ` Nico Williams
2026-10-03 21:38       ` Alejandro Colomar
2026-10-03 22:19         ` Nico Williams [this message]
2026-10-03 21:17     ` Alejandro Colomar
2026-10-08 22:18 ` [RFC v3] git-bisect-rebase Alejandro Colomar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=asF/d50VkzYSAPXn@ubby \
    --to=nico@cryptonector.com \
    --cc=alx@kernel.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=mathstuf@gmail.com \
    --cc=viktor@openssl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.