git/list[1] front-page[2] threads[3] people[4] search[5] about
wed 2026-10-07 16:09 UTC

Re: [PATCH v2 10/13] builtin/multi-pack-index: refuse unknown sources with "--object-dir="

From
Justin Tobler <jltobler@gmail.com>
Date
Sep 8, 2026, 23:12 UTC
Message-ID
<aqCU7e0c84ZgZNab@denethor>
In-Reply-To
<20260902-pks-odb-registering-in-memory-sources-v2-10-c6ca12fdea4d@pks.im>
On 26/09/02 03:34PM, Patrick Steinhardt wrote:
Show 20 quoted lines
> Users can tell git-multi-pack-index(1) to access multi-pack indices that
> are stored in a different object directory via the "--object-dir="
> option. This allows them to for example write or verify a multi-pack
> index other than the one located in the main object directory in case a
> repository has alternates with multiple multi-pack indices.
> 
> But while the documentation explicitly points out that the specified
> object directory must be an alternate of the current repository, we
> never verify that property. Instead, starting with 017db7bb14 (midx:
> load multi-pack indices via their source, 2025-08-11), we now construct
> an ad-hoc source and link it to the main object directory.
> 
> Besides contradicting the documentation, it's dubious that this really
> ought to work in the first place: creating a multi-pack index (and
> potentially a bitmap) for a completely foreign object directory is of
> questionable value, as bitmap commit selection operates on the invoking
> repository's refs. Furthermore, this is the only remaining caller
> outside of our test helpers that constructs an ad-hoc source and links
> it to the database, and we want to get rid of this mechanism as part of
> this series.

I was curious if there was any intentional reason that 017db7bb14 started added these as an alternate source. I assume though the reason was just to address the tests when we started loading multi-pack indexes via sources. So aligning with the prexisting documentation makes sense to me.

Show 29 quoted lines
> Stop constructing the ad-hoc source and instead refuse the operation.
> While this results in a change in behaviour, this restriction has been
> documented as such ever since f57a739691 (midx: avoid opening multiple
> MIDXs when writing, 2021-09-01).
> 
> Note that this change requires us to adapt one test chain in t5319, as
> it creates an object directory that is not connected to any repository
> and then uses it via "--object-dir=". The setup itself already documents
> this and does the necessary gymnastics to link the object directory to a
> temporary repository, but subsequent tests don't. Adapt those tests to
> retain and reuse the temporary repository.
> 
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/multi-pack-index.c  | 3 ++-
>  t/t5319-multi-pack-index.sh | 9 ++++-----
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
> index 6e73c85cde..753bd53a70 100644
> --- a/builtin/multi-pack-index.c
> +++ b/builtin/multi-pack-index.c
> @@ -90,7 +90,8 @@ static struct odb_source_files *handle_object_dir_option(struct repository *repo
>  {
>  	struct odb_source *source = odb_find_source(repo->objects, opts.object_dir);
>  	if (!source)
> -		source = odb_add_to_alternates_memory(repo->objects, opts.object_dir);
> +		die(_("object directory is not an alternate of the current repository: '%s'"),
> +		    opts.object_dir);
Now we no longer add these as in-memory alternates sources. Looks good.
Show 13 quoted lines
>  	return odb_source_files_downcast(source);
>  }
>  
> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
> index 68143cb5b7..00e90f163f 100755
> --- a/t/t5319-multi-pack-index.sh
> +++ b/t/t5319-multi-pack-index.sh
> @@ -698,10 +698,9 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
>  	corrupt_data $idx64 $(test_oid idxoff) "\02" &&
>  	# objects64 is not a real repository, but can serve as an alternate
>  	# anyway so we can write a MIDX into it
> -	git init repo &&
> -	test_when_finished "rm -fr repo" &&
Ok, now we just reuse the properly set up repo. Make sense.
Show 30 quoted lines
> +	git init repo64 &&
>  	(
> -		cd repo &&
> +		cd repo64 &&
>  		( cd ../objects64 && pwd ) >.git/objects/info/alternates &&
>  		midx64=$(git multi-pack-index --object-dir=../objects64 write)
>  	) &&
> @@ -709,7 +708,7 @@ test_expect_success 'force some 64-bit offsets with pack-objects' '
>  '
>  
>  test_expect_success 'verify multi-pack-index with 64-bit offsets' '
> -	git multi-pack-index verify --object-dir=objects64
> +	git -C repo64 multi-pack-index verify --object-dir=../objects64
>  '
>  
>  NUM_OBJECTS=63
> @@ -721,7 +720,7 @@ MIDX_BYTE_LARGE_OFFSET=$(($MIDX_OFFSET_LARGE_OFFSETS + 3))
>  
>  test_expect_success 'verify incorrect 64-bit offset' '
>  	corrupt_midx_and_verify $MIDX_BYTE_LARGE_OFFSET "\07" objects64 \
> -		"incorrect object offset"
> +		"incorrect object offset" "git -C repo64 multi-pack-index verify --object-dir=../objects64"
>  '
>  
>  test_expect_success 'setup expire tests' '
> 
> -- 
> 2.55.0.979.g7e5102b832.dirty
> 
> 
Previous: Justin ToblerNext: Patrick Steinhardt
Message 22 of 23 in “odb: stop registering in-memory sources”
  1. 00/13 odb: stop registering in-memory sourcesPatrick Steinhardt, Sep 2, 2026
  2. 01/13 cache-tree: drop `the_repository` in `cache_tree_fully_valid()`Patrick Steinhardt, Sep 2, 2026
  3. 02/13 cache-tree: remove dependency on `the_repository`Patrick Steinhardt, Sep 2, 2026
  4. 03/13 submodule-config: remove uses of `the_repository`Patrick Steinhardt, Sep 2, 2026
  5. 04/13 submodule-config: stop using `the_hash_algo`Patrick Steinhardt, Sep 2, 2026
  6. 05/13 submodule-config: stop registering submodule sourcesPatrick Steinhardt, Sep 2, 2026
  7. 06/13 builtin/grep: stop registering submodule ODB as sourcePatrick Steinhardt, Sep 2, 2026
  8. 07/13 odb: remove infrastructure to register submodule sourcesPatrick Steinhardt, Sep 2, 2026
  9. 08/13 tmp-objdir: drop unused function to register alternatePatrick Steinhardt, Sep 2, 2026
  10. 09/13 odb/packed: fix memory leaks when freeing sourcePatrick Steinhardt, Sep 2, 2026
  11. 10/13 builtin/multi-pack-index: refuse unknown sources with "--object-dir="Patrick Steinhardt, Sep 2, 2026
  12. 11/13 t/helper: adapt read-midx to not link ad-hoc source anymorePatrick Steinhardt, Sep 2, 2026
  13. 12/13 t/helper: stop registering alternates in "ref-store" commandPatrick Steinhardt, Sep 2, 2026
  14. 13/13 odb: remove the ability to link sources ad-hocPatrick Steinhardt, Sep 2, 2026
  15. Karthik NayakSep 4, 2026
  16. Karthik NayakSep 4, 2026
  17. Karthik NayakSep 4, 2026
  18. Justin ToblerSep 6, 2026
  19. Patrick SteinhardtSep 7, 2026
  20. Patrick SteinhardtSep 7, 2026
  21. Justin ToblerSep 8, 2026
  22. Justin ToblerSep 8, 2026
  23. Patrick SteinhardtSep 9, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.