Re: [PATCH v2 2/9] setup: introduce startup_info->original_cwd

"Elijah Newren via GitGitGadget" [off-list ref] writes:

We assume that, unless we are an auxiliary tool like git-remote-http,
we should always have cwd?

s/wass/was/;

+	 * we're an auxiliary tool like git-remote-http or test-tool.
+	 */
+	if (!startup_info->original_cwd)
+		return;
+
+	/*
+	 * startup_info->original_cwd points to the current working
+	 * directory we inherited from our parent process, which is a
+	 * directory we want to avoid incidentally removing.
+	 *
+	 * For convience, we would like to have the path relative to the
+	 * worktree instead of an absolute path.
+	 *
+	 * Yes, startup_info->original_cwd is usually the same as 'prefix',
+	 * but differs in two ways:
+	 *   - prefix has a trailing '/'
+	 *   - if the user passes '-C' to git, that modifies the prefix but
+	 *     not startup_info->original_cwd.
+	 */
+
+	/* Normalize the directory */
+	strbuf_realpath(&tmp, startup_info->original_cwd, 1);
+	free((char*)startup_info->original_cwd);
+	startup_info->original_cwd = strbuf_detach(&tmp, NULL);
+
+	/* Find out if this is in the worktree */
+	worktree = get_git_work_tree();
+	if (worktree)
+		offset = dir_inside_of(startup_info->original_cwd, worktree);
+	if (offset >= 0) {
+		/*
+		 * original_cwd was inside worktree; precompose it just as
+		 * we do prefix so that built up paths will match
+		 */
+		startup_info->original_cwd = \
+			precompose_string_if_needed(startup_info->original_cwd
+						    + offset);
+	}

I wonder if we want to clear the .original_cwd member, so that the
"cwd protection" do not have to worry about anything at all when we
are in a bare repository.

On Thu, Nov 25, 2021 at 10:52 PM Junio C Hamano [off-list ref] wrote:

"Elijah Newren via GitGitGadget" [off-list ref] writes:

quoted

diff --git a/git.c b/git.c
index 5ff21be21f3..2c98ab48936 100644
--- a/git.c
+++ b/git.c

@@ -866,6 +866,8 @@ int cmd_main(int argc, const char **argv)

      trace_command_performance(argv);

+     startup_info->original_cwd = xgetcwd();

We assume that, unless we are an auxiliary tool like git-remote-http,
we should always have cwd?

Yeah, Ævar caught this mistake too; I should have used
strbuf_getcwd().  Will fix.

quoted

diff --git a/setup.c b/setup.c
index 347d7181ae9..f30657723ea 100644
--- a/setup.c
+++ b/setup.c

@@ -432,6 +432,54 @@ void setup_work_tree(void)
      initialized = 1;
 }

+static void setup_original_cwd(void)
+{
+     struct strbuf tmp = STRBUF_INIT;
+     const char *worktree = NULL;
+     int offset = -1;
+
+     /*
+      * startup_info->original_cwd wass set early on in cmd_main(), unless

s/wass/was/;

Thanks.

quoted

+      * we're an auxiliary tool like git-remote-http or test-tool.
+      */
+     if (!startup_info->original_cwd)
+             return;
+
+     /*
+      * startup_info->original_cwd points to the current working
+      * directory we inherited from our parent process, which is a
+      * directory we want to avoid incidentally removing.
+      *
+      * For convience, we would like to have the path relative to the
+      * worktree instead of an absolute path.
+      *
+      * Yes, startup_info->original_cwd is usually the same as 'prefix',
+      * but differs in two ways:
+      *   - prefix has a trailing '/'
+      *   - if the user passes '-C' to git, that modifies the prefix but
+      *     not startup_info->original_cwd.
+      */
+
+     /* Normalize the directory */
+     strbuf_realpath(&tmp, startup_info->original_cwd, 1);
+     free((char*)startup_info->original_cwd);
+     startup_info->original_cwd = strbuf_detach(&tmp, NULL);
+
+     /* Find out if this is in the worktree */
+     worktree = get_git_work_tree();
+     if (worktree)
+             offset = dir_inside_of(startup_info->original_cwd, worktree);
+     if (offset >= 0) {
+             /*
+              * original_cwd was inside worktree; precompose it just as
+              * we do prefix so that built up paths will match
+              */
+             startup_info->original_cwd = \
+                     precompose_string_if_needed(startup_info->original_cwd
+                                                 + offset);
+     }

I wonder if we want to clear the .original_cwd member, so that the
"cwd protection" do not have to worry about anything at all when we
are in a bare repository.

Yeah, I wondered about this a bit and whether it'd even matter either
way if the original_cwd was something outside the working tree.  I can
obviously come up with scenarios, but can't see protecting or not
protecting such directories as mattering; if the user is in a
directory where git internals are stored and deleted, well, they're
already off-roading.

But, since you bring it up, I might as well scope this to just
protecting a directory within the working tree.  Perhaps that'd make
Ævar more comfortable with the change too.  I'll do that.