Skip to content

t1900: add tests for git repo structure subcommand#2064

Open
MansiSingh17 wants to merge 13 commits intogitgitgadget:masterfrom
MansiSingh17:repo-add-structure-tests-v2
Open

t1900: add tests for git repo structure subcommand#2064
MansiSingh17 wants to merge 13 commits intogitgitgadget:masterfrom
MansiSingh17:repo-add-structure-tests-v2

Conversation

@MansiSingh17
Copy link

No description provided.

ptarjan and others added 12 commits January 1, 2026 22:05
The fsmonitor event tests (edit, create, delete, rename, etc.) were
flaky because there can be a race between the daemon writing events
to the trace file and the test's grep commands checking for them.

Add a retry_grep() helper function (similar to retry_until_success
in lib-git-p4.sh) that retries grep with a timeout, and use it in
all event-checking tests to wait for one expected event before
checking the rest.

Signed-off-by: Paul Tarjan <github@paulisageek.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `core.attributeFile` config value is parsed in
git_default_core_config(), loaded eagerly and stored in the global
variable `git_attributes_file`. Storing this value in a global
variable can lead to it being overwritten by another repository when
more than one Git repository run in the same Git process.

Create a new struct `repo_config_values` to hold this value and
other repository dependent values parsed by `git_default_config()`.
This will ensure the current behaviour remains the same while also
enabling the libification of Git.

An accessor function 'repo_config_values()' s created to ensure
that we do not access an uninitialized repository, or an instance
of a different repository than the current one.

Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The `test-tool genrandom` test helper can be used to generate random
data, either as an infinite stream or with a specified number of bytes.
The way we handle parsing the number of bytes is lacking though:

  - We don't have good error handling, so if the caller for example uses
    `test-tool genrandom 200xyz` then we'll end up generating 200 bytes
    of random data successfully.

  - Many callers want to generate e.g. 1 kilobyte or megabyte of data,
    but they have to either use unwieldy numbers like 1048576, or they
    have to precompute them.

Fix both of these issues by using `git_parse_ulong()` to parse the
argument. This function has better error handling, and it knows to
handle unit suffixes.

Adapt a couple of our tests to use suffixes instead of manual
computations.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `stream_object_signature()` is responsible for verifying
whether the given object ID matches the actual hash of the object's
contents. In contrast to `check_object_signature()` it does so in a
streaming fashion so that we don't have to load the full object into
memory.

In a subsequent commit we'll want to adapt one of its callsites to pass
a preconstructed stream. Prepare for this by accepting a stream as input
that the caller needs to assemble.

While at it, improve the error reporting in `parse_object_with_flags()`
to tell apart the two failure modes.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `packfile_store_read_object_stream()` takes as input an
object ID and then constructs a `struct odb_read_stream` from it. In a
subsequent commit we'll want to create an object stream for a given
combination of packfile and offset though, which is not something that
can currently be done.

Extract a new function `packfile_read_object_stream()` that makes this
functionality available.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It was reported [1] that git-fsck(1) may sometimes run into an infinite
loop when processing packfiles. This bug was bisected to c31bad4
(packfile: track packs via the MRU list exclusively, 2025-10-30), which
refactored our lsit of packfiles to only be tracked via an MRU list,
exclusively. This isn't entirely surprising: any caller that iterates
through the list of packfiles and then hits `find_pack_entry()`, for
example because they read an object from it, may cause the MRU list to
be updated. And if the caller is unlucky, this may cause the mentioned
infinite loop.

While this mechanism is somewhat fragile, it is still surprising that we
encounter it when verifying the packfile. We iterate through objects in
a given pack one by one and then read them via their offset, and doing
this shouldn't ever end up in `find_pack_entry()`.

But there is an edge case here: when the object in question is a blob
bigger than "core.largeFileThreshold", then we will be careful to not
read it into memory. Instead, we read it via an object stream by calling
`odb_read_object_stream()`, and that function will perform an object
lookup via `odb_read_object_info()`. So in the case where there are at
least two blobs in two different packfiles, and both of these blobs
exceed "core.largeFileThreshold", then we'll run into an infinite loop
because we'll always update the MRU.

We could fix this by improving `repo_for_each_pack()` to not update the
MRU, and this would address the issue. But the fun part is that using
`odb_read_object_stream()` is the wrong thing to do in the first place:
it may open _any_ instance of this object, so we ultimately cannot be
sure that we even verified the object in our given packfile.

Fix this bug by creating the object stream for the packed object
directly via `packfile_read_object_stream()`. Add a test that would have
caused the infinite loop.

[1]: <20260222183710.2963424-1-sandals@crustytoothpaste.net>

Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The config value `core.sparseCheckout` is parsed in
`git_default_core_config()` and stored globally in
`core_apply_sparse_checkout`. This could cause it to be overwritten
by another repository when different Git repositories run in the same
process.

Move the parsed value into `struct repo_config_values` in the_repository
to retain current behaviours and move towards libifying Git.

Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
…lues`

The config value `branch.autoSetupMerge` is parsed in
`git_default_branch_config()` and stored in the global variable
`git_branch_track`. This global variable can be overwritten
by another repository when multiple Git repos run in the the same process.

Move this value into `struct repo_config_values` in the_repository to
retain current behaviours and move towards libifying Git.
Since the variable is no longer a global variable, it has been renamed to
`branch_track` in the struct `repo_config_values`.

Suggested-by: Phillip Wood <phillip.wood123@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by: Usman Akinyemi <usmanakinyemi202@gmail.com>
Signed-off-by: Olamide Caleb Bello <belkid98@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Test fixup.

* pt/t7527-flake-workaround:
  t7527: fix flaky fsmonitor event tests with retry logic
The core.attributesfile is intended to be set per repository, but
were kept track of by a single global variable in-core, which has
been corrected by moving it to per-repository data structure.

* ob/core-attributesfile-in-repository:
  environment: move "branch.autoSetupMerge" into `struct repo_config_values`
  environment: stop using core.sparseCheckout globally
  environment: stop storing `core.attributesFile` globally
"fsck" iterates over packfiles and its access to pack data caused
the list to be permuted, which caused it to loop forever; the code
to access pack data by "fsck" has been updated to avoid this.

* ps/fsck-stream-from-the-right-object-instance:
  pack-check: fix verification of large objects
  packfile: expose function to read object stream for an offset
  object-file: adapt `stream_object_signature()` to take a stream
  t/helper: improve "genrandom" test helper
Signed-off-by: Junio C Hamano <gitster@pobox.com>
@gitgitgadget
Copy link

gitgitgadget bot commented Mar 6, 2026

There are merge commits in this Pull Request:

394c18092f3e2a773333a08d3f8241322a97aa39
db227bce2224b55b11954a5f292a0b035b7d9279
d93be9cbcaf80f4d1ff56a3d7aa5d722236b6eac

Please rebase the branch and force-push.

The t1900 test file covers git repo info thoroughly but has
no tests for the git repo structure subcommand. Add basic
tests to verify that:

- git repo structure succeeds and produces no stderr output
- git repo structure --format=keyvalue outputs expected keys
- git repo structure --format=nul succeeds
- git repo structure rejects an unknown format

Signed-off-by: Mansi Singh <mansimaanu8627@gmail.com>
@MansiSingh17 MansiSingh17 force-pushed the repo-add-structure-tests-v2 branch from b706d53 to 999ca58 Compare March 7, 2026 03:43
@gitgitgadget
Copy link

gitgitgadget bot commented Mar 7, 2026

There are merge commits in this Pull Request:

394c18092f3e2a773333a08d3f8241322a97aa39
db227bce2224b55b11954a5f292a0b035b7d9279
d93be9cbcaf80f4d1ff56a3d7aa5d722236b6eac

Please rebase the branch and force-push.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants