summaryrefslogtreecommitdiffstats
path: root/merge-ort.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'en/ort-inner-merge-error-fix'Junio C Hamano2024-07-161-45/+124
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | The "ort" merge backend saw one bugfix for a crash that happens when inner merge gets killed, and assorted code clean-ups. * en/ort-inner-merge-error-fix: merge-ort: fix missing early return merge-ort: convert more error() cases to path_msg() merge-ort: upon merge abort, only show messages causing the abort merge-ort: loosen commented requirements merge-ort: clearer propagation of failure-to-function from merge_submodule merge-ort: fix type of local 'clean' var in handle_content_merge () merge-ort: maintain expected invariant for priv member merge-ort: extract handling of priv member into reusable function
| * merge-ort: fix missing early returnElijah Newren2024-07-061-0/+1
| | | | | | | | | | | | | | | | | | | | One of the conversions in f19b9165 (merge-ort: convert more error() cases to path_msg(), 2024-06-19) accidentally lost the early return. Restore it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * merge-ort: convert more error() cases to path_msg()Elijah Newren2024-06-201-12/+41
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | merge_submodule() stores errors using path_msg(), whereas other call sites make use of the error() function. This is inconsistent, and moving towards path_msg() seems more friendly for libification efforts since it will allow the caller to determine whether the error messages need to be printed. Note that this deferred handling of error messages changes the error message in a recursive merge from error: failed to execute internal merge to From inner merge: error: failed to execute internal merge which provides a little more information about the error which may be useful. Since the recursive merge strategy still only shows the older error, we had to adjust the new testcase introduced a few commits ago to just search for the older message somewhere in the output. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * merge-ort: upon merge abort, only show messages causing the abortElijah Newren2024-06-201-25/+53
| | | | | | | | | | | | | | | | | | | | | | | | When something goes wrong enough that we need to abort early and not even attempt merging the remaining files, it probably does not make sense to report conflicts messages for the subset of files we processed before hitting the fatal error. Instead, only show the messages associated with paths where we hit the fatal error. Also, print these messages to stderr rather than stdout. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * merge-ort: loosen commented requirementsElijah Newren2024-06-201-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | The comment above type_short_descriptions claimed that the order had to match what was found in the conflict_info_and_types enum. Since type_short_descriptions uses designated initializers, the order should not actually matter; I am guessing that positional initializers may have been under consideration when that comment was added, but the comment was not updated when designated initializers were chosen. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * merge-ort: clearer propagation of failure-to-function from merge_submoduleElijah Newren2024-06-201-0/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | The 'clean' member variable is somewhat of a tri-state (1 = clean, 0 = conflicted, -1 = failure-to-determine), but we often like to think of it as binary (ignoring the possibility of a negative value) and use constructs like '!clean' to reflect this. However, these constructs can make codepaths more difficult to understand, unless we handle the negative case early and return pre-emptively; do that in handle_content_merge() to make the code a bit easier to read. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * merge-ort: fix type of local 'clean' var in handle_content_merge ()Elijah Newren2024-06-201-2/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | handle_content_merge() returns an int. Every caller of handle_content_merge() expects an int. However, we declare a local variable 'clean' that we use for the return value to be unsigned. To make matters worse, we also assign 'clean' the return value of merge_submodule() in one codepath, which is defined to return an int. It seems that the only reason to have 'clean' be unsigned was to allow a cutesy bit manipulation operation to be well-defined. Fix the type of the 'clean' local in handle_content_merge(). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * merge-ort: maintain expected invariant for priv memberElijah Newren2024-06-201-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The calling convention for the merge machinery is One call to init_merge_options() One or more calls to merge_incore_[non]recursive() One call to merge_finalize() (possibly indirectly via merge_switch_to_result()) Both merge_switch_to_result() and merge_finalize() expect opt->priv == NULL && result->priv != NULL which is supposed to be set up by our move_opt_priv_to_result_priv() function. However, two codepaths dealing with error cases did not execute this necessary logic, which could result in assertion failures (or, if assertions were compiled out, could result in segfaults). Fix the oversight and add a test that would have caught one of these problems. While at it, also tighten an existing test for a non-recursive merge to verify that it fails with appropriate status. Most merge tests in the testsuite check either for success or conflicts; those testing for neither are rare and it is good to ensure they support the invariant assumed by builtin/merge.c in this comment: /* * The backend exits with 1 when conflicts are * left to be resolved, with 2 when it does not * handle the given merge at all. */ So, explicitly check for the exit status of 2 in these cases. Reported-by: Matt Cree <matt.cree@gearset.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * merge-ort: extract handling of priv member into reusable functionElijah Newren2024-06-201-5/+22
| | | | | | | | | | | | | | | | | | | | | | In preparation for a subsequent commit which will ensure we do not forget to maintain our invariants for the priv member in error codepaths, extract the necessary functionality out into a separate function. This change is cosmetic at this point, and introduces no changes beyond an extra assertion sanity check. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'ps/leakfixes-more'Junio C Hamano2024-07-081-4/+8
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | More memory leaks have been plugged. * ps/leakfixes-more: (29 commits) builtin/blame: fix leaking ignore revs files builtin/blame: fix leaking prefixed paths blame: fix leaking data for blame scoreboards line-range: plug leaking find functions merge: fix leaking merge bases builtin/merge: fix leaking `struct cmdnames` in `get_strategy()` sequencer: fix memory leaks in `make_script_with_merges()` builtin/clone: plug leaking HEAD ref in `wanted_peer_refs()` apply: fix leaking string in `match_fragment()` sequencer: fix leaking string buffer in `commit_staged_changes()` commit: fix leaking parents when calling `commit_tree_extended()` config: fix leaking "core.notesref" variable rerere: fix various trivial leaks builtin/stash: fix leak in `show_stash()` revision: free diff options builtin/log: fix leaking commit list in git-cherry(1) merge-recursive: fix memory leak when finalizing merge builtin/merge-recursive: fix leaking object ID bases builtin/difftool: plug memory leaks in `run_dir_diff()` object-name: free leaking object contexts ...
| * | merge: fix leaking merge basesPatrick Steinhardt2024-06-111-4/+8
| |/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When calling either the recursive or the ORT merge machineries we need to provide a list of merge bases. The ownership of that parameter is then implicitly transferred to the callee, which is somewhat fishy. Furthermore, that list may leak in some cases where the merge machinery runs into an error, thus causing a memory leak. Refactor the code such that we stop transferring ownership. Instead, the merge machinery will now create its own local copies of the passed in list as required if they need to modify the list. Free the list at the callsites as required. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* / global: introduce `USE_THE_REPOSITORY_VARIABLE` macroPatrick Steinhardt2024-06-141-0/+2
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* Merge branch 'pw/checkout-conflict-errorfix'Junio C Hamano2024-04-011-1/+2
|\ | | | | | | | | | | | | | | | | | | | | | | | | "git checkout --conflict=bad" reported a bad conflictStyle as if it were given to a configuration variable; it has been corrected to report that the command line option is bad. * pw/checkout-conflict-errorfix: checkout: fix interaction between --conflict and --merge checkout: cleanup --conflict=<style> parsing merge options: add a conflict style member merge-ll: introduce LL_MERGE_OPTIONS_INIT xdiff-interface: refactor parsing of merge.conflictstyle
| * merge options: add a conflict style memberPhillip Wood2024-03-141-0/+1
| | | | | | | | | | | | | | | | | | Add a conflict_style member to `struct merge_options` and `struct ll_merge_options` to allow callers to override the default conflict style. This 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>
| * merge-ll: introduce LL_MERGE_OPTIONS_INITPhillip Wood2024-03-141-1/+1
| | | | | | | | | | | | | | | | | | Introduce a macro to initialize `struct ll_merge_options` in preparation for the next commit that will add a new member that needs to be initialized to a non-zero value. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'eb/hash-transition'Junio C Hamano2024-03-281-5/+6
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Work to support a repository that work with both SHA-1 and SHA-256 hash algorithms has started. * eb/hash-transition: (30 commits) t1016-compatObjectFormat: add tests to verify the conversion between objects t1006: test oid compatibility with cat-file t1006: rename sha1 to oid test-lib: compute the compatibility hash so tests may use it builtin/ls-tree: let the oid determine the output algorithm object-file: handle compat objects in check_object_signature tree-walk: init_tree_desc take an oid to get the hash algorithm builtin/cat-file: let the oid determine the output algorithm rev-parse: add an --output-object-format parameter repository: implement extensions.compatObjectFormat object-file: update object_info_extended to reencode objects object-file-convert: convert commits that embed signed tags object-file-convert: convert commit objects when writing object-file-convert: don't leak when converting tag objects object-file-convert: convert tag objects when writing object-file-convert: add a function to convert trees between algorithms object: factor out parse_mode out of fast-import and tree-walk into in object.h cache: add a function to read an OID of a specific algorithm tag: sign both hashes commit: export add_header_signature to support handling signatures on tags ...
| * | tree-walk: init_tree_desc take an oid to get the hash algorithmEric W. Biederman2023-10-021-5/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To make it possible for git ls-tree to display the tree encoded in the hash algorithm of the oid specified to git ls-tree, update init_tree_desc to take as a parameter the oid of the tree object. Update all callers of init_tree_desc and init_tree_desc_gently to pass the oid of the tree object. Use the oid of the tree object to discover the hash algorithm of the oid and store that hash algorithm in struct tree_desc. Use the hash algorithm in decode_tree_entry and update_tree_entry_internal to handle reading a tree object encoded in a hash algorithm that differs from the repositories hash algorithm. Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'js/merge-base-with-missing-commit'Junio C Hamano2024-03-111-11/+81
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Make sure failure return from merge_bases_many() is properly caught. * js/merge-base-with-missing-commit: merge-ort/merge-recursive: do report errors in `merge_submodule()` merge-recursive: prepare for `merge_submodule()` to report errors commit-reach(repo_get_merge_bases_many_dirty): pass on errors commit-reach(repo_get_merge_bases_many): pass on "missing commits" errors commit-reach(get_octopus_merge_bases): pass on "missing commits" errors commit-reach(repo_get_merge_bases): pass on "missing commits" errors commit-reach(get_merge_bases_many_0): pass on "missing commits" errors commit-reach(merge_bases_many): pass on "missing commits" errors commit-reach(paint_down_to_common): start reporting errors commit-reach(paint_down_to_common): prepare for handling shallow commits commit-reach(repo_in_merge_bases_many): report missing commits commit-reach(repo_in_merge_bases_many): optionally expect missing commits commit-reach(paint_down_to_common): plug two memory leaks
| * | | merge-ort/merge-recursive: do report errors in `merge_submodule()`Johannes Schindelin2024-03-091-0/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 24876ebf68b (commit-reach(repo_in_merge_bases_many): report missing commits, 2024-02-28), I taught `merge_submodule()` to handle errors reported by `repo_in_merge_bases_many()`. However, those errors were not passed through to the callers. That was unintentional, and this commit remedies that. Note that `find_first_merges()` can now also return -1 (because it passes through that return value from `repo_in_merge_bases()`), and this commit also adds the forgotten handling for that scenario. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Acked-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | commit-reach(repo_get_merge_bases): pass on "missing commits" errorsJohannes Schindelin2024-02-291-1/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The `merge_bases_many()` function was just taught to indicate parsing errors, and now the `repo_get_merge_bases()` function (which is also surfaced via the `repo_get_merge_bases()` macro) is aware of that, too. Naturally, there are a lot of callers that need to be adjusted now, too. Next step: adjust the callers of `get_octopus_merge_bases()`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | commit-reach(repo_in_merge_bases_many): report missing commitsJohannes Schindelin2024-02-281-10/+71
| | |/ | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some functions in Git's source code follow the convention that returning a negative value indicates a fatal error, e.g. repository corruption. Let's use this convention in `repo_in_merge_bases()` to report when one of the specified commits is missing (i.e. when `repo_parse_commit()` reports an error). Also adjust the callers of `repo_in_merge_bases()` to handle such negative return values. Note: As of this patch, errors are returned only if any of the specified merge heads is missing. Over the course of the next patches, missing commits will also be reported by the `paint_down_to_common()` function, which is called by `repo_in_merge_bases_many()`, and those errors will be properly propagated back to the caller at that stage. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'js/merge-tree-3-trees'Junio C Hamano2024-03-081-5/+11
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git merge-tree" has learned that the three trees involved in the 3-way merge only need to be trees, not necessarily commits. * js/merge-tree-3-trees: fill_tree_descriptor(): mark error message for translation cache-tree: avoid an unnecessary check Always check `parse_tree*()`'s return value t4301: verify that merge-tree fails on missing blob objects merge-ort: do check `parse_tree()`'s return value merge-tree: fail with a non-zero exit code on missing tree objects merge-tree: accept 3 trees as arguments
| * | | Always check `parse_tree*()`'s return valueJohannes Schindelin2024-02-231-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Otherwise we may easily run into serious crashes: For example, if we run `init_tree_desc()` directly after a failed `parse_tree()`, we are accessing uninitialized data or trying to dereference `NULL`. Note that the `parse_tree()` function already takes care of showing an error message. The `parse_tree_indirectly()` and `repo_get_commit_tree()` functions do not, therefore those latter call sites need to show a useful error message while the former do not. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | merge-ort: do check `parse_tree()`'s return valueJohannes Schindelin2024-02-231-2/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous commit fixed a bug where a missing tree was reported, but not treated as an error. This patch addresses the same issue for the remaining two callers of `parse_tree()`. This change is not accompanied by a regression test because the code in question is only reached at the `checkout` stage, i.e. after the merge has happened (and therefore the tree objects could only be missing if the disk had gone bad in that short time window, or something similarly tricky to recreate in the test suite). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | merge-tree: fail with a non-zero exit code on missing tree objectsJohannes Schindelin2024-02-231-3/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When `git merge-tree` encounters a missing tree object, it should error out and not continue quietly as if nothing had happened. However, as of time of writing, `git merge-tree` _does_ continue, and then offers the empty tree as result. Let's fix this. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | merge-ort: turn submodule conflict suggestions into an advicePhilippe Blain2024-02-261-1/+2
| |/ / |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new advice type 'submoduleMergeConflict' for the error message shown when a non-trivial submodule conflict is encountered, which was added in 4057523a40 (submodule merge: update conflict error message, 2022-08-04). That commit mentions making this message an advice as possible future work. The message can now be disabled with the advice mechanism. Update the tests as the expected message now appears on stderr instead of stdout. Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'jc/comment-style-fixes'Junio C Hamano2024-02-061-1/+1
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rewrite //-comments to /* comments */ in files whose comments prevalently use the latter. * jc/comment-style-fixes: reftable/pq_test: comment style fix merge-ort.c: comment style fix builtin/worktree: comment style fixes
| * | | merge-ort.c: comment style fixJunio C Hamano2024-01-291-1/+1
| |/ / | | | | | | | | | Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | refs: convert AUTO_MERGE to become a normal pseudo-refPatrick Steinhardt2024-01-191-7/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In 70c70de616 (refs: complete list of special refs, 2023-12-14) we have inrtoduced a new `is_special_ref()` function that classifies some refs as being special. The rule is that special refs are exclusively read and written via the filesystem directly, whereas normal refs exclucsively go via the refs API. The intent of that commit was to record the status quo so that we know to route reads of such special refs consistently. Eventually, the list should be reduced to its bare minimum of refs which really are special, namely FETCH_HEAD and MERGE_HEAD. Follow up on this promise and convert the AUTO_MERGE ref to become a normal pseudo-ref by using the refs API to both read and write it instead of accessing the filesystem directly. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | treewide: remove unnecessary includes in source filesElijah Newren2023-12-261-1/+0
| | | | | | | | | | | | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | treewide: remove unnecessary includes in source filesElijah Newren2023-12-261-2/+0
|/ / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Each of these were checked with gcc -E -I. ${SOURCE_FILE} | grep ${HEADER_FILE} to ensure that removing the direct inclusion of the header actually resulted in that header no longer being included at all (i.e. that no other header pulled it in transitively). ...except for a few cases where we verified that although the header was brought in transitively, nothing from it was directly used in that source file. These cases were: * builtin/credential-cache.c * builtin/pull.c * builtin/send-pack.c Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'wx/merge-ort-comment-typofix'Junio C Hamano2023-10-291-1/+1
|\ \ | | | | | | | | | | | | | | | | | | Typofix. * wx/merge-ort-comment-typofix: merge-ort.c: fix typo 'neeed' to 'needed'
| * | merge-ort.c: fix typo 'neeed' to 'needed'王常新2023-10-221-1/+1
| | | | | | | | | | | | | | | Signed-off-by: 王常新 <wchangxin824@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'jc/merge-ort-attr-index-fix'Junio C Hamano2023-10-181-0/+1
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix "git merge-tree" to stop segfaulting when the --attr-source option is used. * jc/merge-ort-attr-index-fix: merge-ort: initialize repo in index state
| * | | merge-ort: initialize repo in index stateJohn Cai2023-10-091-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | initialize_attr_index() does not initialize the repo member of attr_index. Starting in 44451a2e5e (attr: teach "--attr-source=<tree>" global option to "git", 2023-05-06), this became a problem because istate->repo gets passed down the call chain starting in git_check_attr(). This gets passed all the way down to replace_refs_enabled(), which segfaults when accessing r->gitdir. Fix this by initializing the repository in the index state. Signed-off-by: John Cai <johncai86@gmail.com> Helped-by: Christian Couder <christian.couder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | merge-ort: lowercase a few error messagesJeff King2023-09-171-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | As noted in CodingGuidelines, error messages should not be capitalized. Fix up a few of these that were copied verbatim from merge-recursive to match our modern style. We'll likewise fix up the matching ones from merge-recursive. We care a bit less there, since the hope is that it will eventually go away. But besides being the right thing to do in the meantime, it is necessary for t6406 to pass both with and without GIT_TEST_MERGE_ALGORITHM set (one of our CI jobs sets it to "recursive", which will use the merge-recursive.c code). An alternative would be to use "grep -i" in the test to check the message, but it's nice for the test suite to be be more exact (we'd notice if the capitalization fix regressed). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | merge-ort: drop unused "opt" parameter from merge_check_renames_reusable()Jeff King2023-09-141-3/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The merge_options parameter has never been used since the function was introduced in 64aceb6d73 (merge-ort: add code to check for whether cached renames can be reused, 2021-05-20). In theory some merge options might impact our decisions here, but that has never been the case so far. Let's drop it to appease -Wunused-parameter; it would be easy to add back later if we need to (there is only one caller). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | merge-ort: drop unused parameters from detect_and_process_renames()Jeff King2023-09-141-6/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This function takes three trees representing the merge base and both sides of the merge, but never looks at any of them. This is due to f78cf97617 (merge-ort: call diffcore_rename() directly, 2021-02-14). Prior to that commit, we passed pairs of trees to diff_tree_oid(). But after that commit, we collect a custom diff_queue for each pair in the merge_options struct, and just run diffcore_rename() on the result. So the function does not need to know about the original trees at all anymore. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | merge-ort: stop passing "opt" to read_oid_strbuf()Jeff King2023-09-141-4/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This function doesn't look at its merge_options parameter. It used to pass it down to err(), but that function no longer exists (and didn't look at "opt" anyway). We can drop it here. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | merge-ort: drop custom err() functionJeff King2023-09-141-23/+5
| |_|/ |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The merge-ort code has an err() function, but it's really just error() in disguise. It differs in two ways: 1. It takes a "struct merge_options" argument. But the function completely ignores it! We can simply remove it. 2. It formats the error string into a strbuf, prepending "error: ", and then feeds the result into error(). But this is wrong! The error() function already adds the prefix, so we end up with: error: error: Failed to execute internal merge So let's just drop this function entirely and call error() directly, as the functions are otherwise identical (note that they both always return -1). Presumably nobody noticed the bogus messages because they are quite hard to trigger (they are mostly internal errors reading and writing objects). However, one easy trigger is a custom merge driver which dies by signal; we have a test already here, but we were not checking the contents of stderr. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | object-store-ll.h: split this header out of object-store.hElijah Newren2023-06-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The vast majority of files including object-store.h did not need dir.h nor khash.h. Split the header into two files, and let most just depend upon object-store-ll.h, while letting the two callers that need it depend on the full object-store.h. After this patch: $ git grep -h include..object-store | sort | uniq -c 2 #include "object-store.h" 129 #include "object-store-ll.h" Diff best viewed with `--color-moved`. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | merge-ll: rename from ll-mergeElijah Newren2023-06-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | A long term (but rather minor) pet-peeve of mine was the name ll-merge.[ch]. I thought it made it harder to realize what stuff was related to merging when I was working on the merge machinery and trying to improve it. Further, back in d1cbe1e6d8a ("hash-ll.h: split out of hash.h to remove dependency on repository.h", 2023-04-22), we have split the portions of hash.h that do not depend upon repository.h into a "hash-ll.h" (due to the recommendation to use "ll" for "low-level" in its name[1], but which I used as a suffix precisely because of my distaste for "ll-merge"). When we discussed adding additional "*-ll.h" files, a request was made that we use "ll" consistently as either a prefix or a suffix. Since it is already in use as both a prefix and a suffix, the only way to do so is to rename some files. Besides my distaste for the ll-merge.[ch] name, let me also note that the files ll-fsmonitor.h, ll-hash.h, ll-merge.h, ll-object-store.h, ll-read-cache.h would have essentially nothing to do with each other and make no sense to group. But giving them the common "ll-" prefix would group them. Using "-ll" as a suffix thus seems just much more logical to me. Rename ll-merge.[ch] to merge-ll.[ch] to achieve this consistency, and to ensure we get a more logical grouping of files. [1] https://lore.kernel.org/git/kl6lsfcu1g8w.fsf@chooglen-macbookpro.roam.corp.google.com/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | repository: remove unnecessary include of path.hElijah Newren2023-06-211-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | This also made it clear that several .c files that depended upon path.h were missing a #include for it; add the missing includes while at it. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | cache.h: remove this no-longer-used headerElijah Newren2023-06-211-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Since this header showed up in some places besides just #include statements, update/clean-up/remove those other places as well. Note that compat/fsmonitor/fsm-path-utils-darwin.c previously got away with violating the rule that all files must start with an include of git-compat-util.h (or a short-list of alternate headers that happen to include it first). This change exposed the violation and caused it to stop building correctly; fix it by having it include git-compat-util.h first, as per policy. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | read-cache*.h: move declarations for read-cache.c functions from cache.hElijah Newren2023-06-211-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For the functions defined in read-cache.c, move their declarations from cache.h to a new header, read-cache-ll.h. Also move some related inline functions from cache.h to read-cache.h. The purpose of the read-cache-ll.h/read-cache.h split is that about 70% of the sites don't need the inline functions and the extra headers they include. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | sparse-index.h: move declarations for sparse-index.c from cache.hElijah Newren2023-06-211-0/+1
|/ / | | | | | | | | | | | | | | Note in particular that this reverses the decision made in 118a2e8bde0 ("cache: move ensure_full_index() to cache.h", 2021-04-01). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'en/header-split-cache-h-part-2'Junio C Hamano2023-05-101-0/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | More header clean-up. * en/header-split-cache-h-part-2: (22 commits) reftable: ensure git-compat-util.h is the first (indirect) include diff.h: reduce unnecessary includes object-store.h: reduce unnecessary includes commit.h: reduce unnecessary includes fsmonitor: reduce includes of cache.h cache.h: remove unnecessary headers treewide: remove cache.h inclusion due to previous changes cache,tree: move basic name compare functions from read-cache to tree cache,tree: move cmp_cache_name_compare from tree.[ch] to read-cache.c hash-ll.h: split out of hash.h to remove dependency on repository.h tree-diff.c: move S_DIFFTREE_IFXMIN_NEQ define from cache.h dir.h: move DTYPE defines from cache.h versioncmp.h: move declarations for versioncmp.c functions from cache.h ws.h: move declarations for ws.c functions from cache.h match-trees.h: move declarations for match-trees.c functions from cache.h pkt-line.h: move declarations for pkt-line.c functions from cache.h base85.h: move declarations for base85.c functions from cache.h copy.h: move declarations for copy.c functions from cache.h server-info.h: move declarations for server-info.c functions from cache.h packfile.h: move pack_window and pack_entry from cache.h ...
| * | match-trees.h: move declarations for match-trees.c functions from cache.hElijah Newren2023-04-241-0/+1
| | | | | | | | | | | | | | | Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'en/ort-finalize-after-0-merges-fix'Junio C Hamano2023-05-021-4/+4
|\ \ \ | |/ / |/| | | | | | | | | | | | | | A small API fix to the ort merge strategy backend. * en/ort-finalize-after-0-merges-fix: merge-ort: fix calling merge_finalize() with no intermediate merge
| * | merge-ort: fix calling merge_finalize() with no intermediate mergeElijah Newren2023-04-241-4/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If some code sets up the data structures for a merge, but then never actually performs one before calling merge_finalize(), then merge_finalize() wouldn't notice that result->priv was NULL and return early, resulting in following that NULL pointer and getting a segfault. There is currently no code in the git codebase that does this, but this issue was found during testing of some proposed patches that had the following structure: struct merge_options merge_opt; struct merge_result result; init_merge_options(&merge_opt, the_repository); memset(&result, 0, sizeof(result)); <do N merges, for some value of N> merge_finalize(&merge_opt, &result); where some flags could cause the code to have N=0, i.e. doing no merges. Add a check for result->priv being NULL and return early to avoid a segfault in these kinds of cases. While at it, ensure the FREE_AND_NULL() in the function does something useful with the nulling aspect, namely sets result->priv to NULL rather than a mere temporary. Reported-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>