[pull] master from git:master - #192
Merged
Merged
Conversation
Commit 430b75f (commit: give correct advice for empty commit during a rebase, 2019-12-06) introduced a FROM_REBASE_PICK enum value and an is_from_rebase() function. Those names failed to convey that they were specifically about hitting a commit that becomes empty when rebasing. Clarify their names now. While at it, change `whence == FROM_REBASE_NOW_EMPTY` to use `is_from_rebase_now_empty(whence)`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
For years, we disallowed partial commits during merges or cherry-picks. In commit 430b75f (commit: give correct advice for empty commit during a rebase, 2019-12-06) it was noted that the "cannot do a partial commit during a cherry-pick" message was also printed when rebasing a commit that became empty, and rather than drop the check in that case, that commit opted to make the message print the actual operation that was in progress. Since a commit that has become empty comes without conflicts, a new partial commit poses no problems; remove the error in that case. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a rebase applies a commit that becomes empty, it stops and asks the
user to decide whether to keep it or drop it. HEAD still points at the
previously-applied commit at that point, so amending is refused, with:
You are in the middle of a rebase -- cannot amend.
That message would suggest that amending is not allowed during an 'edit'
or 'break' stop, which is misleading, plus it lacks the specificity that
might help the user know why their particular case is a problem: the
commit they intended to amend became empty and was dropped, so amending
would affect the wrong commit. Reword the error accordingly.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Running `git commit --amend` during conflict resolution is an ugly
foot-gun. For many years, we have rejected amending during conflict
resolution in the middle of
- a merge
- a cherry-pick
However, this was never extended to other operations that can also
produce conflicts:
- an `am` operation
- a revert
- a rebase
Extend it to handle these other cases now.
Extending to `am`, revert, and the apply backend of rebase are fairly
straightforward. However, with the merge backend of rebase we have to
be more careful, since it powers interactive rebases and
- the interactive machinery internally uses `git commit --amend` for
`squash` and `reword` directives
- users are expected to `git commit --amend` after hitting an `edit`
or `break` directive
So, we need to be careful with rebase to only reject amending when doing
conflict resolution.
A few files under the rebase-merge/ directory provide us the necessary
information:
- stopped-sha is written only when the rebase stops and hands control
back to the user, so its presence marks a genuine stop -- as opposed
to the sequencer's own internal `git commit --amend` while applying
a squash, fixup, or reword, during which no stopped-sha exists.
- amend is written only when the rebase stops with HEAD already
pointing at the commit the user is meant to amend: a clean `edit`,
or a fast-forward `reword`. Its absence at a stop therefore means
the commit did not apply, so HEAD is the previously-applied commit
rather than the one being rebased -- exactly the case we refuse.
So for the merge backend we die when stopped-sha exists and amend does
not. This covers a plain conflicted pick as well as a conflicted `edit`
(both leave HEAD on the previously-applied commit), while still allowing
a clean `edit` or `reword` stop and a `break` stop (no stopped-sha).
stopped-sha is unlinked at the start of the resume loop, so a resumed
squash's internal amend is unaffected.
Signed-off-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Similar to the previous commit, just as `git commit --amend` is a foot-gun during conflict resolution, so is a partial commit (`git commit <paths>`). Recording a conflict resolution is about capturing the state of the entire tree on top of HEAD, not a subset of paths. For many years we have rejected partial commits in the middle of - a merge - a cherry-pick but, just like amending, this was never extended to the other operations that can also leave conflicts to resolve: - an `am` operation - a revert - a rebase that stopped for conflict resolution Reuse sequencer_ongoing_operation(), introduced for the analogous `--amend` check, to detect these and refuse the partial commit. A rebase that stopped because a pick became empty is not conflict resolution and, as an earlier patch established, is deliberately left permitted. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Calling repo_clear() twice in a row will segfault because the second call will invoke parse_object_pool_clear() on a NULL pointer. This is not usually a big deal, but we can make some error cleanup a little simpler if callers do not need to worry about invoking it twice. We can fix it by catching the NULL case. The rest of repo_clear() appears to be idempotent. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
If repo setup fails, we'll return an error without freeing the allocated url string, leaking the memory. The test suite does trigger this error, but never with the leak. We only allocate a url if submodule_from_path() returned something, but our tests use other situations, like totally nonexistent submodules. We can cover this case by asking about a submodule that exists but which has not been initialized. The new test fails with SANITIZE=leak. The smallest fix would just be a call to free(url), but I think it's a little nicer to set up a dedicated out-path for cleanup here. The previous commit made it safe to call repo_clear() even if repo_submodule_init() fails. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
write_ondisk_index() dereferences the return value of repo_parse_tree_indirect() unconditionally. If the parent commit's tree object is missing from the object store (corrupt repository, object removed by tooling, or incomplete restore), the function returns NULL and "git history split" crashes with a SIGSEGV. Guard the parse result and error out gracefully, following the codebase convention for objects that cannot be loaded. Signed-off-by: Jinbao Chen <zkd18cjb@mail.ustc.edu.cn> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
No existing code modifies these two strings in pathspec elements after they are created via these two pointers. Declare them as "const char *" to stress on this fact and cast away constness from the code that frees these two strings. Signed-off-by: Junio C Hamano <gitster@pobox.com>
dee8094 (doc: add an explanation of Git's data model, 2025-11-12) added gitdatamodel(7), documenting Git’s data model. But it is not mentioned anywhere. Let’s start by listing it under Guides in git(1) and with `git help --guides`. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The previous commit added the first mention of gitdatamodel(7) here, under Guides. But there are also other sections where a mention is relevant. Let’s mention it: • under Description, since it is as useful as the other tutorials already mentioned there for those who are interested; • under Terminology, since it complements gitglossary(7) as a pedagogical rather than reference source for the core terms;[1] and • under See Also, since the other tutorials (plus the user manual) are mentioned there. We don’t need to mention it under Further Documentation since we now mention it under Description. † 1: See dee8094 (doc: add an explanation of Git's data model, 2025-11-12): `gitglossary`. This makes a good effort, but it's an alphabetically ordered dictionary and a dictionary is not a good way to learn concepts. You have to jump around too much and it's not possible to present the concepts in the order that they should be explained. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Four of the terms in the glossary are discussed in gitdatamodel(7). Let’s link to the data model page from the glossary. The phrasing needs to be tweaked based on what gitdatamodel(7) offers for each term compared to the glossary, or even other pages (see the git-reflog(1) mention). For instance, the ref/reference discussion can be called a “see also” since the glossary here already goes into detail. On the other hand, gitdatamodel(7) offers more details on the subject of “the index”. Let’s also add gitdatamodel(7) to See Also. It is at least as relevant as the other tutorial pages that are already mentioned. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
We linked from the glossary to the data model page in the last commit. It can also be useful to link the other way for readers who might want to reference more terminology. Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Whether CHERRY_PICK_HEAD is written depends on the command, on whether the merge started, and on --no-commit, all in one condition in do_pick_commit(). The suite checks the clean --no-commit pick; nothing checks the conflicting one. The test that already runs a conflicting --no-commit pick compares the advice the command prints, which is what tells us it stopped on a conflict. Assert the ref is missing there too. Signed-off-by: Aleksei Sviridkin <f@lex.la> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The list of what happens when a change is hard to apply states without qualification that CHERRY_PICK_HEAD is set. Under --no-commit it is not: d7e5c0c (Introduce CHERRY_PICK_HEAD, 2011-02-19) skips the ref on purpose there, presuming the user intends to further edit the result and possibly pick more commits on top. The option's own description says nothing about the ref or about authorship. "git commit" reads the author of a cherry-pick from CHERRY_PICK_HEAD, so without it a plain commit records you as the author. Say so where the option is described, and say that this is the point of the option rather than a wrinkle: what is being built is the user's own work, not a reproduction of the original commit. Signed-off-by: Aleksei Sviridkin <f@lex.la> Signed-off-by: Junio C Hamano <gitster@pobox.com>
When a protocol v2 fetch includes packfile URIs, the client downloads each advertised pack in a separate http-fetch process. Existing Trace2 regions cover negotiation, but not the time spent downloading these packs or the number of advertised URIs. Add a Trace2 region around the packfile URI download loop and record the number of URIs. This makes the cost of downloading external packs visible without emitting an event for each pack. Extend the existing packfile URI test to verify the region and count. Signed-off-by: Ted Nyman <tnyman@openai.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The regex quantifier {,n} was added in Perl 5.34.0 as a shorthand
for {0,n}. That makes it too new an introduction for Git which
targets Perl 5.26.0.
Even though Documentation/lint-gitlink.perl is a development helper
script, let's stick to the general Perl version requirement for
consistency, and use an explicit zero in .{0,8}.
Signed-off-by: Tuomas Ahola <taahol@utu.fi>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Fix conjoined bullet items for the maintenance strategies in git-config(1). Reportedly, asciidoctor renders this document correctly both before and after this patch. asciidoc renders it correctly only _after_ this patch. Signed-off-by: Brigham Campbell <me@brighamcampbell.com> Acked-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Our CI Documentation builds have pinned asciidoctor since 615a6c3 (ci: stick with Asciidoctor v1.5.8 for now, 2019-03-29). Back then a few changes were needed to build with the then-new v2.0.0. We've since made those changes, in f6461b8 (Documentation: fix build with Asciidoctor 2, 2019-09-15) and other commits. So it is not only safe to use newer versions of asciidoctor, but preferable: it's what people are likely to use for actually building the documentation in practice! I don't think there's any need to pin to a specific version. We can just use what ships in our Ubuntu image, which should give us an arbitrary representative version (and if it changes and something breaks, we'd want to know). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
The CI script to install dependencies for the documentation build has been updated to install asciidoctor directly via the system package manager instead of pinning to an older version via gem. Additionally, an obsolete variable used for retired Azure Pipelines environments has been removed. * jk/ci-use-system-asciidoctor: ci: use system asciidoctor
Mark-up fix for 'git maintenance' documentation pages. * bc/maintenance-doc-markup-fix-for-asciidoc: doc: fix conjoined maintenance strategies in git-config(1)
Teach 'am', 'revert', and 'rebase' that running 'commit --amend' or a partial 'commit <paths>' makes no sense during operations that stop and return control to the user to resolve conflicts left in the working tree, just like 'cherry-pick' and 'merge' do. * en/no-amend-during-conflicts: commit: refuse partial commits during conflict resolution commit: refuse to amend during conflict resolution commit: reword the empty-commit rebase amend error commit: allow a partial commit when a rebase pick becomes empty commit: clarify FROM_REBASE_PICK and is_from_rebase() names
The error path in 'git submodule--helper' has been updated to plug a memory leak when a repository handle could not be obtained, leveraging an updated idempotent repo_clear(). * jk/submodule-error-leak: submodule--helper: free URL when repository setup fails repository: make repo_clear() idempotent
Two members in "struct pathspec_item" were of type "char *", but nobody updated the string through these pointers. They have been made "const char *" instead. * jc/pathspec-match-const: pathspec: match and original in pathspec_item are const
The process of downloading packfile URIs in protocol v2 has been instrumented with a Trace2 region. This visibility allows tracking the cumulative time spent downloading external packs and the number of advertised URIs without emitting a separate event per pack. * tn/fetch-pack-trace-packfile-uri: fetch-pack: trace packfile URI downloads
The gitdatamodel documentation page has been linked from a handful of key documentaiton pages. * kh/doc-datamodel: doc: datamodel: link to the glossary doc: glossary: link four of the terms to gitdatamodel(7) doc: git: link to the gitdatamodel(7) tutorial doc: git: list gitdatamodel(7) as a concept guide
The documentation for 'git cherry-pick' has been updated to clarify that the '--no-commit' option intentionally skips setting the 'CHERRY_PICK_HEAD' ref. A test has also been added to ensure this behavior holds even when the operation stops for conflicts. * as/cherry-pick-no-commit-doc: doc: cherry-pick: note --no-commit skips CHERRY_PICK_HEAD t3507: check no CHERRY_PICK_HEAD after conflicting --no-commit
Running "git history" in a corrupt repository can (unsurprisingly) segfault when a necessary tree object is not found. * jc/history-missing-tree-errorfix: history: do not dereference NULL when parent tree is missing
The development helper script to lint gitlink references in the
documentation has been updated to avoid a newer Perl regular
expression syntax that breaks on older Perl versions.
* ta/lint-gitlink-older-perl-fix:
lint-gitlink: don't use empty lower bound in .{0,8}
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.
9 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 : )