git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Glen Choo <chooglen@google.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Elijah Newren <newren@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH] sequencer, stash: fix running from worktree subdir
Date: Wed, 26 Jan 2022 01:43:45 +0000	[thread overview]
Message-ID: <pull.1205.git.git.1643161426138.gitgitgadget@gmail.com> (raw)

From: Elijah Newren <newren@gmail.com>

In commits bc3ae46b42 ("rebase: do not attempt to remove
startup_info->original_cwd", 2021-12-09) and 0fce211ccc ("stash: do not
attempt to remove startup_info->original_cwd", 2021-12-09), we wanted to
allow the subprocess to know which directory the parent process was
running from, so that the subprocess could protect it.  However...

When run from a non-main worktree, setup_git_directory() will note
that the discovered git directory
(/PATH/TO/.git/worktree/non-main-worktree) does not match
DEFAULT_GIT_DIR_ENVIRONMENT (see setup_discovered_git_dir()), and
decide to set GIT_DIR in the environment.  This matters because...

Whenever git is run with the GIT_DIR environment variable set, and
GIT_WORK_TREE not set, it presumes that '.' is the working tree.  So...

This combination results in the subcommand being very confused about
the working tree.  Fix it by also setting the GIT_WORK_TREE environment
variable along with setting cmd.dir.

A possibly more involved fix we could consider for later would be to
make setup.c set GIT_WORK_TREE whenever (a) it discovers both the git
directory and the working tree and (b) it decides to set GIT_DIR in the
environment.  I did not attempt that here as such would be too big of a
change for a 2.35.1 release.

Test-case-by: Glen Choo <chooglen@google.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
    [2.35 regression] sequencer, stash: fix running from worktree subdir
    
    Sorry for the regression in Git 2.35; thanks for the report Glen (and
    Eric for feedback on earlier patches we've been bouncing back and forth
    that I've incorporated here)
    
    Previous discussion thread starting over here:
    https://lore.kernel.org/git/kl6lilu71rzl.fsf@chooglen-macbookpro.roam.corp.google.com/

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1205%2Fnewren%2Ffix-rebase-subdir-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1205/newren/fix-rebase-subdir-v1
Pull-Request: https://github.com/git/git/pull/1205

 builtin/stash.c   |  6 +++++-
 sequencer.c       |  5 ++++-
 t/t3400-rebase.sh | 21 +++++++++++++++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/builtin/stash.c b/builtin/stash.c
index 1ef2017c595..86cd0b456e7 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -1539,8 +1539,12 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
 			struct child_process cp = CHILD_PROCESS_INIT;
 
 			cp.git_cmd = 1;
-			if (startup_info->original_cwd)
+			if (startup_info->original_cwd) {
 				cp.dir = startup_info->original_cwd;
+				strvec_pushf(&cp.env_array, "%s=%s",
+					     GIT_WORK_TREE_ENVIRONMENT,
+					     the_repository->worktree);
+			}
 			strvec_pushl(&cp.args, "clean", "--force",
 				     "--quiet", "-d", ":/", NULL);
 			if (include_untracked == INCLUDE_ALL_FILES)
diff --git a/sequencer.c b/sequencer.c
index 6abd72160cc..5213d16e971 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4223,8 +4223,11 @@ static int run_git_checkout(struct repository *r, struct replay_opts *opts,
 
 	cmd.git_cmd = 1;
 
-	if (startup_info->original_cwd)
+	if (startup_info->original_cwd) {
 		cmd.dir = startup_info->original_cwd;
+		strvec_pushf(&cmd.env_array, "%s=%s",
+			     GIT_WORK_TREE_ENVIRONMENT, r->worktree);
+	}
 	strvec_push(&cmd.args, "checkout");
 	strvec_push(&cmd.args, commit);
 	strvec_pushf(&cmd.env_array, GIT_REFLOG_ACTION "=%s", action);
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index 23dbd3c82ed..71b1735e1dd 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -416,4 +416,25 @@ test_expect_success MINGW,SYMLINKS_WINDOWS 'rebase when .git/logs is a symlink'
 	mv actual_logs .git/logs
 '
 
+test_expect_success 'rebase when inside worktree subdirectory' '
+	git init main-wt &&
+	(
+		cd main-wt &&
+		git commit --allow-empty -m "initial" &&
+		mkdir -p foo/bar &&
+		test_commit foo/bar/baz &&
+		mkdir -p a/b &&
+		test_commit a/b/c &&
+		# create another branch for our other worktree
+		git branch other &&
+		git worktree add ../other-wt other &&
+		cd ../other-wt &&
+		# create and cd into a subdirectory
+		mkdir -p random/dir &&
+		cd random/dir &&
+		# now do the rebase
+		git rebase --onto HEAD^^ HEAD^  # drops the HEAD^ commit
+	)
+'
+
 test_done

base-commit: 89bece5c8c96f0b962cfc89e63f82d603fd60bed
-- 
gitgitgadget

             reply	other threads:[~2022-01-26  1:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-26  1:43 Elijah Newren via GitGitGadget [this message]
2022-01-26 17:41 ` [PATCH] sequencer, stash: fix running from worktree subdir Glen Choo
2022-01-26 20:01 ` Junio C Hamano

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=pull.1205.git.git.1643161426138.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=chooglen@google.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=sunshine@sunshineco.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).