[pull] master from git:master #144
Merged
Conversation
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
Parsing of the alternates file and environment variable is currently
split up across multiple different functions and is entangled with
`link_alt_odb_entries()`, which is responsible for linking the parsed
object database sources. This results in two downsides:
- We have mutual recursion between parsing alternates and linking them
into the object database. This is because we also parse alternates
that the newly added sources may have.
- We mix up the actual logic to parse the data and to link them into
place.
Refactor the logic so that parsing of the alternates file is entirely
self-contained. Note that this doesn't yet fix the above two issues, but
it is a necessary step to get there.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Parsing alternates and resolving potential relative paths is currently handled in two separate steps. This has the effect that the logic to retrieve alternates is not entirely self-contained. We want it to be just that though so that we can eventually move the logic to list alternates into the `struct odb_source`. Move the logic to resolve relative alternative paths into `parse_alternates()`. Besides bringing us a step closer towards the above goal, it also neatly separates concerns of generating the list of alternatives and linking them into the object database. Note that we ignore any errors when the relative path cannot be resolved. This isn't really a change in behaviour though: if the path cannot be resolved to a directory then `alt_odb_usable()` still knows to bail out. While at it, rename the function to `odb_add_alternate_recursively()` to more clearly indicate what its intent is and to align it with modern terminology. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function `alt_odb_usable()` receives as input the object database, the path it's supposed to determine usability for as well as the normalized path of the main object directory of the repository. The last part is derived by the function's caller from the object database. As we already pass the object database to `alt_odb_usable()` it is redundant information. Drop the extra parameter and compute the normalized object directory in the function itself. While at it, rename the function to `odb_is_source_usable()` to align it with modern terminology. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When calling `odb_add_to_alternates_file()` we know to add the newly added source to the object database in case we have already loaded alternates. This is done so that we can make its objects accessible immediately without having to fully reload all alternates. The way we do this though is to call `link_alt_odb_entries()`, which adds _multiple_ sources to the object database source in case we have newline-separated entries. This behaviour is not documented in the function documentation of `odb_add_to_alternates_file()`, and all callers only ever pass a single directory to it. It's thus entirely surprising and a conceptual mismatch. Fix this issue by directly calling `odb_add_alternate_recursively()` instead. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When adding an alternative object database source we not only have to
consider the added source itself, but we also have to add _its_ sources
to our database. We implement this via mutual recursion:
1. We first call `link_alt_odb_entries()`.
2. `link_alt_odb_entries()` calls `parse_alternates()`.
3. We then add each alternate via `odb_add_alternate_recursively()`.
4. `odb_add_alternate_recursively()` calls `link_alt_odb_entries()`
again.
This flow is somewhat hard to follow, but more importantly it means that
parsing of alternates is somewhat tied to the recursive behaviour.
Refactor the function to remove the mutual recursion between adding
sources and parsing alternates. The parsing step thus becomes completely
oblivious to the fact that there is recursive behaviour going on at all.
The recursion is handled by `odb_add_alternate_recursively()` instead,
which now recurses with itself.
This refactoring allows us to move parsing of alternates into object
database sources in a subsequent step.
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Now that we have removed the mutual recursion in the preceding commit it is not necessary anymore to have a forward declaration of the `read_info_alternates()` function. Move the function and its dependencies further up so that we can remove it. Note that this commit also removes the function documentation of `read_info_alternates()`. It's unclear what it's documenting, but it for sure isn't documenting the modern behaviour of the function anymore. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Adapt how we read alternates so that the interface is structured around the object database source we're reading from. This will eventually allow us to abstract away this behaviour with pluggable object databases so that every format can have its own mechanism for listing alternates. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Refactor writing of alternates so that the actual business logic is structured around the object database source we want to write the alternate to. Same as with the preceding commit, this will eventually allow us to have different logic for writing alternates depending on the backend used. Note that after the refactoring we start to call `odb_add_alternate_recursively()` unconditionally. This is fine though as we know to skip adding sources that are tracked already. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Since Aug 2006, the DarwinPorts project renamed themselves as MacPorts. Those who are not intimately familiar with the Opensource ecosystem around macOS from olden days, the name DarwinPorts may not ring a bell, even when they are using MacPorts. Signed-off-by: Junio C Hamano <gitster@pobox.com> Reviewed-by: Carlo Marcelo Arenas Belón <carenas@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
…-fix * sa/replay-atomic-ref-updates: replay: add replay.refAction config option replay: make atomic ref updates the default behavior replay: use die_for_incompatible_opt2() for option validation
cmd_replay() aborts if the pointer "onto" is NULL after argument parsing, e.g. when specifying a non-existing commit with --onto. 15cd4ef (replay: make atomic ref updates the default behavior, 2025-11-06) added code that dereferences this pointer before the check. Switch their places to avoid a segmentation fault. Reported-by: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Unless there are good reasons, it is customary to have the options[] array used with the parse-options API declared in function scope rather than at file scope. Move builtin/pull.c:cmd_pull()’s options[] array into the function to match that convention. Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git --version reports its version with the prefix "git version ". Remove precisely this string instead of everything up to and including the rightmost space to avoid butchering version strings that contain spaces. This helps Apple's release of Git, which reports its version like this: "git version 2.50.1 (Apple Git-155)". Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Some commands will produce output on stderr if there are conflicts, but git-replay(1) is completely silent. Explicitly spell that out. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
There is no documentation for `--contained`. Start by copying the text from `replay_options` in `builtin/ replay.c`. But some people think that the existing text is a bit unclear; what does it mean for a branch to be contained in a revision range? Let’s include the implied commits here: the branches that point at commits in the range. Also use “update” instead of “advance”. “Update” is the verb commonly used in this context. Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Code refactoring around alternate object store. * ps/odb-alternates-object-sources: odb: write alternates via sources odb: read alternates via sources odb: drop forward declaration of `read_info_alternates()` odb: remove mutual recursion when parsing alternates odb: stop splitting alternate in `odb_add_to_alternates_file()` odb: move computation of normalized objdir into `alt_odb_usable()` odb: resolve relative alternative paths when parsing odb: refactor parsing of alternates to be self-contained
"git replay" documentation updates. * kh/doc-replay-updates: doc: replay: link section using markup replay: improve --contained and add to doc doc: replay: mention no output on conflicts
"git replay --onto=<commit> ...", when <commit> is mistyped, started to segfault with recent change, which has been corrected. * rs/replay-wrong-onto-fix: replay: move onto NULL check before first use
Makefile in-comment doc update. * jc/macports-darwinports: Makefile: help macOS novices by mentioning MacPorts
Code clean-up. * kj/pull-options-decl-cleanup: pull: move options[] array into function scope
Test fix. * rs/t4014-git-version-string-fix: t4014: support Git version strings with spaces
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 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 : )