[pull] master from git:master - #255
Merged
Merged
Conversation
When we search for the start of the basename and `len` is zero, `name` ends up being `path` - 1, out of bounds. Avoid that by checking before decrementing. Fixes git-for-windows#6346. Original-patch-by: Matthias Aßhauer <mha1993@live.de> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
worktree_basename() extracts an empty basename from a path consisting only of zero or more path separators. We can't use that as a worktree name. Properly report such a path as invalid instead of triggering a BUG that asks the user what just happened. Original-patch-by: Matthias Aßhauer <mha1993@live.de> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
worktree_basename() sets `n` to the length of `path` without trailing path separators, not to the length of the basename. This matters when deriving a branch name from a path with more than one component. E.g.: path: /new/worktree/ s: ^ n: |-----------| So here xstrndup(s, n) copies up to 13 characters from "worktree/", effectively to the end of the string, including the trailing dash. Path separators are not allowed at the end of branch names, so strip them off by calculating the basename length and extracting just that part. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
…ck-fallback * ps/odb-generic-corrupt-objects: odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically odb/source: allow `read_object_info()` to bubble up error messages odb/source: let callers discern missing and corrupt objects odb/source: introduce error status when reading objects odb/source-packed: flag known-bad objects as corrupt and not missing
worktree_basename() requires callers to do pointer arithmetic to get the actual basename. Simplify them by doing the calculations in the function and returning a copy of the basename directly. Remind programmers to free the result by renaming the function to worktree_basename_dup(). Among the three callers of the original function, two immediately make copies of the returned string before using and freeing it, which makes for an easy conversion. Convert the other one from resetting a shared strbuf to freeing the allocated string, which requires the same number of lines, but no arithmetic. The added allocation is negligible because it's small and there's only one per run of "git worktree add". Signed-off-by: René Scharfe <l.s.r@web.de> [jc: rephrased the second paragraph a bit.] Signed-off-by: Junio C Hamano <gitster@pobox.com>
…ic ref git-symbolic-ref(1) documents that reading a name that is not a symbolic ref fails, and that --quiet does so silently. Tests such as t2020 and t5621 already rely on "symbolic-ref -q HEAD" failing on a detached HEAD, but none checks that the plain form reports the error or that --quiet stays silent. Assert that a non-symbolic ref fails with the "is not a symbolic ref" message, and that --quiet fails with no output. Use test_must_fail rather than pinning the exact exit codes, which are documented but not worth freezing in the test. Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
git-check-ref-format(1) documents that a refname cannot contain a space, tilde, caret, colon, question-mark, asterisk, open-bracket or backslash, nor the sequence "..", and cannot be the single character "@". Of these, only "?", "\" and ".." were tested embedded in an otherwise-valid refname; "*" was checked only as a lone character or with --refspec-pattern. Test all of them in that embedded form with a single loop, and check that "@" alone is rejected even with --allow-onelevel -- where "@" is otherwise a valid refname component, as "refs/@" confirms. Signed-off-by: Nikolaus Schuetz <nikolauspschuetz@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When objects involved in the merge cannot be read, the merge machinery will return early with result.clean = -1, and result.tree left as NULL. pick_regular_commit() tested only "if (!result->clean)", ignoring the case where "clean < 0". That causes the code to try to use result->tree, resulting in a SIGSEGV. Handle clean < 0 explicitly; the merge machinery will already have printed messages such as "Could not read <object>" and "collecting merge info failed for trees...", so we don't need to add much detail beyond the fact that the merge failed. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In --batch mode "git mktree" reuses its entry buffer across trees, resetting `used` to 0 after writing each tree. It never frees the `treeent` structures the previous tree appended, though, so once the next tree overwrites those slots the earlier allocations are leaked. A single-tree invocation hides this, as the entries stay reachable through the `entries` global until exit. Free each entry when resetting the buffer, and free the buffer itself before returning. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
mktree_line() checks each referenced object's type with odb_read_object_info_extended() under OBJECT_INFO_QUICK. QUICK skips the reprepare-and-retry that reloads the on-disk pack set, so a resident "git mktree --batch" reader reports an object that a concurrent repack just relocated into a new pack as missing, and rejects the entry. QUICK entered this lookup in 817b0f6 (mktree: do not check type of remote objects, 2022-06-21) only to avoid lazily fetching promisor objects; OBJECT_INFO_SKIP_FETCH_OBJECT already provides that. Drop OBJECT_INFO_QUICK and keep OBJECT_INFO_SKIP_FETCH_OBJECT, so mktree still avoids a promisor fetch but recovers an object that was merely repacked. Add a regression test driving a resident mktree --batch reader across a concurrent repack that retires a pack. Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A geometric repack writes a new pack and multi-pack-index and then deletes the packs the new one subsumes. A process still using the previous MIDX keeps seeing a removed pack listed as the owner of some objects. Since a MIDX attributes each object to exactly one pack, such an object is served only through its recorded owner; if that owner was just removed, find_pack_entry() cannot serve it -- the MIDX lookup routes to the missing pack, and the regular pack fallback deliberately skips every MIDX-covered pack, so a surviving copy in another covered pack (e.g. a kept base pack) is never consulted. Unlike the ordinary "a pack's .idx is mapped but its .pack is gone" race, the second read does not rescue us. Reloading the on-disk pack set does not reload the borrowed, cached MIDX (freeing it under the code that caches the "struct multi_pack_index *" would be a use-after-free), so the stale MIDX keeps routing to the removed pack and the surviving copy stays hidden behind the covered-pack skip. cat-file, rev-list and pack-objects can thus all spuriously fail with "unable to read object". Teach find_pack_entry() to recover. The MIDX lookup now returns a tri-state, distinguishing an object absent from the MIDX from one it owns via a pack that can no longer be opened; in the latter case, once the regular fallback has also missed, scan the MIDX's packs directly for a surviving copy. Because the return value is no longer a boolean, rename fill_midx_entry() to midx_fill_entry() so callers must reckon with the new enum rather than silently treat MIDX_FILL_OWNER_UNAVAILABLE as a hit. Do the scan only on the second read (OBJECT_INFO_SECOND_READ): by then the cheaper on-disk reload has run, so an object merely relocated into a new (uncovered) pack has already been found by the regular fallback, and only a genuine hidden duplicate reaches the rescan. A QUICK caller that skips the second read simply accepts the false negative, as QUICK is designed to. Reloading the stale MIDX would be a more complete fix but is much more involved (the borrowers above need proper invalidation), so leave that for later. Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol Helped-by: Jeff King <peff@peff.net> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The function replay_revisions() in replay.c is rather lengthy. Extract the logic to put a commit entry into a `struct mapped_commits` into a helper function put_mapped_commit(). While at it, rename mapped_commit() to get_mapped_commit() to pair with this new function. Signed-off-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Depending on what gets passed into the function pick_regular_commit(), it decides the new base for the replayed commit. It first tries to find the replayed results of `pickme`'s parent in the `replayed_commits` map. If not found, it falls back to `onto`. When using git-replay(1) with --onto, the fallback is the revision passed in with this option, but when using --revert, the fallback is `last_commit`. It's rather confusing the base is decided partly inside pick_regular_commit() and partly by its caller. Move the base selection completely into the caller: replay_revisions(). This bundles all the logic of deciding on the base together. Also, this reduces the number of parameters of pick_regular_commit(), making its interface cleaner. This refactoring doesn't bring any behavior changes. Signed-off-by: Toon Claes <toon@iotcl.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
One of the stated goals of git-replay(1) is to allow implementing the
git-rebase(1) functionality on the server side.
The default mode of git-rebase(1) is to act as if `--no-rebase-merges`
was given. This mode drops merge commits instead of replaying them, and
linearizes the history into a sequence of regular (single-parent)
commits.
Add option `--linearize` to git-replay(1) to do the same. Each replayed
commit is stacked on top of the previously replayed one. When a merge is
encountered, the commits reachable from all of its sides are replayed
into the single line and the merge itself is dropped.
If a ref was pointing to a merge commit, that ref is updated to the
merge's last replayed ancestor.
git-replay(1) accepts multiple branches, for example:
$ git replay --onto main topic1 topic2
Without `--linearize` this replays 'topic1' and 'topic2' onto 'main'
(keeping shared portions of history shared and divergent parts
divergent) and updates both refs.
Due to current implementation limitations, replaying multiple branches
with `--linearize` is disallowed to avoid concatenating unrelated
histories into a single line. For the same reason disallow the use of
`--contained` with `--linearize`.
Users who want to linearize multiple branches are advised to do this in
separate git-replay(1) invocations. Linearizing multiple branches at
once might be added later.
Note that `--linearize` is not modeled after git-rebase(1)'s
`--rebase-merges[=<mode>]` interface. Recreating merges, by preserving
their topology, is a distinct operation that would be a separate mode.
`--linearize` only drops merges and replays commits linearly. So
git-replay(1) uses its own option rather than reusing that interface.
Based-on-patches-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Toon Claes <toon@iotcl.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The CI workflow groups all runs by commit hash using
`group: ${{ github.sha }}`. This means every push to a pull
request starts a separate workflow run, and all workflows
triggered by the same commit share the same concurrency group.
With this change, pull request runs are grouped by pull request
number instead of commit hash, and runs superseded by a newer
push are canceled. The concurrency group becomes
`${{ github.workflow }}-${{ github.event.pull_request.number ||
github.sha }}` and `cancel-in-progress` is set to true for
pull request events.
For pull request events, the group is `<workflow>-<pull-request-number>`
(e.g., "main-workflow-42"). If you push a new commit to an
existing pull request before the CI working on it finishes, the
new request will be placed in the same group and cancel the
currently running run.
For non-pull-request events, the group is `${{ github.workflow }}-${{
github.sha }}` and `cancel-in-progress` defaults to false, so
there is no regression in behavior.
Note that the previous configuration used `group: ${{ github.sha }}`,
which meant all workflows sharing the same commit hash were in the
same group. The new configuration includes the workflow name in
the group, so each workflow has its own concurrency group per
commit/PR.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
The patch fixes two typos in two places. versioncmp.c: "fractionnal" -> "fractional" t/t0022-crlf-rename.sh: "similiarity" -> "similarity" No functional changes, only update a comment and a test_description. Signed-off-by: Hardik Kumar <hardikxk@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
In setup_revisions() we rewrite the incoming argv array, losing references to the strings it contains. For a synthetic argv array constructed from heap strings, that traditionally meant we leaked those allocated strings. We fixed the leak in cd43948 (revision: manage memory ownership of argv in setup_revisions(), 2025-09-19). Now callers can tell the revision code that argv entries are allocated and should be freed, which it will do before overwriting them. But this introduced a new bug! The overwritten entries go away as soon as option parsing is finished, but a few options may actually create new references to those strings. And once we free the strings, those stale references become use-after-free bugs. For example, running: git stash show --src-prefix=foo/ demonstrates the problem: 1. The stash command generates its own synthetic argv (because it has to treat the stash specifiers specially) which it then passes to setup_revisions(). 2. Parsing will create a reference to the partial string "foo/" in revs.diffopt.a_prefix. 3. When setup_revisions() finishes, we rewrite argv to throw away parsed strings. This frees the entry holding "--src-prefix=foo", at which point we have a dangling reference in revs.diffopt. 4. We generate an actual diff, accessing garbage memory via revs.diffopt.a_prefix. The output is usually garbled, but ASan also detects this reliably. One obvious fix here is to allocate new strings when we pull data out of the argv array. But doing so is error prone (every string option must remember to do it or risk a subtle bug), and creates more questions about memory ownership (e.g., some callers assign string literals directly to a_prefix, and we would not want to free those). Instead we can fix this centrally by delaying the free() calls. We'll collect any "freed" strings in a new array, hold on to it for the life of the rev_info struct, and then release it at the end. We can easily use a strvec for this, since it handles growth and cleanup for us. This fixes the prefix case above (which is now tested in t3903), and should fix any other stray cases. Though I could not find any; we use OPT_STRING only in the prefix diff options, and very few revision opts store strings. Those that do (like --format and --encoding) already make a copy of the string. They do not need for us to hold on to the memory longer, but it does not hurt them if we do. One may note that combined with cd43948 we have approached a simpler solution in a roundabout way. We are still hacking up argv, but now carefully constructing a parallel argv of old strings we've overwritten (and will eventually free). In an alternate universe, we could instead leave the original argv pristine and return a new reduced-size argv. This is conceptually simpler, though it does mean that every caller must free that new argv array itself (not the entries). That's not something they traditionally had to do, so it would mean tweaking every caller. So even though the combination of this cd43948 and this patch is a little convoluted, it should make things just work (no leaks and no use-after-free) without modifying any callers. Reported-by: Nicolas Le Cam <niko.lecam@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
You do not want to mark an argv element for freeing unless the caller has given us the free_removed_argv_elements flag. Originally we just called free() in this case, so each caller checked the flag itself. Now that we mark them via a helper function, we can push the check down into the helper. This saves a little bit of duplicated code, but also hopefully makes the result conceptually simpler. Every caller but one was already checking this flag. The exception is setup_revisions_from_strvec(), but it always sets the flag explicitly (since its whole purpose is managing argv memory). So even though it was not checking the flag, doing so is OK (it will always be set). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
A few tests for the reference handling subsystem have been added to exercise the handling of forbidden characters and symbolic references. * ns/ref-symref-additional-tests: t1402: test forbidden characters in refnames t1401: check symbolic-ref failure and --quiet silence on a non-symbolic ref
GitHub Actions CI workflow runs triggered by pull requests have been configured to cancel older runs when a new push is made to the same pull request. * hn/ci-cancel-stale-pr-runs: ci: cancel stale pull request workflow runs
The string extraction logic for the branch name and worktree name from the given path in 'git worktree add' has been corrected and simplified to avoid out-of-bounds reads and improper handling of trailing slashes. * rs/worktree-add-basename-fixes: worktree add: let worktree_basename() return string copy worktree add: trim slashes when deriving branch name from path worktree add: reject separator-only path worktree add: don't read out of bounds in worktree_basename()
Various spelling mistakes in comments and test descriptions have been corrected. * hk/typofix: versioncmp: fix typo in versioncmp.c, t/t0022-crlf-rename.sh
The 'git replay' command has been taught the '--linearize' option to drop merge commits and linearize the replayed history, mimicking 'git rebase --no-rebase-merges'. * tc/replay-linearize: replay: offer an option to linearize the commit topology replay: resolve the replay base outside pick_regular_commit() replay: add helper to put entry into replayed_commits
The memory ownership of argv elements passed to the revision machinery has been made more robust by keeping logically "freed" elements alive until the rev_info struct is released, preventing use-after-free bugs when options store references to them. * jk/rev-info-argv-to-free: revision: simplify mark_argv_for_free() callers revision: hang on to "freed" argv elements
The object lookup machinery has been taught to gracefully recover when a multi-pack-index points to an owning pack that was removed during a concurrent geometric repack, and 'git replay' has been fixed to not segfault when reading such missing objects. * en/midx-missing-pack-fallback: packfile: recover when a multi-pack-index names a removed pack mktree: do not use OBJECT_INFO_QUICK when checking objects mktree: plug per-tree leak in --batch mode replay: fail gracefully when a merge input is unreadable
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.
8 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 : )