git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Derrick Stolee <stolee@gmail.com>
To: Johannes Berg <johannes@sipsolutions.net>, git@vger.kernel.org
Cc: "Signed-off-by : Taylor Blau" <me@ttaylorr.com>
Subject: Re: [PATCH] multi-pack-index: fix --object-dir from outside repo
Date: Sun, 22 Aug 2021 19:51:01 -0400	[thread overview]
Message-ID: <04ed58aa-94fa-010e-f4db-f41cd51876a5@gmail.com> (raw)
In-Reply-To: <20210820193504.37044-1-johannes@sipsolutions.net>

On 8/20/2021 3:35 PM, Johannes Berg wrote:
> If using --object-dir to point into a repo, 'write' will
> segfault trying to access the object-dir via the repo it
> found, but that's not fully initialized. Fix it to use
> the object_dir properly.

Thanks for finding this! It's difficult to cover all of
the cases, but I'm glad you found this and added a test.

> +test_expect_success 'multi-pack-index with --object-dir need not be in repo' '
> +	p="$(pwd)" &&
> +	rm -f $objdir/multi-pack-index &&
> +	cd / &&
> +	git multi-pack-index --object-dir="$p/$objdir" write &&
> +	cd "$p"

Why are you using "cd /" here? Even if you mean to use "cd",
please do so within a sub-shell.

Could you instead init a new repo within the current directory
and point the object-dir to that location?

It could look something like this, (warning: I did not test this)

	git init other &&
	test_commit -C other first &&
	git multi-pack-index --object-dir=other/.git/objects write

And is the only post-condition you are checking that we do not
crash? Or is there a specific result you are looking for? For
instance, we can double check that the MIDX was written:

	test_path_is_file other/.git/objects/pack/multi-pack-index

but also you seem to be touching areas that delete files. Could
we 'touch' some of those and then see them get deleted?

Thanks,
-Stolee

  reply	other threads:[~2021-08-22 23:51 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-20 19:35 [PATCH] multi-pack-index: fix --object-dir from outside repo Johannes Berg
2021-08-22 23:51 ` Derrick Stolee [this message]
2021-08-23  7:21   ` Johannes Berg
2021-08-23  8:05     ` Junio C Hamano
2021-08-23  8:10       ` Johannes Berg
2021-08-23 13:19         ` Derrick Stolee
2021-08-23 13:40           ` Johannes Berg
2021-08-23 16:16             ` Junio C Hamano
2021-08-23 16:06         ` Junio C Hamano
2021-08-23  0:54 ` Taylor Blau
2021-08-23  7:32   ` Johannes Berg

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=04ed58aa-94fa-010e-f4db-f41cd51876a5@gmail.com \
    --to=stolee@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes@sipsolutions.net \
    --cc=me@ttaylorr.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).