path: root/commit.c
AgeCommit message (Collapse)Author
2021-03-26Merge branch 'cm/rebase-i-fixup-amend-reword'Junio C Hamano
"git commit --fixup=<commit>", which was to tweak the changes made to the contents while keeping the original log message intact, learned "--fixup=(amend|reword):<commit>", that can be used to tweak both the message and the contents, and only the message, respectively. * cm/rebase-i-fixup-amend-reword: doc/git-commit: add documentation for fixup=[amend|reword] options t3437: use --fixup with options to create amend! commit t7500: add tests for --fixup=[amend|reword] options commit: add a reword suboption to --fixup commit: add amend suboption to --fixup to create amend! commit sequencer: export and rename subject_length()
2021-03-15sequencer: export and rename subject_length()Charvi Mendiratta
This function can be used in other parts of git. Let's move the function to commit.c and also rename it to make the name of the function more generic. Mentored-by: Christian Couder <> Mentored-by: Phillip Wood <> Helped-by: Eric Sunshine <> Signed-off-by: Charvi Mendiratta <> Signed-off-by: Junio C Hamano <>
2021-03-14use CALLOC_ARRAYRené Scharfe
Add and apply a semantic patch for converting code that open-codes CALLOC_ARRAY to use it instead. It shortens the code and infers the element size automatically. Signed-off-by: René Scharfe <> Signed-off-by: Junio C Hamano <>
2021-02-23Merge branch 'bc/signed-objects-with-both-hashes'Junio C Hamano
Signed commits and tags now allow verification of objects, whose two object names (one in SHA-1, the other in SHA-256) are both signed. * bc/signed-objects-with-both-hashes: gpg-interface: remove other signature headers before verifying ref-filter: hoist signature parsing commit: allow parsing arbitrary buffers with headers gpg-interface: improve interface for parsing tags commit: ignore additional signatures when parsing signed commits ref-filter: switch some uses of unsigned long to size_t
2021-02-18Merge branch 'ak/corrected-commit-date'Junio C Hamano
The commit-graph learned to use corrected commit dates instead of the generation number to help topological revision traversal. * ak/corrected-commit-date: doc: add corrected commit date info commit-reach: use corrected commit dates in paint_down_to_common() commit-graph: use generation v2 only if entire chain does commit-graph: implement generation data chunk commit-graph: implement corrected commit date commit-graph: return 64-bit generation number commit-graph: add a slab to store topological levels t6600-test-reach: generalize *_three_modes commit-graph: consolidate fill_commit_graph_info revision: parse parent in indegree_walk_step() commit-graph: fix regression when computing Bloom filters
2021-02-11commit: allow parsing arbitrary buffers with headersbrian m. carlson
Currently only commits are signed with headers. However, in the future, we'll also sign tags with headers as well. Let's refactor out a function called parse_buffer_signed_by_header which does exactly that. In addition, since we'll want to sign things other than commits this way, let's call the function sign_with_header instead of do_sign_commit. Signed-off-by: brian m. carlson <> Signed-off-by: Junio C Hamano <>
2021-02-11gpg-interface: improve interface for parsing tagsbrian m. carlson
We have a function which parses a buffer with a signature at the end, parse_signature, and this function is used for signed tags. However, we'll need to store values for multiple algorithms, and we'll do this by using a header for the non-default algorithm. Adjust the parse_signature interface to store the parsed data in two strbufs and turn the existing function into parse_signed_buffer. The latter is still used in places where we know we always have a signed buffer, such as push certs. Adjust all the callers to deal with this new interface. Signed-off-by: brian m. carlson <> Signed-off-by: Junio C Hamano <>
2021-01-28oid_pos(): access table through const pointersJeff King
When we are looking up an oid in an array, we obviously don't need to write to the array. Let's mark it as const in the function interfaces, as well as in the local variables we use to derference the void pointer (note a few cases use pointers-to-pointers, so we mark everything const). Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2021-01-28hash_pos(): convert to oid_pos()Jeff King
All of our callers are actually looking up an object_id, not a bare hash. Likewise, the arrays they are looking in are actual arrays of object_id (not just raw bytes of hashes, as we might find in a pack .idx; those are handled by bsearch_hash()). Using an object_id gives us more type safety, and makes the callers slightly shorter. It also gets rid of the word "sha1" from several access functions, though we could obviously also rename those with s/sha1/hash/. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2021-01-28commit_graft_pos(): take an oid instead of a bare hashJeff King
All of our callers have an object_id, and are just dereferencing the hash field to pass to us. Let's take the actual object_id instead. We still access the hash to pass to hash_pos, but it's a step in the right direction. This makes the callers slightly simpler, but also gets rid of the untyped pointer, as well as the now-inaccurate name "sha1". Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2021-01-19commit: ignore additional signatures when parsing signed commitsbrian m. carlson
When we create a commit with multiple signatures, neither of these signatures includes the other. Consequently, when we produce the payload which has been signed so we can verify the commit, we must strip off any other signatures, or the payload will differ from what was signed. Do so, and in preparation for verifying with multiple algorithms, pass the algorithm we want to verify into parse_signed_commit. Signed-off-by: brian m. carlson <> Signed-off-by: Junio C Hamano <>
2021-01-19commit-graph: return 64-bit generation numberAbhishek Kumar
In a preparatory step for introducing corrected commit dates, let's return timestamp_t values from commit_graph_generation(), use timestamp_t for local variables and define GENERATION_NUMBER_INFINITY as (2 ^ 63 - 1) instead. We rename GENERATION_NUMBER_MAX to GENERATION_NUMBER_V1_MAX to represent the largest topological level we can store in the commit data chunk. With corrected commit dates implemented, we will have two such *_MAX variables to denote the largest offset and largest topological level that can be stored. Signed-off-by: Abhishek Kumar <> Reviewed-by: Taylor Blau <> Reviewed-by: Derrick Stolee <> Signed-off-by: Junio C Hamano <>
2021-01-15Merge branch 'ma/sha1-is-a-hash'Junio C Hamano
Retire more names with "sha1" in it. * ma/sha1-is-a-hash: hash-lookup: rename from sha1-lookup sha1-lookup: rename `sha1_pos()` as `hash_pos()` object-file.c: rename from sha1-file.c object-name.c: rename from sha1-name.c
2021-01-07Merge branch 'en/merge-ort-recursive'Junio C Hamano
The ORT merge strategy learned to synthesize virtual ancestor tree by recursively merging multiple merge bases together, just like the recursive backend has done for years. * en/merge-ort-recursive: merge-ort: implement merge_incore_recursive() merge-ort: make clear_internal_opts() aware of partial clearing merge-ort: copy a few small helper functions from merge-recursive.c commit: move reverse_commit_list() from merge-recursive
2021-01-04hash-lookup: rename from sha1-lookupMartin Ågren
Change all remnants of "sha1" in hash-lookup.c and .h and rename them to reflect that we're not just able to handle SHA-1 these days. Signed-off-by: Martin Ågren <> Reviewed-by: Derrick Stolee <> Signed-off-by: Junio C Hamano <>
2021-01-04sha1-lookup: rename `sha1_pos()` as `hash_pos()`Martin Ågren
Rename this function to reflect that we're not just able to handle SHA-1 these days. There are a few instances of "sha1" left in sha1-lookup.[ch] after this, but those will be addressed in the next commit. Signed-off-by: Martin Ågren <> Reviewed-by: Derrick Stolee <> Signed-off-by: Junio C Hamano <>
2020-12-17commit: move reverse_commit_list() from merge-recursiveElijah Newren
Signed-off-by: Elijah Newren <> Signed-off-by: Junio C Hamano <>
2020-12-08commit: implement commit_list_contains()Derrick Stolee
It can be helpful to check if a commit_list contains a commit. Use pointer equality, assuming lookup_commit() was used. Signed-off-by: Derrick Stolee <> Signed-off-by: Taylor Blau <> Signed-off-by: Junio C Hamano <>
2020-10-20Documentation: stylistically normalize references to Signed-off-by:Bradley M. Kuhn
Ted reported an old typo in the git-commit.txt and merge-options.txt. Namely, the phrase "Signed-off-by line" was used without either a definite nor indefinite article. Upon examination, it seems that the documentation (including items in Documentation/, but also option help strings) have been quite inconsistent on usage when referring to `Signed-off-by`. First, very few places used a definite or indefinite article with the phrase "Signed-off-by line", but that was the initial typo that led to this investigation. So, normalize using either an indefinite or definite article consistently. The original phrasing, in Commit 3f971fc425b (Documentation updates, 2005-08-14), is "Add Signed-off-by line". Commit 6f855371a53 (Add --signoff, --check, and long option-names. 2005-12-09) switched to using "Add `Signed-off-by:` line", but didn't normalize the former commit to match. Later commits seem to have cut and pasted from one or the other, which is likely how the usage became so inconsistent. Junio stated on the git mailing list in <> a preference to leave off the colon. Thus, prefer `Signed-off-by` (with backticks) for the documentation files and Signed-off-by (without backticks) for option help strings. Additionally, Junio argued that "trailer" is now the standard term to refer to `Signed-off-by`, saying that "becomes plenty clear that we are not talking about any random line in the log message". As such, prefer "trailer" over "line" anywhere the former word fits. However, leave alone those few places in documentation that use Signed-off-by to refer to the process (rather than the specific trailer), or in places where mail headers are generally discussed in comparison with Signed-off-by. Reported-by: "Theodore Y. Ts'o" <> Signed-off-by: Bradley M. Kuhn <> Acked-by: Taylor Blau <> Signed-off-by: Junio C Hamano <>
2020-09-09Merge branch 'jt/interpret-branch-name-fallback'Junio C Hamano
"git status" has trouble showing where it came from by interpreting reflog entries that recordcertain events, e.g. "checkout @{u}", and gives a hard/fatal error. Even though it inherently is impossible to give a correct answer because the reflog entries lose some information (e.g. "@{u}" does not record what branch the user was on hence which branch 'the upstream' needs to be computed, and even if the record were available, the relationship between branches may have changed), at least hide the error to allow "status" show its output. * jt/interpret-branch-name-fallback: wt-status: tolerate dangling marks refs: move dwim_ref() to header file sha1-name: replace unsigned int with option struct
2020-09-03Merge branch 'pw/rebase-i-more-options'Junio C Hamano
"git rebase -i" learns a bit more options. * pw/rebase-i-more-options: t3436: do not run git-merge-recursive in dashed form rebase: add --reset-author-date rebase -i: support --ignore-date rebase -i: support --committer-date-is-author-date am: stop exporting GIT_COMMITTER_DATE rebase -i: add --ignore-whitespace flag
2020-09-02wt-status: tolerate dangling marksJonathan Tan
When a user checks out the upstream branch of HEAD, the upstream branch not being a local branch, and then runs "git status", like this: git clone $URL client cd client git checkout @{u} git status no status is printed, but instead an error message: fatal: HEAD does not point to a branch (This error message when running "git branch" persists even after checking out other things - it only stops after checking out a branch.) This is because "git status" reads the reflog when determining the "HEAD detached" message, and thus attempts to DWIM "@{u}", but that doesn't work because HEAD no longer points to a branch. Therefore, when calculating the status of a worktree, tolerate dangling marks. This is done by adding an additional parameter to dwim_ref() and repo_dwim_ref(). Signed-off-by: Jonathan Tan <> Signed-off-by: Junio C Hamano <>
2020-08-17am: stop exporting GIT_COMMITTER_DATEPhillip Wood
The implementation of --committer-date-is-author-date exports GIT_COMMITTER_DATE to override the default committer date but does not reset GIT_COMMITTER_DATE in the environment after creating the commit so it is set in the environment of any hooks that get run. We're about to add the same functionality to the sequencer and do not want to have GIT_COMMITTER_DATE set when running hooks or exec commands so lets update commit_tree_extended() to take an explicit committer so we override the default date without setting GIT_COMMITTER_DATE in the environment. Signed-off-by: Phillip Wood <> Signed-off-by: Junio C Hamano <>
2020-07-31strvec: rename struct fieldsJeff King
The "argc" and "argv" names made sense when the struct was argv_array, but now they're just confusing. Let's rename them to "nr" (which we use for counts elsewhere) and "v" (which is rather terse, but reads well when combined with typical variable names like "args.v"). Note that we have to update all of the callers immediately. Playing tricks with the preprocessor is hard here, because we wouldn't want to rewrite unrelated tokens. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2020-07-28strvec: convert more callers away from argv_array nameJeff King
We eventually want to drop the argv_array name and just use strvec consistently. There's no particular reason we have to do it all at once, or care about interactions between converted and unconverted bits. Because of our preprocessor compat layer, the names are interchangeable to the compiler (so even a definition and declaration using different names is OK). This patch converts remaining files from the first half of the alphabet, to keep the diff to a manageable size. The conversion was done purely mechanically with: git ls-files '*.c' '*.h' | xargs perl -i -pe ' s/ARGV_ARRAY/STRVEC/g; s/argv_array/strvec/g; ' and then selectively staging files with "git add '[abcdefghjkl]*'". We'll deal with any indentation/style fallouts separately. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2020-07-09Merge branch 'tb/fix-persistent-shallow' into masterJunio C Hamano
When "fetch.writeCommitGraph" configuration is set in a shallow repository and a fetch moves the shallow boundary, we wrote out broken commit-graph files that do not match the reality, which has been corrected. * tb/fix-persistent-shallow: commit.c: don't persist substituted parents when unshallowing
2020-07-08commit.c: don't persist substituted parents when unshallowingTaylor Blau
Since 37b9dcabfc (shallow.c: use '{commit,rollback}_shallow_file', 2020-04-22), Git knows how to reset stat-validity checks for the $GIT_DIR/shallow file, allowing it to change between a shallow and non-shallow state in the same process (e.g., in the case of 'git fetch --unshallow'). However, when $GIT_DIR/shallow changes, Git does not alter or remove any grafts (nor substituted parents) in memory. This comes up in a "git fetch --unshallow" with fetch.writeCommitGraph set to true. Ordinarily in a shallow repository (and before 37b9dcabfc, even in this case), commit_graph_compatible() would return false, indicating that the repository should not be used to write a commit-graphs (since commit-graph files cannot represent a shallow history). But since 37b9dcabfc, in an --unshallow operation that check succeeds. Thus even though the repository isn't shallow any longer (that is, we have all of the objects), the in-core representation of those objects still has munged parents at the shallow boundaries. When the commit-graph write proceeds, we use the incorrect parentage, producing wrong results. There are two ways for a user to work around this: either (1) set 'fetch.writeCommitGraph' to 'false', or (2) drop the commit-graph after unshallowing. One way to fix this would be to reset the parsed object pool entirely (flushing the cache and thus preventing subsequent reads from modifying their parents) after unshallowing. That would produce a problem when callers have a now-stale reference to the old pool, and so this patch implements a different approach. Instead, attach a new bit to the pool, 'substituted_parent', which indicates if the repository *ever* stored a commit which had its parents modified (i.e., the shallow boundary prior to unshallowing). This bit needs to be sticky because all reads subsequent to modifying a commit's parents are unreliable when unshallowing. Modify the check in 'commit_graph_compatible' to take this bit into account, and correctly avoid generating commit-graphs in this case, thus solving the bug. Helped-by: Derrick Stolee <> Helped-by: Jonathan Nieder <> Reported-by: Jay Conrod <> Reviewed-by: Jonathan Nieder <> Signed-off-by: Taylor Blau <> Signed-off-by: Junio C Hamano <>
2020-06-17commit-graph: minimize commit_graph_data_slab accessAbhishek Kumar
In an earlier patch, multiple struct acccesses to `graph_pos` and `generation` were auto-converted to multiple method calls. Since the values are fixed and commit-slab access costly, we would be better off with storing the values as a local variable and reusing it. Signed-off-by: Abhishek Kumar <> Signed-off-by: Junio C Hamano <>
2020-06-17commit: move members graph_pos, generation to a slabAbhishek Kumar
We remove members `graph_pos` and `generation` from the struct commit. The default assignments in init_commit_node() are no longer valid, which is fine as the slab helpers return appropriate default values and the assignments are removed. We will replace existing use of commit->generation and commit->graph_pos by commit_graph_data_slab helpers using `contrib/coccinelle/commit.cocci'. Signed-off-by: Abhishek Kumar <> Signed-off-by: Junio C Hamano <>
2020-06-17object: drop parsed_object_pool->commit_countAbhishek Kumar
14ba97f8 (alloc: allow arbitrary repositories for alloc functions, 2018-05-15) introduced parsed_object_pool->commit_count to keep count of commits per repository and was used to assign commit->index. However, commit-slab code requires commit->index values to be unique and a global count would be correct, rather than a per-repo count. Let's introduce a static counter variable, `parsed_commits_count` to keep track of parsed commits so far. As commit_count has no use anymore, let's also drop it from the struct. Signed-off-by: Abhishek Kumar <> Signed-off-by: Junio C Hamano <>
2020-05-13Merge branch 'tb/shallow-cleanup'Junio C Hamano
Code cleanup. * tb/shallow-cleanup: shallow: use struct 'shallow_lock' for additional safety shallow.h: document '{commit,rollback}_shallow_file' shallow: extract a header file for shallow-related functions commit: make 'commit_graft_pos' non-static
2020-04-30shallow: extract a header file for shallow-related functionsTaylor Blau
There are many functions in commit.h that are more related to shallow repositories than they are to any sort of generic commit machinery. Likely this began when there were only a few shallow-related functions, and commit.h seemed a reasonable enough place to put them. But, now there are a good number of shallow-related functions, and placing them all in 'commit.h' doesn't make sense. This patch extracts a 'shallow.h', which takes all of the declarations from 'commit.h' for functions which already exist in 'shallow.c'. We will bring the remaining shallow-related functions defined in 'commit.c' in a subsequent patch. For now, move only the ones that already are implemented in 'shallow.c', and update the necessary includes. Signed-off-by: Taylor Blau <> Signed-off-by: Junio C Hamano <>
2020-04-30commit: make 'commit_graft_pos' non-staticTaylor Blau
In the next patch, some functions will be moved from 'commit.c' to have prototypes in a new 'shallow.h' and their implementations in 'shallow.c'. Three functions in 'commit.c' use 'commit_graft_pos()' (they are 'register_commit_graft()', 'lookup_commit_graft()', and 'unregister_shallow()'). The first two of these will stay in 'commit.c', but the latter will move to 'shallow.c', and thus needs 'commit_graft_pos' to be non-static. Prepare for that by making 'commit_graft_pos' non-static so that it can be called from both 'commit.c' and 'shallow.c'. Signed-off-by: Taylor Blau <> Signed-off-by: Junio C Hamano <>
2020-03-27Merge branch 'at/rebase-fork-point-regression-fix'Junio C Hamano
The "--fork-point" mode of "git rebase" regressed when the command was rewritten in C back in 2.20 era, which has been corrected. * at/rebase-fork-point-regression-fix: rebase: --fork-point regression fix
2020-02-24commit: use expected signature header for SHA-256brian m. carlson
The transition plan anticipates that we will allow signatures using multiple algorithms in a single commit. In order to do so, we need to use a different header per algorithm so that it will be obvious over which data to compute the signature. The transition plan specifies that we should use "gpgsig-sha256", so wire up the commit code such that it can write and parse the current algorithm, and it can remove the headers for any algorithm when creating a new commit. Add tests to ensure that we write using the right header and that git fsck doesn't reject these commits. Signed-off-by: brian m. carlson <> Signed-off-by: Junio C Hamano <>
2020-02-17Merge branch 'rs/strbuf-insertstr'Junio C Hamano
Code clean-up. * rs/strbuf-insertstr: mailinfo: don't insert header prefix for handle_content_type() strbuf: add and use strbuf_insertstr()
2020-02-11rebase: --fork-point regression fixJunio C Hamano
"git rebase --fork-point master" used to work OK, as it internally called "git merge-base --fork-point" that knew how to handle short refname and dwim it to the full refname before calling the underlying get_fork_point() function. This is no longer true after the command was rewritten in C, as its internall call made directly to get_fork_point() does not dwim a short ref. Move the "dwim the refname argument to the full refname" logic that is used in "git merge-base" to the underlying get_fork_point() function, so that the other caller of the function in the implementation of "git rebase" behaves the same way to fix this regression. Signed-off-by: Alex Torok <> [jc: revamped the fix and used Alex's tests] Signed-off-by: Junio C Hamano <>
2020-02-10strbuf: add and use strbuf_insertstr()René Scharfe
Add a function for inserting a C string into a strbuf. Use it throughout the source to get rid of magic string length constants and explicit strlen() calls. Like strbuf_addstr(), implement it as an inline function to avoid the implicit strlen() calls to cause runtime overhead. Helped-by: Taylor Blau <> Helped-by: Eric Sunshine <> Signed-off-by: René Scharfe <> Signed-off-by: Junio C Hamano <>
2020-01-15gpg-interface: add minTrustLevel as a configuration optionHans Jerry Illikainen
Previously, signature verification for merge and pull operations checked if the key had a trust-level of either TRUST_NEVER or TRUST_UNDEFINED in verify_merge_signature(). If that was the case, the process die()d. The other code paths that did signature verification relied entirely on the return code from check_commit_signature(). And signatures made with a good key, irregardless of its trust level, was considered valid by check_commit_signature(). This difference in behavior might induce users to erroneously assume that the trust level of a key in their keyring is always considered by Git, even for operations where it is not (e.g. during a verify-commit or verify-tag). The way it worked was by gpg-interface.c storing the result from the key/signature status *and* the lowest-two trust levels in the `result` member of the signature_check structure (the last of these status lines that were encountered got written to `result`). These are documented in GPG under the subsection `General status codes` and `Key related`, respectively [1]. The GPG documentation says the following on the TRUST_ status codes [1]: """ These are several similar status codes: - TRUST_UNDEFINED <error_token> - TRUST_NEVER <error_token> - TRUST_MARGINAL [0 [<validation_model>]] - TRUST_FULLY [0 [<validation_model>]] - TRUST_ULTIMATE [0 [<validation_model>]] For good signatures one of these status lines are emitted to indicate the validity of the key used to create the signature. The error token values are currently only emitted by gpgsm. """ My interpretation is that the trust level is conceptionally different from the validity of the key and/or signature. That seems to also have been the assumption of the old code in check_signature() where a result of 'G' (as in GOODSIG) and 'U' (as in TRUST_NEVER or TRUST_UNDEFINED) were both considered a success. The two cases where a result of 'U' had special meaning were in verify_merge_signature() (where this caused git to die()) and in format_commit_one() (where it affected the output of the %G? format specifier). I think it makes sense to refactor the processing of TRUST_ status lines such that users can configure a minimum trust level that is enforced globally, rather than have individual parts of git (e.g. merge) do it themselves (except for a grace period with backward compatibility). I also think it makes sense to not store the trust level in the same struct member as the key/signature status. While the presence of a TRUST_ status code does imply that the signature is good (see the first paragraph in the included snippet above), as far as I can tell, the order of the status lines from GPG isn't well-defined; thus it would seem plausible that the trust level could be overwritten with the key/signature status if they were stored in the same member of the signature_check structure. This patch introduces a new configuration option: gpg.minTrustLevel. It consolidates trust-level verification to gpg-interface.c and adds a new `trust_level` member to the signature_check structure. Backward-compatibility is maintained by introducing a special case in verify_merge_signature() such that if no user-configurable gpg.minTrustLevel is set, then the old behavior of rejecting TRUST_UNDEFINED and TRUST_NEVER is enforced. If, on the other hand, gpg.minTrustLevel is set, then that value overrides the old behavior. Similarly, the %G? format specifier will continue show 'U' for signatures made with a key that has a trust level of TRUST_UNDEFINED or TRUST_NEVER, even though the 'U' character no longer exist in the `result` member of the signature_check structure. A new format specifier, %GT, is also introduced for users that want to show all possible trust levels for a signature. Another approach would have been to simply drop the trust-level requirement in verify_merge_signature(). This would also have made the behavior consistent with other parts of git that perform signature verification. However, requiring a minimum trust level for signing keys does seem to have a real-world use-case. For example, the build system used by the Qubes OS project currently parses the raw output from verify-tag in order to assert a minimum trust level for keys used to sign git tags [2]. [1];a=blob;f=doc/doc/DETAILS;h=bd00006e933ac56719b1edd2478ecd79273eae72;hb=refs/heads/master [2] Signed-off-by: Hans Jerry Illikainen <> Signed-off-by: Junio C Hamano <>
2019-12-01Merge branch 'jk/cleanup-object-parsing-and-fsck'Junio C Hamano
Crufty code and logic accumulated over time around the object parsing and low-level object access used in "git fsck" have been cleaned up. * jk/cleanup-object-parsing-and-fsck: (23 commits) fsck: accept an oid instead of a "struct tree" for fsck_tree() fsck: accept an oid instead of a "struct commit" for fsck_commit() fsck: accept an oid instead of a "struct tag" for fsck_tag() fsck: rename vague "oid" local variables fsck: don't require an object struct in verify_headers() fsck: don't require an object struct for fsck_ident() fsck: drop blob struct from fsck_finish() fsck: accept an oid instead of a "struct blob" for fsck_blob() fsck: don't require an object struct for report() fsck: only require an oid for skiplist functions fsck: only provide oid/type in fsck_error callback fsck: don't require object structs for display functions fsck: use oids rather than objects for object_name API fsck_describe_object(): build on our get_object_name() primitive fsck: unify object-name code fsck: require an actual buffer for non-blobs fsck: stop checking tag->tagged fsck: stop checking commit->parent counts fsck: stop checking commit->tree value commit, tag: don't set parsed bit for parse failures ...
2019-11-10Merge branch 'pw/post-commit-from-sequencer'Junio C Hamano
"rebase -i" ceased to run post-commit hook by mistake in an earlier update, which has been corrected. * pw/post-commit-from-sequencer: sequencer: run post-commit hook move run_commit_hook() to libgit and use it there sequencer.h fix placement of #endif t3404: remove uneeded calls to set_fake_editor t3404: set $EDITOR in subshell t3404: remove unnecessary subshell
2019-10-28commit, tag: don't set parsed bit for parse failuresJeff King
If we can't parse a commit, then parse_commit() will return an error code. But it _also_ sets the "parsed" flag, which tells us not to bother trying to re-parse the object. That means that subsequent parses have no idea that the information in the struct may be bogus. I.e., doing this: parse_commit(commit); ... if (parse_commit(commit) < 0) die("commit is broken"); will never trigger the die(). The second parse_commit() will see the "parsed" flag and quietly return success. There are two obvious ways to fix this: 1. Stop setting "parsed" until we've successfully parsed. 2. Keep a second "corrupt" flag to indicate that we saw an error (and when the parsed flag is set, return 0/-1 depending on the corrupt flag). This patch does option 1. The obvious downside versus option 2 is that we might continually re-parse a broken object. But in practice, corruption like this is rare, and we typically die() or return an error in the caller. So it's OK not to worry about optimizing for corruption. And it's much simpler: we don't need to use an extra bit in the object struct, and callers which check the "parsed" flag don't need to learn about the corrupt bit, too. There's no new test here, because this case is already covered in t5318. Note that we do need to update the expected message there, because we now detect the problem in the return from "parse_commit()", and not with a separate check for a NULL tree. In fact, we can now ditch that explicit tree check entirely, as we're covered robustly by this change (and the previous recent change to treat a NULL tree as a parse error). We'll also give tags the same treatment. I don't know offhand of any cases where the problem can be triggered (it implies somebody ignoring a parse error earlier in the process), but consistently returning an error should cause the least surprise. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2019-10-21parse_commit_buffer(): treat lookup_tree() failure as parse errorJeff King
If parsing a commit yields a valid tree oid, but we've seen that same oid as a non-tree in the same process, the resulting commit struct will end up with a NULL tree pointer, but not otherwise report a parsing failure. That's perhaps convenient for callers which want to operate on even partially corrupt commits (e.g., by still looking at the parents). But it leaves a potential trap for most callers, who now have to manually check for a NULL tree. Most do not, and it's likely that there are possible segfaults lurking. I say "possible" because there are many candidates, and I don't think it's worth following through on reproducing them when we can just fix them all in one spot. And certainly we _have_ seen real-world cases, such as the one fixed by 806278dead (commit-graph.c: handle corrupt/missing trees, 2019-09-05). Note that we can't quite drop the check in the caller added by that commit yet, as there's some subtlety with repeated parsings (which will be addressed in a future commit). Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2019-10-21parse_commit_buffer(): treat lookup_commit() failure as parse errorJeff King
While parsing the parents of a commit, if we are able to parse an actual oid but lookup_commit() fails on it (because we previously saw it in this process as a different object type), we silently omit the parent and do not report any error to the caller. The caller has no way of knowing this happened, because even an empty parent list is a valid parse result. As a result, it's possible to fool our "rev-list" connectivity check into accepting a corrupted set of objects. There's a test for this case already in t6102, but unfortunately it has a slight error. It creates a broken commit with a parent line pointing to a blob, and then checks that rev-list notices the problem in two cases: 1. the "lone" case: we traverse the broken commit by itself (here we try to actually load the blob from disk and find out that it's not a commit) 2. the "seen" case: we parse the blob earlier in the process, and then when calling lookup_commit() we realize immediately that it's not a commit The "seen" variant for this test mistakenly parsed another commit instead of the blob, meaning that we were actually just testing the "lone" case again. Changing that reveals the breakage (and shows that this fixes it). Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2019-10-16move run_commit_hook() to libgit and use it therePhillip Wood
This function was declared in commit.h but was implemented in builtin/commit.c so was not part of libgit. Move it to libgit so we can use it in the sequencer. This simplifies the implementation of run_prepare_commit_msg_hook() and will be used in the next commit. Signed-off-by: Phillip Wood <> Signed-off-by: Junio C Hamano <>
2019-10-07Merge branch 'tb/commit-graph-harden'Junio C Hamano
The code to parse and use the commit-graph file has been made more robust against corrupted input. * tb/commit-graph-harden: commit-graph.c: handle corrupt/missing trees commit-graph.c: handle commit parsing errors t/t5318: introduce failing 'git commit-graph write' tests
2019-09-30Merge branch 'mh/release-commit-memory-fix'Junio C Hamano
Leakfix. * mh/release-commit-memory-fix: commit: free the right buffer in release_commit_memory
2019-09-09commit-graph.c: handle corrupt/missing treesTaylor Blau
Apply similar treatment as in the previous commit to handle an unchecked call to 'get_commit_tree_oid()'. Previously, a NULL return value from this function would be immediately dereferenced with '->hash', and then cause a segfault. Before dereferencing to access the 'hash' member, check the return value of 'get_commit_tree_oid()' to make sure that it is not NULL. To make this check correct, a related change is also needed in 'commit.c', which is to check the return value of 'get_commit_tree' before taking its address. If 'get_commit_tree' returns NULL, we encounter an undefined behavior when taking the address of the return value of 'get_commit_tree' and then taking '->object.oid'. (On my system, this is memory address 0x8, which is obviously wrong). Fix this by making sure that 'get_commit_tree' returns something non-NULL before digging through a structure that is not there, thus preventing a segfault down the line in the commit graph code. Signed-off-by: Taylor Blau <> Signed-off-by: Junio C Hamano <>
2019-08-26commit: free the right buffer in release_commit_memoryMike Hommey
The index field in the commit object is used to find the buffer corresponding to that commit in the buffer_slab. Resetting it first means free_commit_buffer is not going to free the right buffer. Signed-off-by: Mike Hommey <> Signed-off-by: Junio C Hamano <>
2019-07-29Merge branch 'ds/close-object-store' into maintJunio C Hamano
The commit-graph file is now part of the "files that the runtime may keep open file descriptors on, all of which would need to be closed when done with the object store", and the file descriptor to an existing commit-graph file now is closed before "gc" finalizes a new instance to replace it. * ds/close-object-store: packfile: rename close_all_packs to close_object_store packfile: close commit-graph in close_all_packs commit-graph: use raw_object_store when closing commit-graph: extract write_commit_graph_file() commit-graph: extract copy_oids_to_commits() commit-graph: extract count_distinct_commits() commit-graph: extract fill_oids_from_all_packs() commit-graph: extract fill_oids_from_commit_hex() commit-graph: extract fill_oids_from_packs() commit-graph: create write_commit_graph_context commit-graph: remove Future Work section commit-graph: collapse parameters into flags commit-graph: return with errors during write commit-graph: fix the_repository reference