From: Elijah Newren <newren@gmail.com>
To: Jonathan Tan <jonathantanmy@google.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
"Junio C Hamano" <gitster@pobox.com>,
"Ævar Arnfjörð" <avarab@gmail.com>,
jabolopes@google.com
Subject: Re: [PATCH v2] sparse-checkout: create leading directory
Date: Fri, 21 Jan 2022 18:33:18 -0800 [thread overview]
Message-ID: <CABPp-BG9rxJsA9XwgUxpny5TDW4W7Z85PAkBdzKJXrSJxAim1g@mail.gmail.com> (raw)
In-Reply-To: <20220121174441.3991963-1-jonathantanmy@google.com>
On Fri, Jan 21, 2022 at 6:05 PM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> When creating the sparse-checkout file, Git does not create the leading
> directory, "$GIT_DIR/info", if it does not exist. This causes problems
> if the repository does not have that directory. Therefore, ensure that
> the leading directory is created.
>
> This is the only "open" in builtin/sparse-checkout.c that does not have
> a leading directory check. (The other one in write_patterns_and_update()
> does.)
>
> Note that the test needs to explicitly specify a template when running
> "git init" because the default template used in the tests has the
> "info/" directory included.
If wanted, you could avoid that by using `git worktree add ...`;
git-worktree will create a $GIT_DIR that does not contain an info
subdirectory.
(No need to resubmit or anything, mostly I'm just mentioning another
way folks might have triggered this issue.)
> Helped-by: Jose Lopes <jabolopes@google.com>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
> Changes from v1:
> - made test simpler
> - added attribution to Jose Lopes for finding and making the first
> draft of this patch (after confirming with them)
>
> Ævar mentioned "git sparse-checkout add" but I think that that is a
> different problem - in the "git sparse-checkout init" case, we could get
> into this case with a template that does not have .git/info, but in the
> "git sparse-checkout add" case, the user would have had to explicitly
> remove the info directory. So I'll limit this patch to the "init" case,
> for now.
> ---
> builtin/sparse-checkout.c | 3 +++
> t/t1091-sparse-checkout-builtin.sh | 6 ++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c
> index 679c107036..2b0e1db2d2 100644
> --- a/builtin/sparse-checkout.c
> +++ b/builtin/sparse-checkout.c
> @@ -471,6 +471,9 @@ static int sparse_checkout_init(int argc, const char **argv)
> FILE *fp;
>
> /* assume we are in a fresh repo, but update the sparse-checkout file */
> + if (safe_create_leading_directories(sparse_filename))
> + die(_("unable to create leading directories of %s"),
> + sparse_filename);
> fp = xfopen(sparse_filename, "w");
> if (!fp)
> die(_("failed to open '%s'"), sparse_filename);
> diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh
> index 42776984fe..3189d3da96 100755
> --- a/t/t1091-sparse-checkout-builtin.sh
> +++ b/t/t1091-sparse-checkout-builtin.sh
> @@ -79,6 +79,12 @@ test_expect_success 'git sparse-checkout init' '
> check_files repo a
> '
>
> +test_expect_success 'git sparse-checkout init in empty repo' '
> + test_when_finished rm -rf empty-repo blank-template &&
> + git init --template= empty-repo &&
> + git -C empty-repo sparse-checkout init
> +'
> +
> test_expect_success 'git sparse-checkout list after init' '
> git -C repo sparse-checkout list >actual &&
> cat >expect <<-\EOF &&
> --
> 2.35.0.rc0.227.g00780c9af4-goog
Patch looks good to me:
Reviewed-by: Elijah Newren <newren@gmail.com>
next prev parent reply other threads:[~2022-01-22 2:33 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-20 18:55 [PATCH] sparse-checkout: create leading directory Jonathan Tan
2022-01-20 20:26 ` Junio C Hamano
2022-01-20 21:30 ` Ævar Arnfjörð Bjarmason
2022-01-22 7:58 ` SZEDER Gábor
2022-01-21 17:44 ` [PATCH v2] " Jonathan Tan
2022-01-21 18:48 ` Ævar Arnfjörð Bjarmason
2022-01-22 5:21 ` Elijah Newren
2022-01-22 12:06 ` Ævar Arnfjörð Bjarmason
2022-01-22 17:29 ` Elijah Newren
2022-01-24 11:17 ` Ævar Arnfjörð Bjarmason
2022-01-24 18:25 ` Jonathan Tan
2022-01-25 5:37 ` Elijah Newren
2022-01-22 2:33 ` Elijah Newren [this message]
2022-01-24 18:22 ` Jonathan Tan
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABPp-BG9rxJsA9XwgUxpny5TDW4W7Z85PAkBdzKJXrSJxAim1g@mail.gmail.com \
--to=newren@gmail.com \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jabolopes@google.com \
--cc=jonathantanmy@google.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.
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 NNTP newsgroup(s).