summaryrefslogtreecommitdiff
path: root/commit.c
AgeCommit message (Collapse)Author
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 <xmqqy2k1dfoh.fsf@gitster.c.googlers.com> 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" <tytso@mit.edu> Signed-off-by: Bradley M. Kuhn <bkuhn@sfconservancy.org> Acked-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <dstolee@microsoft.com> Helped-by: Jonathan Nieder <jrnieder@gmail.com> Reported-by: Jay Conrod <jayconrod@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <abhishekkumar8222@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <abhishekkumar8222@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <abhishekkumar8222@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <alext9@gmail.com> [jc: revamped the fix and used Alex's tests] Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <me@ttaylorr.com> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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] https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=doc/doc/DETAILS;h=bd00006e933ac56719b1edd2478ecd79273eae72;hb=refs/heads/master [2] https://github.com/QubesOS/qubes-builder/blob/9674c1991deef45b1a1b1c71fddfab14ba50dccf/scripts/verify-git-tag#L43 Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <mh@glandium.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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
2019-07-09Merge branch 'jk/oidhash'Junio C Hamano
Code clean-up to remove hardcoded SHA-1 hash from many places. * jk/oidhash: hashmap: convert sha1hash() to oidhash() hash.h: move object_id definition from cache.h khash: rename oid helper functions khash: drop sha1-specific map types pack-bitmap: convert khash_sha1 maps into kh_oid_map delta-islands: convert island_marks khash to use oids khash: rename kh_oid_t to kh_oid_set khash: drop broken oid_map typedef object: convert create_object() to use object_id object: convert internal hash_obj() to object_id object: convert lookup_object() to use object_id object: convert lookup_unknown_object() to use object_id pack-objects: convert locate_object_entry_hash() to object_id pack-objects: convert packlist_find() to use object_id pack-bitmap-write: convert some helpers to use object_id upload-pack: rename a "sha1" variable to "oid" describe: fix accidental oid/hash type-punning
2019-07-09Merge branch 'ds/commit-graph-write-refactor'Junio C Hamano
Renamed from commit-graph-format-v2 and changed scope. * ds/commit-graph-write-refactor: 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
2019-06-20object: convert create_object() to use object_idJeff King
There are no callers left of create_object() that aren't just passing us the "hash" member of a "struct object_id". Let's take the whole struct, which gets us closer to removing all raw sha1 variables. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-06-20object: convert lookup_object() to use object_idJeff King
There are no callers left of lookup_object() that aren't just passing us the "hash" member of a "struct object_id". Let's take the whole struct, which gets us closer to removing all raw sha1 variables. It also matches the existing conversions of lookup_blob(), etc. The conversions of callers were done by hand, but they're all mechanical one-liners. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-13commit-graph: fix the_repository referenceDerrick Stolee
The parse_commit_buffer() method takes a repository pointer, so it should not refer to the_repository anymore. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-05-08Merge branch 'tb/unexpected'Junio C Hamano
Code tightening against a "wrong" object appearing where an object of a different type is expected, instead of blindly assuming that the connection between objects are correctly made. * tb/unexpected: rev-list: detect broken root trees rev-list: let traversal die when --missing is not in use get_commit_tree(): return NULL for broken tree list-objects.c: handle unexpected non-tree entries list-objects.c: handle unexpected non-blob entries t: introduce tests for unexpected object types t: move 'hex2oct' into test-lib-functions.sh
2019-04-16commit.c: add repo_get_commit_tree()Nguyễn Thái Ngọc Duy
Remove the implicit dependency on the_repository in this function. It will be used in sha1-name.c functions when they are updated to take any 'struct repository'. get_commit_tree() remains as a compat wrapper, to be slowly replaced later. Any access to "maybe_tree" field directly will result in _broken_ code after running through commit.cocci because we can't know what is the right repository to use. the_repository would be correct most of the time. But we're relying less and less on the_repository and that assumption may no longer be true. The transformation now is more of a poor man replacement for a C++ compiler catching access to private fields. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-16commit.cocci: refactor code, avoid double rewriteNguyễn Thái Ngọc Duy
"maybe" pointer in 'struct commit' is tricky because it can be lazily initialized to take advantage of commit-graph if available. This makes it not safe to access directly. This leads to a rule in commit.cocci to rewrite 'x->maybe_tree' to 'get_commit_tree(x)'. But that rule alone could lead to incorrectly rewrite assignments, e.g. from x->maybe_tree = yes to get_commit_tree(x) = yes Because of this we have a second rule to revert this effect. Szeder found out that we could do better by performing the assignment rewrite rule first, then the remaining is read-only access and handled by the current first rule. For this to work, we need to transform "x->maybe_tree = y" to something that does NOT contain "x->maybe_tree" to avoid the original first rule. This is where set_commit_tree() comes in. Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-04-10get_commit_tree(): return NULL for broken treeJeff King
Return NULL from 'get_commit_tree()' when a commit's root tree is corrupt, doesn't exist, or points to an object which is not a tree. In [1], this situation became a BUG(), but it can certainly occur in cases which are not a bug in Git, for e.g., if a caller manually crafts a commit whose tree is corrupt in any of the above ways. Note that the expect_failure test in t6102 triggers this BUG(), but we can't flip it to expect_success yet. Solving this problem actually reveals a second bug. [1]: 7b8a21dba1 (commit-graph: lazy-load trees for commits, 2018-04-06) Co-authored-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-05Merge branch 'sb/more-repo-in-api'Junio C Hamano
The in-core repository instances are passed through more codepaths. * sb/more-repo-in-api: (23 commits) t/helper/test-repository: celebrate independence from the_repository path.h: make REPO_GIT_PATH_FUNC repository agnostic commit: prepare free_commit_buffer and release_commit_memory for any repo commit-graph: convert remaining functions to handle any repo submodule: don't add submodule as odb for push submodule: use submodule repos for object lookup pretty: prepare format_commit_message to handle arbitrary repositories commit: prepare logmsg_reencode to handle arbitrary repositories commit: prepare repo_unuse_commit_buffer to handle any repo commit: prepare get_commit_buffer to handle any repo commit-reach: prepare in_merge_bases[_many] to handle any repo commit-reach: prepare get_merge_bases to handle any repo commit-reach.c: allow get_merge_bases_many_0 to handle any repo commit-reach.c: allow remove_redundant to handle any repo commit-reach.c: allow merge_bases_many to handle any repo commit-reach.c: allow paint_down_to_common to handle any repo commit: allow parse_commit* to handle any repo object: parse_object to honor its repository argument object-store: prepare has_{sha1, object}_file to handle any repo object-store: prepare read_object_file to deal with any repo ...
2018-12-28commit: prepare free_commit_buffer and release_commit_memory for any repoStefan Beller
Pass the object pool to free_commit_buffer and release_commit_memory, such that we can eliminate access to 'the_repository'. Also remove the TODO in release_commit_memory, as commit->util was removed in 9d2c97016f (commit.h: delete 'util' field in struct commit, 2018-05-19) Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-18Merge branch 'jk/verify-sig-merge-into-void'Junio C Hamano
"git merge" and "git pull" that merges into an unborn branch used to completely ignore "--verify-signatures", which has been corrected. * jk/verify-sig-merge-into-void: pull: handle --verify-signatures for unborn branch merge: handle --verify-signatures for unborn branch merge: extract verify_merge_signature() helper
2018-11-18Merge branch 'ds/reachable-topo-order'Junio C Hamano
The revision walker machinery learned to take advantage of the commit generation numbers stored in the commit-graph file. * ds/reachable-topo-order: t6012: make rev-list tests more interesting revision.c: generation-based topo-order algorithm commit/revisions: bookkeeping before refactoring revision.c: begin refactoring --topo-order logic test-reach: add rev-list tests test-reach: add run_three_modes method prio-queue: add 'peek' operation
2018-11-14commit: prepare repo_unuse_commit_buffer to handle any repoStefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-14commit: prepare get_commit_buffer to handle any repoStefan Beller
Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-14commit: allow parse_commit* to handle any repoStefan Beller
Just like the previous commit, parse_commit and friends are used a lot and are found in new patches, so we cannot change their signature easily. Re-introduce these function prefixed with 'repo_' that take a repository argument and keep the original as a shallow macro. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-07merge: extract verify_merge_signature() helperJeff King
The logic to implement "merge --verify-signatures" is inline in cmd_merge(), but this site misses some cases. Let's extract the logic into a function so we can call it from more places. We'll move it to commit.[ch], since one of the callers (git-pull) is outside our source file. This function isn't all that general (after all, its main function is to exit the program) but it's not worth trying to fix that. The heavy lifting is done by check_commit_signature(), and our purpose here is just sharing the die() logic. We'll mark it with a comment to make that clear. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-02commit/revisions: bookkeeping before refactoringDerrick Stolee
There are a few things that need to move around a little before making a big refactoring in the topo-order logic: 1. We need access to record_author_date() and compare_commits_by_author_date() in revision.c. These are used currently by sort_in_topological_order() in commit.c. 2. Moving these methods to commit.h requires adding an author_date_slab declaration to commit.h. Consumers will need their own implementation. 3. The add_parents_to_list() method in revision.c performs logic around the UNINTERESTING flag and other special cases depending on the struct rev_info. Allow this method to ignore a NULL 'list' parameter, as we will not be populating the list for our walk. Also rename the method to the slightly more generic name process_parents() to make clear that this method does more than add to a list (and no list is required anymore). Helped-by: Jeff King <peff@peff.net> Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>