[pull] master from git:master - #259
Merged
Merged
Conversation
…-fsck * ps/odb-pluggable-pack-generation: bundle: generate packfiles via the object database bundle: get (mostly) rid of `the_repository` builtin/bundle: refactor option handling for progress meter send-pack: generate packfiles via the object database upload-pack: generate packfiles via the object database odb: introduce interface to generate packfiles
* ps/odb-eagerly-load-alternates: odb: drop `alternates_db` field odb: drop `loaded_alternates` field odb: eagerly initialize alternates odb: decouple source path comparisons from `the_repository` setup: create ref and object databases after config is written
…istering-in-memory-sources * ty/repository-fetch-if-missing: repository: move fetch_if_missing into struct repository
The function `cache_tree_fully_valid()` verifies whether the cache tree owned by the index is valid or not. As part of that, the function checks whether the objects referenced by the cache all exist. But because the function has no repository available, it is using the object database of `the_repository` instead. We could of course adapt callers to pass in a repository as parameter explicitly to get rid of this implicit dependency on global state. But all of them pass the cache tree owned by a `struct index_state`, and that structure already has a reference to its owning repository. So instead, adapt the function to accept a `struct index_state`, which ensures that callers will implicitly always pass the correct repository. Adapt callers accordingly. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The "cache-tree" subsystem still depends on `the_repository`. Adapt it to instead use repositories provided via the context, either as a new parameter or the one passed in via `struct index_state`. Besides getting rid of `the_repository`, this also removes the last dependency on registering submodule sources with the main object database. When reading gitmodules from a submodule's index we implicitly read that object via `the_repository`'s object database, which is of course wrong. This works though because we would then register the submodule's object database with the main object database, but a later patch is going to get rid of that mechanism. You can verify that we indeed no longer depend on this mechanism by running tests with `GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB=true`. Without this patch we fail in t1092, with this patch we never register submodule object databases anymore. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Several functions in the submodule-config subsystem implicitly depend on `the_repository`. Refactor these to take a `struct repository` as parameter and adapt callers accordingly. Note that as usual with these refactorings, callers simply pass `the_repository` even if they already have a different repository available in the calling context. This simplifies the migration and ensures that we don't have a change in behaviour. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We have two uses of `the_hash_algo` in "submodule-config.c":
- One trivial use in `gitmodules_cb`, which we can convert to use the
hash algorithm of the repository that's already available in the
caller's context.
- One use where we compute the hashmap key of an object ID. We should
only ever get valid, populated object IDs here, and consequently we
can easily adapt that function to use the hash algorithm of the
passed-in object ID.
Adapt both sites accordingly. Safeguard us against the case where the
passed-in object ID is _not_ properly initialized. While this case
shouldn't ever happen, it doesn't hurt to be defensive.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When reading the ".gitmodules" file from a blob in a repository other than `the_repository`, we register that repository's object database as an in-memory source of `the_repository`'s object database. This call has its origins in d9b8b8f (submodule-config.c: use repo_get_oid for reading .gitmodules, 2019-04-16): back then, `config_with_options()` was not able to read a blob from an arbitrary repository, but would always read it via `the_repository`. So even though the blob could be resolved in the submodule repository via `repo_get_oid()`, the submodule's object database had to be registered as an in-memory source of `the_repository` so that the subsequent object read was able to find the blob at all. That need went away with e3e8bf0 (submodule-config: pass repo upon blob config read, 2021-08-16), which taught the config machinery to read the blob from the repository we pass to it. The same series converted the eager submodule source registration into a lazy mechanism that only registers submodule sources with the object database when an object lookup failed. The intent though was that we don't ever have to fall back to this mechanism in the first place, and to verify that this is the case we introduced GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB. If set, then any such lazy registration would cause us to BUG. At the beginning of this series, we still triggered this bug in t1092. But now that we have converted the "cache-tree" subsystem to not depend on `the_repository` anymore it also knows to properly access objects via the submodule. With that change, GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB does not cause any failures anymore. Remove the call to `odb_add_submodule_source_by_path()`. This removes the last user of `the_repository`, so at the same time we can also get rid of `USE_THE_REPOSITORY_VARIABLE`. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Same as with the preceding commit, git-grep(1) registers each submodule's object database as an in-memory source of the main object database before grepping it. This was introduced as an eager alternate registration and converted into the lazy mechanism via 8d33c3a (grep: use submodule-ODB-as-alternate lazy-addition, 2021-08-16). Starting with 0693806 (grep: add repository to OID grep sources, 2021-08-16), the command instead knows to pass submodule repositories to our workers, which means that those now use that repository to look up objects, too. As a consequence, registering submodule sources as alternates is not required anymore. Remove the logic to register submodule sources. Unfortunately, this does not allow us to get rid of the object read lock as initializing the subrepository is still racy. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The preceding commits have removed the last two users of `odb_add_submodule_source_by_path()`. The mechanism was only ever meant as a transitional crutch while migrating submodule object access away from "add the submodule ODB as an alternate of the_repository" towards explicitly passing the submodule repository, see a35e03d (submodule: lazily add submodule ODBs as alternates, 2021-08-16). Remove it. As GIT_TEST_FATAL_REGISTER_SUBMODULE_ODB is now a no-op, remove its documentation and the exports from the test suite, as well. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The last caller of `tmp_objdir_add_as_alternate()` went away in bdee7b3 (builtin/receive-pack: stage incoming objects via ODB transactions, 2026-07-10) and is unused now. Remove the function. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When freeing a "packed" source we don't close either its packs nor its multi-pack indices. This can cause memory leaks in case we create an ad-hoc packed source. As we used to always link packed sources to the main object database we never noticed this issue until now, but it's going to surface in subsequent commits where we stop linking them. Plug the memory leaks by closing the source first. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 017db7b (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. 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 f57a739 (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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Same as in the preceding commit, refactor the setup of ad-hoc object database sources when accessing a multi-pack index in an arbitrary location to not link the newly created source into the main object database anymore. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When using the "ref-store" command we support access to multiple different reference stores. As part of that we allow the caller to explicitly exercise stores of a submodule. This allows us to verify low-level behaviour of submodule stores, which is exercised in t1406. When doing so we also link the submodule's object database into the main object database. The intent of this is that it allows us to access objects of the submodule, too. But that functionality is not even needed anymore: when creating a submodule reference store, we will first initialize the submodule repository and then initialize the store with that repository. And as the reference subsystem doesn't depend on `the_repository` anymore all subsequent object lookups performed by the reference store will be routed to the submodule repository. It is thus not needed anymore to register the submodule object store with the main object database. Remove the call. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Over the course of this patch series we have adapted all callers of `odb_add_to_alternates_memory()` to not do so anymore. Remove the function. This series of refactorings doesn't only simplify our code base. More importantly, with those changes in place we can now unconditionally assume that the list of sources linked to the object database only consists of the primary source and its alternates. This serves as the foundation to eventually move handling of alternates into the "files" backend itself. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When checking loose objects we manually parse the object buffer we have read from the on-disk file, mark the object and then call `fsck_obj()`. The exact same steps are also performed by `fsck_obj_buffer()`. Stop open-coding this logic and call `fsck_obj_buffer()` instead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The interfaces of the functions `fsck_obj()` and `fsck_obj_buffer()` are somewhat similar to one another. The only difference between those two is that `fsck_obj()` takes an already-parsed object as input, whereas `fsck_obj_buffer()` parses the buffer and then calls `fsck_obj()`. Furthermore, `fsck_obj()` has no callers other than `fsck_obj_buffer()`. Refactor the code by merging those two functions. This makes it obvious which function does what, and it allows us to get rid of the early return in `fsck_obj()` in case `SEEN` is set as the only caller unconditionally clears that bit before calling it anyway. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In subsequent commits we're about to rework some of the option handling in git-fsck(1) a bit. It is currently a bit of a mess though due to lots of global state that makes it hard to see which flags are used where exactly. Refactor the code by moving the fsck options into `cmd_fsck()`. This allows us to convert some of the options into function-local variables. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
According to git-fsck(1), the "--full" option behaves in the following
way:
Check not just objects in GIT_OBJECT_DIRECTORY ($GIT_DIR/objects), but
also the ones found in alternate object pools listed in
GIT_ALTERNATE_OBJECT_DIRECTORIES or $GIT_DIR/objects/info/alternates,
and in packed Git archives found in $GIT_DIR/objects/pack and
corresponding pack subdirectories in alternate object pools.
So ultimately, it is supposed to control two things: (1) whether we only
check the main object directory, and (2) whether we check packfiles.
In its current state though, the flag only controls whether we check
packfiles or not, and if so we verify packfiles of all attached sources.
But we also have checks for loose objects in git-fsck(1), and here we
unconditionally check them in all sources.
The flag is arguably conflating two unrelated concerns with one another,
and it really should be split up into two flags: one that controls how
thorough we want to check individual sources, and one that controls
which sources we want to check in the first place. So ideally, we would
have:
- "--include-alternates": check all sources, not only the local one.
- "--include-optimized-objects": check not only loose objects, but
also those that have been packed. Note that we explicitly don't say
"--include-packed-objects" here to be more backend-agnostic.
- "--full": implies both of the above flags.
This feels out of scope for this series though. So for now, simply fix
the code by honoring locality of the sources for loose objects.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The on-disk consistency checks in git-fsck(1) are conceptually backend-specific: while connectivity checks and object-level parsing checks are generic, verifying the physical integrity of packfiles and loose objects is meaningful only to backends that use these formats: Having these checks live in "builtin/fsck.c" violates that layering, because it forces the command to reach directly into format-specific internals. Provide new infrastructure to make these format-specific checks pluggable and implement stubs for the different source types we already have. In subsequent commits we'll move functionality over piece by piece. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Move the packfile verification out of `cmd_fsck()` and into the "packed" source. While doing so, thread the progress meter and object callback through the newly introduced `struct odb_fsck_options` so that the caller's preferences are honoured without exposing those details at the "builtin/fsck.c" level. Note that the old code reported failures when verifying packfiles with the `ERROR_PACK` bit, which gets returned to the caller via the exit code. This bit is neither exercised in our test suite nor is it documented anywhere in our codebase. Furthermore, this bit is highly specific to the object storage backend, which makes it a bad fit for the new pluggable infrastructure. So instead of retaining these semantics, we drop them and return the generic `ERROR_OBJECT` bit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for reverse indexes live in `check_pack_rev_indexes()`, which
is hosted in "builtin/fsck.c". These checks are obviously specific to
the "packed" backend.
Move the logic into `odb_source_packed_fsck()`. As in the preceding
commit, drop the dedicated `ERROR_PACK_REV_INDEX` bit and instead use
the generic `ERROR_OBJECT` bit.
Note that this changes behaviour in two ways:
- The checks are now skipped when "--connectivity-only" was passed.
This is because we don't even run `odb_fsck()` at all when that
flag has been passed by the user, and not verifying data structures
of the object database matches the documented intent of that flag,
which is to only check the connectivity of reachable objects.
- The checks are now skipped for non-local sources when "--no-full"
was passed. This is, again, in line with the documented intent of
that flag.
Add a test to cast these semantics into stone.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for bitmaps live in `verify_bitmap_files()`, which is called by "builtin/fsck.c". These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in preceding commits, this means that we now properly honor both "--connectivity-only" and "--no-full". Furthermore, we drop the dedicated `ERROR_BITMAP` bit and instead use the generic `ERROR_OBJECT` bit. Note that this change also adapts `verify_bitmap_files()` to be focused on a single "packed" source instead of verifying bitmaps from all sources. This change is required as we already know to loop around the sources in `odb_fsck()` itself. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The checks for multi-pack indexes are hosted in `cmd_fsck()` directly. These checks are obviously specific to the "packed" backend. Move the logic into `odb_source_packed_fsck()`. As in preceding commits, this means that we now properly honor both "--connectivity-only" and "--no-full". Furthermore, we drop the dedicated `ERROR_MULTI_PACK_INDEX` bit and instead use the generic `ERROR_OBJECT` bit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The consistency checks for loose objects are hosted by "builtin/fsck.c". These checks are obviously specific to the "loose" backend. Move the logic into `odb_source_loose_fsck()`. Introduce a new "verbose" flag so that we can properly retain semantics around whether or not we want to print some status messages. Note that this fixes a bug as a side effect: the progress meter was captured in the callback data before `start_progress()` was even called, so the per-subdirectory progress updates always operated on a NULL pointer and the meter jumped straight from 0 to 256 upon completion. The new code only sets up the callback data's progress meter after it has been created, so the progress display now advances incrementally again. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
As of a year or two ago, there is this push to align MSYS2 more closely with Cygwin, so as to benefit from a closer collaboration. Part of that is that the triplet `x86_64-pc-cygwin` is used nowadays, whereas it had been `x86_64-pc-msys` previously. Likewise, Perl now reports `$^O` as `cygwin` instead of `msys`. The Perl module test used `msys` as tell-tale when to accommodate for a native Windows version of `git.exe` which would report absolute _Windows_ paths rather than those pseudo-Unix paths. We cannot use that tell-tale anymore, and we also cannot adjust it to `cygwin` because that would break in Cygwin (where `git.exe` reports absolute pseudo-Unix paths). Let's use the environment variable `MSYSTEM` instead (being mindful that the `MSYSTEM=MSYS` variant would _also_ reflect a setup where `git.exe` won't report absolute _Windows_ paths). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The assumption of this test is that Perl and Git have the same idea how to perform encoding conversions. However, in Git for Windows, Git is a native Win32 program, and such programs have a very different concept of encodings (called "Code Pages", and they are not controlled via environment variables at all), whereas the Perl interpreter used in Git for Windows is a pseudo-Unix one that uses the MSYS2 runtime (which _does_ try very much to abide by Unix' `LC_ALL` and friends, and totally ignores Windows' current or active code pages). As such, these test cases _cannot_ work with Git for Windows. So let's just skip them on that platform. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Git for Windows needs to ship with a lot of Unix tools that Git takes
for granted, such as `sed`, `awk`, a C compiler and a Unix shell, just
to name a few. In Git for Windows, these are provided by the MSYS2
project.
Part of these tools (such as `bash.exe`) use a POSIX emulation layer
("MSYS2 runtime", a friendly fork of the Cygwin runtime), but others
target a native Win32 toolchain, e.g. `git.exe`. There are multiple
flavors of that toolchain, and historically Git for Windows used MINGW64
on x64 Windows. This toolchain uses the old MSVC runtime, and therefore
the MSYS2 project deprecated it.
As a consequence, Git for Windows switches to UCRT64 with v2.56.0. That
flavor still uses GCC to compile native Win32 binaries, but targets the
Universal C Runtime ("UCRT"). Internally, this means that the new
`git.exe` is installed into a new prefix, `/ucrt64/`, whereas the old
`git.exe` was installed into `/mingw64/`.
A recently-upstreamed commit hard-codes this expectation even into the
CMake-based build, so that the built `git.exe` "knows where it lives"
and can ensure that the tools it expects on the `PATH` are found.
Naturally, this hard-coded MINGW64 needs to change to UCRT64 now, too.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Correct tests that depend on Perl running on MinGW * js/mingw-test-fixes-around-perl: t9129: skip UTF-8 tests on Windows t9700: accommodate for MSYS2 Perl reporting as `cygwin`
Windows build switches from MINGW64 to URCR64 runtime starting Git 2.56.0; switch the cmake based build at the same time. * js/win-cmake-use-ucrt64: cmake(windows): accommodate for Git for Windows' migration to UCRT64
The mechanism to register in-memory alternate object sources has been removed, as submodule object databases are now accessed natively via their own repository structures. This simplifies object database management and prepares the codebase for migrating alternate tracking into the files backend. * ps/odb-stop-registering-in-memory-sources: odb: remove the ability to link sources ad-hoc t/helper: stop registering alternates in "ref-store" command t/helper: adapt read-midx to not link ad-hoc source anymore builtin/multi-pack-index: refuse unknown sources with "--object-dir=" odb/packed: fix memory leaks when freeing source tmp-objdir: drop unused function to register alternate odb: remove infrastructure to register submodule sources builtin/grep: stop registering submodule ODB as source submodule-config: stop registering submodule sources submodule-config: stop using `the_hash_algo` submodule-config: remove uses of `the_repository` cache-tree: remove dependency on `the_repository` cache-tree: drop `the_repository` in `cache_tree_fully_valid()`
The consistency checks for the object database (fsck) have been decoupled from the generic builtin implementation and moved into the backend-specific object source layers, making them pluggable for different object storage formats. * ps/odb-pluggable-fsck: builtin/fsck: move loose object verification into the loose source builtin/fsck: move multi-pack index verification into the packed source builtin/fsck: move bitmap verification into the packed source builtin/fsck: move reverse index verification into the packed source builtin/fsck: move packfile verification into the packed source odb: provide infrastructure for pluggable fsck checks builtin/fsck: don't check alternates with "--no-full" builtin/fsck: de-globalize option handling builtin/fsck: merge `fsck_obj_buffer()` and `fsck_obj()` builtin/fsck: use `fsck_obj_buffer()` when checking loose objects
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )