| Message ID | 4140fca4f454310d215df8bdac237caeb5c38521.1709495964.git.code@khaugsbakk.name (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase: teach `--exec` about `GIT_REBASE_BRANCH` | expand |
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".
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
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
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
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(-)