[PATCH 0/1] rebase: teach `--exec` about `GIT_REBASE_BRANCH`

[PATCH 0/1] rebase: teach `--exec` about `GIT_REBASE_BRANCH`

[Kristoffer Haugsbakk]

The following patch adds an env. variable for the branch name that we
were on before the rebase operation started. This is for use by `--exec
<cmd>`.

The need for fetching the branch name came up while making a script for
`--exec` and it seemed that parsing the name out of the first line of
`git branch --list` was the best approach.

I thought that was inconvenient. Why not an environment variable set by
git-rebase(1)? (open question)

See: https://stackoverflow.com/a/50124157/1725151

§ Implementation

The implementation is inspired by
`builtin/branch.c:print_current_branch_name`.

Kristoffer Haugsbakk (1):
  rebase: teach `--exec` about `GIT_REBASE_BRANCH`

 Documentation/git-rebase.txt |  4 ++++
 builtin/rebase.c             | 15 ++++++++++++++-
 t/t3409-rebase-environ.sh    | 19 +++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

-- 
2.44.0.64.g52b67adbeb2

[PATCH 1/1] rebase: teach `--exec` about `GIT_REBASE_BRANCH`

[Kristoffer Haugsbakk]

The command fed to `--exec` might need some contextual information from
the branch name. But there is no convenient access to the branch name
that we were on before starting the rebase; rebase operates in detached
HEAD mode so we cannot ask for it directly. This means that we need to
parse something like this from the first line of `git branch --list`:

    (no branch, rebasing <branch>)

This is a moderate amount of effort for something that git-rebase(1) can
store for us.

To that end, teach `--exec` about an env. variable which stores the
branch name for the rebase-in-progress, if applicable.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
 Documentation/git-rebase.txt |  4 ++++
 builtin/rebase.c             | 15 ++++++++++++++-
 t/t3409-rebase-environ.sh    | 19 +++++++++++++++++++
 3 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 06206521fc3..9b3d6ee8203 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -578,6 +578,10 @@ squash/fixup series.
 This uses the `--interactive` machinery internally, but it can be run
 without an explicit `--interactive`.
 +
+The command has access to the environment variable `GIT_REBASE_BRANCH`
+which stores the branch name that `HEAD` was pointing at when the rebase
+started, if applicable.
++
 See also INCOMPATIBLE OPTIONS below.
 
 --root::
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 5b086f651a6..0202130c2d7 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -1044,6 +1044,17 @@ static int check_exec_cmd(const char *cmd)
 	return 0;
 }
 
+static void try_set_env_git_rebase_branch(void)
+{
+	const char *refname = resolve_ref_unsafe("HEAD", 0, NULL, NULL);
+	const char *shortname = NULL;
+
+	if (refname)
+		skip_prefix(refname, "refs/heads/", &shortname);
+	if (shortname)
+		xsetenv("GIT_REBASE_BRANCH", shortname, true);
+}
+
 int cmd_rebase(int argc, const char **argv, const char *prefix)
 {
 	struct rebase_options options = REBASE_OPTIONS_INIT;
@@ -1451,8 +1462,10 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
 	if (gpg_sign)
 		options.gpg_sign_opt = xstrfmt("-S%s", gpg_sign);
 
-	if (options.exec.nr)
+	if (options.exec.nr) {
 		imply_merge(&options, "--exec");
+		try_set_env_git_rebase_branch();
+	}
 
 	if (options.type == REBASE_APPLY) {
 		if (ignore_whitespace)
diff --git a/t/t3409-rebase-environ.sh b/t/t3409-rebase-environ.sh
index acaf5558dbe..5b1d78a255a 100755
--- a/t/t3409-rebase-environ.sh
+++ b/t/t3409-rebase-environ.sh
@@ -21,4 +21,23 @@ test_expect_success 'rebase --exec does not muck with GIT_WORK_TREE' '
 	test_must_be_empty environ
 '
 
+test_expect_success 'rebase --exec cmd can access GIT_REBASE_BRANCH' '
+	write_script cmd <<-\EOF &&
+printf "%s\n" $GIT_REBASE_BRANCH >actual
+EOF
+	git branch --show-current >expect &&
+	git rebase --exec ./cmd HEAD~1 &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rebase --exec cmd has no GIT_REBASE_BRANCH when on detached HEAD' '
+	test_when_finished git checkout - &&
+	git checkout --detach &&
+	write_script cmd <<-\EOF &&
+printf "%s" $GIT_REBASE_BRANCH >environ
+EOF
+	git rebase --exec ./cmd HEAD~1 &&
+	test_must_be_empty environ
+'
+
 test_done
-- 
2.44.0.64.g52b67adbeb2

Re: [PATCH 1/1] rebase: teach `--exec` about `GIT_REBASE_BRANCH`

[Junio C Hamano]

Kristoffer Haugsbakk <code@khaugsbakk.name> writes:

> The command fed to `--exec` might need some contextual information from
> the branch name. But there is no convenient access to the branch name
> that we were on before starting the rebase; rebase operates in detached
> HEAD mode so we cannot ask for it directly. This means that we need to
> parse something like this from the first line of `git branch --list`:
>
>     (no branch, rebasing <branch>)
>
> This is a moderate amount of effort for something that git-rebase(1) can
> store for us.
>
> To that end, teach `--exec` about an env. variable which stores the
> branch name for the rebase-in-progress, if applicable.

You seem to be saying that `git branch --list` output already
contains the necessary information but it is shown in a hard to use
format.  Is the information given at least always accurate and
reliable?

Assuming it is, do you know where "git branch --list" gets that
information when it says "(no branch, rebasing <branch>)"?

git-rebase(1) is already storing information sufficient to let "git
branch --list" to produce that information, and there are other ways
to inspect that state ("git status" gives the same information but
it also is in a "meant for humans" format).

So, isn't it just the matter of surfacing the information that we
are already recording and is already available in a fashion that is
easier to use?  For example, if "git status --porcelain=[version]"
does not give the information, perhaps you can add a line or two to
it, instead of duplicating the same information in two places?

It comes from wt-status.c:wt_status_check_rebase() where state->branch
is assigned to, by reading "$GIT_DIR/rebase-{apply,merge}/head-name".

Re: [PATCH 1/1] rebase: teach `--exec` about `GIT_REBASE_BRANCH`

[Phillip Wood]

On 03/03/2024 23:24, Junio C Hamano wrote:
> Kristoffer Haugsbakk <code@khaugsbakk.name> writes:
> 
> So, isn't it just the matter of surfacing the information that we
> are already recording and is already available in a fashion that is
> easier to use?  For example, if "git status --porcelain=[version]"
> does not give the information, perhaps you can add a line or two to
> it, instead of duplicating the same information in two places?

That was my thought as well. I also don't think it is helpful to think 
of a single branch being associated with a rebase these days. If we 
update the output of "git stasus --porcelain" we should show all the 
refs that are being rewritten by reading the contents of 
rebase_path_update_refs() as well as the head-name file.

Best Wishes

Phillip

Re: [PATCH 1/1] rebase: teach `--exec` about `GIT_REBASE_BRANCH`

[Kristoffer Haugsbakk]

On Mon, Mar 4, 2024, at 00:24, Junio C Hamano wrote:
> Kristoffer Haugsbakk <code@khaugsbakk.name> writes:
>
>> The command fed to `--exec` might need some contextual information from
>> the branch name. But there is no convenient access to the branch name
>> that we were on before starting the rebase; rebase operates in detached
>> HEAD mode so we cannot ask for it directly. This means that we need to
>> parse something like this from the first line of `git branch --list`:
>>
>>     (no branch, rebasing <branch>)
>>
>> This is a moderate amount of effort for something that git-rebase(1) can
>> store for us.
>>
>> To that end, teach `--exec` about an env. variable which stores the
>> branch name for the rebase-in-progress, if applicable.
>
> You seem to be saying that `git branch --list` output already
> contains the necessary information but it is shown in a hard to use
> format.  Is the information given at least always accurate and
> reliable?
>
> Assuming it is, do you know where "git branch --list" gets that
> information when it says "(no branch, rebasing <branch>)"?
>
> git-rebase(1) is already storing information sufficient to let "git
> branch --list" to produce that information, and there are other ways
> to inspect that state ("git status" gives the same information but
> it also is in a "meant for humans" format).
>
> So, isn't it just the matter of surfacing the information that we
> are already recording and is already available in a fashion that is
> easier to use?  For example, if "git status --porcelain=[version]"
> does not give the information, perhaps you can add a line or two to
> it, instead of duplicating the same information in two places?
>
> It comes from wt-status.c:wt_status_check_rebase() where state->branch
> is assigned to, by reading "$GIT_DIR/rebase-{apply,merge}/head-name".

Okay, thanks for the code directions and input (both). I’ll try to get
back to a rewrite on this topic in a while.

Cheers

-- 
Kristoffer Haugsbakk