From: Elijah Newren <newren@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>,
"Git Mailing List" <git@vger.kernel.org>,
"Jeff King" <peff@peff.net>, "René Scharfe" <l.s.r@web.de>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Glen Choo" <chooglen@google.com>,
"Philip Oakley" <philipoakley@iee.email>
Subject: Re: [PATCH v2 2/9] setup: introduce startup_info->original_cwd
Date: Fri, 26 Nov 2021 10:01:41 -0800 [thread overview]
Message-ID: <CABPp-BG8narMYNeaBHQfhcOFJ4hHJM+HGoy7S4hyOA-7PbOEPA@mail.gmail.com> (raw)
In-Reply-To: <xmqqo8678l3x.fsf@gitster.g>
On Thu, Nov 25, 2021 at 10:52 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > 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.
> > 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.
> > + * 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.
next prev parent reply other threads:[~2021-11-26 18:22 UTC|newest]
Thread overview: 163+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-11-21 0:46 [PATCH 0/8] Avoid removing the current working directory, even if it becomes empty Elijah Newren via GitGitGadget
2021-11-21 0:46 ` [PATCH 1/8] t2501: add various tests for removing the current working directory Elijah Newren via GitGitGadget
2021-11-21 17:57 ` Ævar Arnfjörð Bjarmason
2021-11-23 1:45 ` Elijah Newren
2021-11-23 2:19 ` Ævar Arnfjörð Bjarmason
2021-11-23 3:11 ` Elijah Newren
2021-11-25 10:04 ` Ævar Arnfjörð Bjarmason
2021-11-21 0:46 ` [PATCH 2/8] repository, setup: introduce the_cwd Elijah Newren via GitGitGadget
2021-11-21 8:00 ` Junio C Hamano
2021-11-22 22:38 ` Elijah Newren
2021-11-21 8:56 ` René Scharfe
2021-11-22 23:09 ` Elijah Newren
2021-11-21 0:46 ` [PATCH 3/8] unpack-trees: refuse to remove the current working directory Elijah Newren via GitGitGadget
2021-11-21 0:46 ` [PATCH 4/8] unpack-trees: add special cwd handling Elijah Newren via GitGitGadget
2021-11-21 0:46 ` [PATCH 5/8] symlinks: do not include current working directory in dir removal Elijah Newren via GitGitGadget
2021-11-21 8:56 ` René Scharfe
2021-11-23 0:35 ` Elijah Newren
2021-11-21 0:46 ` [PATCH 6/8] clean: do not attempt to remove current working directory Elijah Newren via GitGitGadget
2021-11-21 17:51 ` Ævar Arnfjörð Bjarmason
2021-11-23 1:28 ` Elijah Newren
2021-11-21 0:46 ` [PATCH 7/8] stash: " Elijah Newren via GitGitGadget
2021-11-21 0:47 ` [PATCH 8/8] dir: avoid removing the " Elijah Newren via GitGitGadget
2021-11-23 0:39 ` Glen Choo
2021-11-23 1:19 ` Elijah Newren
2021-11-23 18:19 ` Glen Choo
2021-11-23 19:56 ` Elijah Newren
2021-11-23 20:32 ` Glen Choo
2021-11-23 21:57 ` Junio C Hamano
2021-11-23 23:23 ` Elijah Newren
2021-11-24 5:46 ` Junio C Hamano
2021-11-23 23:13 ` Elijah Newren
2021-11-24 0:39 ` Glen Choo
2021-11-24 5:46 ` Junio C Hamano
2021-11-24 1:10 ` Ævar Arnfjörð Bjarmason
2021-11-24 4:35 ` Elijah Newren
2021-11-24 11:14 ` Ævar Arnfjörð Bjarmason
2021-11-24 14:11 ` Ævar Arnfjörð Bjarmason
2021-11-25 2:54 ` Elijah Newren
2021-11-25 11:12 ` Ævar Arnfjörð Bjarmason
2021-11-26 21:40 ` The overhead of bin-wrappers/ (was: [PATCH 8/8] dir: avoid removing the current working directory) Ævar Arnfjörð Bjarmason
2021-11-24 14:33 ` [PATCH 8/8] dir: avoid removing the current working directory Philip Oakley
2021-11-24 19:46 ` Junio C Hamano
2021-11-25 12:54 ` Philip Oakley
2021-11-25 13:51 ` Ævar Arnfjörð Bjarmason
2021-11-25 2:48 ` Elijah Newren
2021-11-24 19:43 ` Junio C Hamano
2021-11-21 8:11 ` [PATCH 0/8] Avoid removing the current working directory, even if it becomes empty Junio C Hamano
2021-11-25 8:39 ` [PATCH v2 0/9] " Elijah Newren via GitGitGadget
2021-11-25 8:39 ` [PATCH v2 1/9] t2501: add various tests for removing the current working directory Elijah Newren via GitGitGadget
2021-11-25 10:21 ` Ævar Arnfjörð Bjarmason
2021-11-25 8:39 ` [PATCH v2 2/9] setup: introduce startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-25 10:44 ` Ævar Arnfjörð Bjarmason
2021-11-26 17:55 ` Elijah Newren
2021-11-26 6:52 ` Junio C Hamano
2021-11-26 18:01 ` Elijah Newren [this message]
2021-11-29 14:05 ` Derrick Stolee
2021-11-29 17:18 ` Elijah Newren
2021-11-29 17:43 ` Derrick Stolee
2021-11-29 17:42 ` Junio C Hamano
2021-11-25 8:39 ` [PATCH v2 3/9] unpack-trees: refuse to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-25 10:56 ` Ævar Arnfjörð Bjarmason
2021-11-26 18:06 ` Elijah Newren
2021-11-29 14:10 ` Derrick Stolee
2021-11-29 17:26 ` Elijah Newren
2021-11-25 8:39 ` [PATCH v2 4/9] unpack-trees: add special cwd handling Elijah Newren via GitGitGadget
2021-11-29 14:14 ` Derrick Stolee
2021-11-29 17:33 ` Elijah Newren
2021-11-25 8:39 ` [PATCH v2 5/9] symlinks: do not include startup_info->original_cwd in dir removal Elijah Newren via GitGitGadget
2021-11-25 8:39 ` [PATCH v2 6/9] clean: do not attempt to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-25 8:39 ` [PATCH v2 7/9] stash: " Elijah Newren via GitGitGadget
2021-11-25 10:58 ` Ævar Arnfjörð Bjarmason
2021-11-26 18:04 ` Elijah Newren
2021-11-25 8:39 ` [PATCH v2 8/9] dir: avoid incidentally removing the original_cwd in remove_path() Elijah Newren via GitGitGadget
2021-11-25 8:39 ` [PATCH v2 9/9] dir: new flag to remove_dir_recurse() to spare the original_cwd Elijah Newren via GitGitGadget
2021-11-26 22:40 ` [PATCH v3 00/11] Avoid removing the current working directory, even if it becomes empty Elijah Newren via GitGitGadget
2021-11-26 22:40 ` [PATCH v3 01/11] t2501: add various tests for removing the current working directory Elijah Newren via GitGitGadget
2021-11-27 10:32 ` Ævar Arnfjörð Bjarmason
2021-11-27 19:16 ` Elijah Newren
2021-11-26 22:40 ` [PATCH v3 02/11] setup: introduce startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-27 10:35 ` Ævar Arnfjörð Bjarmason
2021-11-27 17:05 ` Elijah Newren
2021-11-27 10:40 ` Ævar Arnfjörð Bjarmason
2021-11-27 18:31 ` Elijah Newren
2021-11-28 18:04 ` Ævar Arnfjörð Bjarmason
2021-11-29 21:58 ` Elijah Newren
2021-11-26 22:40 ` [PATCH v3 03/11] unpack-trees: refuse to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-26 22:40 ` [PATCH v3 04/11] unpack-trees: add special cwd handling Elijah Newren via GitGitGadget
2021-11-26 22:40 ` [PATCH v3 05/11] symlinks: do not include startup_info->original_cwd in dir removal Elijah Newren via GitGitGadget
2021-11-26 22:40 ` [PATCH v3 06/11] clean: do not attempt to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-26 22:40 ` [PATCH v3 07/11] rebase: " Elijah Newren via GitGitGadget
2021-11-29 17:50 ` Derrick Stolee
2021-11-29 19:22 ` Elijah Newren
2021-11-29 19:42 ` Derrick Stolee
2021-11-26 22:40 ` [PATCH v3 08/11] stash: " Elijah Newren via GitGitGadget
2021-11-26 22:41 ` [PATCH v3 09/11] dir: avoid incidentally removing the original_cwd in remove_path() Elijah Newren via GitGitGadget
2021-11-26 22:41 ` [PATCH v3 10/11] dir: new flag to remove_dir_recurse() to spare the original_cwd Elijah Newren via GitGitGadget
2021-11-26 22:41 ` [PATCH v3 11/11] t2501: simplify the tests since we can now assume desired behavior Elijah Newren via GitGitGadget
2021-11-29 17:57 ` [PATCH v3 00/11] Avoid removing the current working directory, even if it becomes empty Derrick Stolee
2021-11-29 22:37 ` [PATCH v4 " Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 01/11] t2501: add various tests for removing the current working directory Elijah Newren via GitGitGadget
2021-11-30 6:47 ` Junio C Hamano
2021-11-30 6:53 ` Elijah Newren
2021-11-29 22:37 ` [PATCH v4 02/11] setup: introduce startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 03/11] unpack-trees: refuse to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 04/11] unpack-trees: add special cwd handling Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 05/11] symlinks: do not include startup_info->original_cwd in dir removal Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 06/11] clean: do not attempt to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 07/11] rebase: " Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 08/11] stash: " Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 09/11] dir: avoid incidentally removing the original_cwd in remove_path() Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 10/11] dir: new flag to remove_dir_recurse() to spare the original_cwd Elijah Newren via GitGitGadget
2021-11-29 22:37 ` [PATCH v4 11/11] t2501: simplify the tests since we can now assume desired behavior Elijah Newren via GitGitGadget
2021-11-29 23:38 ` [PATCH v4 00/11] Avoid removing the current working directory, even if it becomes empty Eric Sunshine
2021-11-30 0:16 ` Elijah Newren
2021-12-01 6:40 ` [PATCH v5 " Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 01/11] t2501: add various tests for removing the current working directory Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 02/11] setup: introduce startup_info->original_cwd Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 03/11] unpack-trees: refuse to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 04/11] unpack-trees: add special cwd handling Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 05/11] symlinks: do not include startup_info->original_cwd in dir removal Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 06/11] clean: do not attempt to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 07/11] rebase: " Elijah Newren via GitGitGadget
2022-01-25 20:26 ` [Bug] Rebase from worktree subdir is broken (was Re: [PATCH v5 07/11] rebase: do not attempt to remove startup_info->original_cwd) Glen Choo
2022-01-25 23:59 ` Elijah Newren
2022-01-26 0:30 ` Glen Choo
2022-01-26 19:04 ` Elijah Newren
2022-01-26 0:32 ` Eric Sunshine
2022-01-26 0:38 ` Eric Sunshine
2022-01-26 0:51 ` Elijah Newren
2022-01-26 1:15 ` Glen Choo
2022-01-26 1:38 ` Elijah Newren
2022-01-26 11:00 ` Phillip Wood
2022-01-26 17:53 ` Eric Sunshine
2022-01-27 11:01 ` Phillip Wood
2022-01-27 20:03 ` Elijah Newren
2022-02-05 11:23 ` Eric Sunshine
2022-02-05 11:42 ` Eric Sunshine
2022-02-05 22:35 ` Elijah Newren
2021-12-01 6:40 ` [PATCH v5 08/11] stash: do not attempt to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 09/11] dir: avoid incidentally removing the original_cwd in remove_path() Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 10/11] dir: new flag to remove_dir_recurse() to spare the original_cwd Elijah Newren via GitGitGadget
2021-12-01 6:40 ` [PATCH v5 11/11] t2501: simplify the tests since we can now assume desired behavior Elijah Newren via GitGitGadget
2021-12-07 16:09 ` [PATCH v5 00/11] Avoid removing the current working directory, even if it becomes empty Derrick Stolee
2021-12-07 18:30 ` Ævar Arnfjörð Bjarmason
2021-12-07 20:57 ` Derrick Stolee
2021-12-08 10:23 ` Ævar Arnfjörð Bjarmason
2021-12-07 20:43 ` Elijah Newren
2021-12-07 21:00 ` Derrick Stolee
2021-12-09 5:08 ` [PATCH v6 " Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 01/11] t2501: add various tests for removing the current working directory Elijah Newren via GitGitGadget
2022-03-11 11:57 ` Ævar Arnfjörð Bjarmason
2021-12-09 5:08 ` [PATCH v6 02/11] setup: introduce startup_info->original_cwd Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 03/11] unpack-trees: refuse to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 04/11] unpack-trees: add special cwd handling Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 05/11] symlinks: do not include startup_info->original_cwd in dir removal Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 06/11] clean: do not attempt to remove startup_info->original_cwd Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 07/11] rebase: " Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 08/11] stash: " Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 09/11] dir: avoid incidentally removing the original_cwd in remove_path() Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 10/11] dir: new flag to remove_dir_recurse() to spare the original_cwd Elijah Newren via GitGitGadget
2021-12-09 5:08 ` [PATCH v6 11/11] t2501: simplify the tests since we can now assume desired behavior Elijah Newren via GitGitGadget
2021-11-30 11:04 ` [PATCH v3 00/11] Avoid removing the current working directory, even if it becomes empty Phillip Wood
2021-12-01 0:03 ` Elijah Newren
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=CABPp-BG8narMYNeaBHQfhcOFJ4hHJM+HGoy7S4hyOA-7PbOEPA@mail.gmail.com \
--to=newren@gmail.com \
--cc=avarab@gmail.com \
--cc=chooglen@google.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
--cc=peff@peff.net \
--cc=philipoakley@iee.email \
/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).