summaryrefslogtreecommitdiff
path: root/merge-recursive.c
AgeCommit message (Collapse)Author
7 daysMerge branch 'ds/sparse-index-protections'Junio C Hamano
Builds on top of the sparse-index infrastructure to mark operations that are not ready to mark with the sparse index, causing them to fall back on fully-populated index that they always have worked with. * ds/sparse-index-protections: (47 commits) name-hash: use expand_to_path() sparse-index: expand_to_path() name-hash: don't add directories to name_hash revision: ensure full index resolve-undo: ensure full index read-cache: ensure full index pathspec: ensure full index merge-recursive: ensure full index entry: ensure full index dir: ensure full index update-index: ensure full index stash: ensure full index rm: ensure full index merge-index: ensure full index ls-files: ensure full index grep: ensure full index fsck: ensure full index difftool: ensure full index commit: ensure full index checkout: ensure full index ...
2021-04-16Merge branch 'en/ort-readiness'Junio C Hamano
Plug the ort merge backend throughout the rest of the system, and start testing it as a replacement for the recursive backend. * en/ort-readiness: Add testing with merge-ort merge strategy t6423: mark remaining expected failure under merge-ort as such Revert "merge-ort: ignore the directory rename split conflict for now" merge-recursive: add a bunch of FIXME comments documenting known bugs merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict t: mark several submodule merging tests as fixed under merge-ort merge-ort: implement CE_SKIP_WORKTREE handling with conflicted entries t6428: new test for SKIP_WORKTREE handling and conflicts merge-ort: support subtree shifting merge-ort: let renormalization change modify/delete into clean delete merge-ort: have ll_merge() use a special attr_index for renormalization merge-ort: add a special minimal index just for renormalization merge-ort: use STABLE_QSORT instead of QSORT where required
2021-04-14merge-recursive: ensure full indexDerrick Stolee
Before iterating over all cache entries, ensure that a sparse index is expanded to a full index to avoid unexpected behavior. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-04-14*: remove 'const' qualifier for struct index_stateDerrick Stolee
Several methods specify that they take a 'struct index_state' pointer with the 'const' qualifier because they intend to only query the data, not change it. However, we will be introducing a step very low in the method stack that might modify a sparse-index to become a full index in the case that our queries venture inside a sparse-directory entry. This change only removes the 'const' qualifiers that are necessary for the following change which will actually modify the implementation of index_name_stage_pos(). Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Reviewed-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-30Merge branch 'ab/read-tree'Junio C Hamano
Code simplification by removing support for a caller that is long gone. * ab/read-tree: tree.h API: simplify read_tree_recursive() signature tree.h API: expose read_tree_1() as read_tree_at() archive: stop passing "stage" through read_tree_recursive() ls-files: refactor away read_tree() ls-files: don't needlessly pass around stage variable tree.c API: move read_tree() into builtin/ls-files.c ls-files tests: add meaningful --with-tree tests show tests: add test for "git show <tree>"
2021-03-20tree.h API: simplify read_tree_recursive() signatureÆvar Arnfjörð Bjarmason
Simplify the signature of read_tree_recursive() to omit the "base", "baselen" and "stage" arguments. No callers of it use these parameters for anything anymore. The last function to call read_tree_recursive() with a non-"" path was read_tree_recursive() itself, but that was changed in ffd31f661d5 (Reimplement read_tree_recursive() using tree_entry_interesting(), 2011-03-25). The last user of the "stage" parameter went away in the last commit, and even that use was mere boilerplate. So let's remove those and rename the read_tree_recursive() function to just read_tree(). We had another read_tree() function that I've refactored away in preceding commits, since all in-tree users read trees recursively with a callback we can change the name to signify that this is the norm. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-03-20merge-recursive: add a bunch of FIXME comments documenting known bugsElijah Newren
The plan is to just delete merge-recursive, but not until everyone is comfortable with merge-ort as a replacement. Given that I haven't switched all callers of merge-recursive over yet (e.g. git-am still uses merge-recursive), maybe there's some value documenting known bugs in the algorithm in case we end up keeping it or someone wants to dig it up in the future. Signed-off-by: Elijah Newren <newren@gmail.com> Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
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 <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-12-17commit: move reverse_commit_list() from merge-recursiveElijah Newren
Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-02hashmap: provide deallocation function namesElijah Newren
hashmap_free(), hashmap_free_entries(), and hashmap_free_() have existed for a while, but aren't necessarily the clearest names, especially with hashmap_partial_clear() being added to the mix and lazy-initialization now being supported. Peff suggested we adopt the following names[1]: - hashmap_clear() - remove all entries and de-allocate any hashmap-specific data, but be ready for reuse - hashmap_clear_and_free() - ditto, but free the entries themselves - hashmap_partial_clear() - remove all entries but don't deallocate table - hashmap_partial_clear_and_free() - ditto, but free the entries This patch provides the new names and converts all existing callers over to the new naming scheme. [1] https://lore.kernel.org/git/20201030125059.GA3277724@coredump.intra.peff.net/ Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-10Merge branch 'en/eol-attrs-gotchas'Junio C Hamano
All "mergy" operations that internally use the merge-recursive machinery should honor the merge.renormalize configuration, but many of them didn't. * en/eol-attrs-gotchas: checkout: support renormalization with checkout -m <paths> merge: make merge.renormalize work for all uses of merge machinery t6038: remove problematic test t6038: make tests fail for the right reason
2020-08-03merge: make merge.renormalize work for all uses of merge machineryElijah Newren
The 'merge' command is not the only one that does merges; other commands like checkout -m or rebase do as well. Unfortunately, the only area of the code that checked for the "merge.renormalize" config setting was in builtin/merge.c, meaning it could only affect merges performed by the "merge" command. Move the handling of this config setting to merge_recursive_config() so that other commands can benefit from it as well. Fixes a few tests in t6038. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-08-02merge-recursive: fix unclear and outright wrong commentsElijah Newren
Commits 7c0a6c8e47 ("merge-recursive: move some definitions around to clean up the header", 2019-08-17), and b4db8a2b76 ("merge-recursive: remove useless parameter in merge_trees()", 2019-08-17) added some useful documentation to the functions, but had a few places where the new comments were unclear or even misleading. Fix those comments. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-05-14merge-recursive: fix rename/rename(1to2) for working tree with a binaryElijah Newren
With a rename/rename(1to2) conflict, we attempt to do a three-way merge of the file contents, so that the correct contents can be placed in the working tree at both paths. If the file is a binary, however, no content merging is possible and we should just use the original version of the file at each of the paths. Reported-by: Chunlin Zhang <zhangchunlin@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-16convert: permit passing additional metadata to filter processesbrian m. carlson
There are a variety of situations where a filter process can make use of some additional metadata. For example, some people find the ident filter too limiting and would like to include the commit or the branch in their smudged files. This information isn't available during checkout as HEAD hasn't been updated at that point, and it wouldn't be available in archives either. Let's add a way to pass this metadata down to the filter. We pass the blob we're operating on, the treeish (preferring the commit over the tree if one exists), and the ref we're operating on. Note that we won't pass this information in all cases, such as when renormalizing or when we're performing diffs, since it doesn't make sense in those cases. The data we currently get from the filter process looks like the following: command=smudge pathname=git.c 0000 With this change, we'll get data more like this: command=smudge pathname=git.c refname=refs/tags/v2.25.1 treeish=c522f061d551c9bb8684a7c3859b2ece4499b56b blob=7be7ad34bd053884ec48923706e70c81719a8660 0000 There are a couple things to note about this approach. For operations like checkout, treeish will always be a commit, since we cannot check out individual trees, but for other operations, like archive, we can end up operating on only a particular tree, so we'll provide only a tree as the treeish. Similar comments apply for refname, since there are a variety of cases in which we won't have a ref. This commit wires up the code to print this information, but doesn't pass any of it at this point. In a future commit, we'll have various code paths pass the actual useful data down. Signed-off-by: brian m. carlson <bk2204@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-09Merge branch 'en/merge-path-collision'Junio C Hamano
Handling of conflicting renames in merge-recursive have further been made consistent with how existing codepaths try to mimic what is done to add/add conflicts. * en/merge-path-collision: merge-recursive: apply collision handling unification to recursive case
2020-03-02Merge branch 'en/t3433-rebase-stat-dirty-failure'Junio C Hamano
The merge-recursive machinery failed to refresh the cache entry for a merge result in a couple of places, resulting in an unnecessary merge failure, which has been fixed. * en/t3433-rebase-stat-dirty-failure: merge-recursive: fix the refresh logic in update_file_flags t3433: new rebase testcase documenting a stat-dirty-like failure
2020-02-27merge-recursive: apply collision handling unification to recursive caseElijah Newren
In the en/merge-path-collision topic (see commit ac193e0e0aa5, "Merge branch 'en/merge-path-collision'", 2019-01-04), all the "file collision" conflict types were modified for consistency. In particular, rename/add, rename/rename(2to1) and each rename/add piece of a rename/rename(1to2)/add[/add] conflict were made to behave like add/add conflicts have always been handled. However, this consistency was not enforced when opt->priv->call_depth > 0 for rename/rename conflicts. Update rename/rename(1to2) and rename/rename(2to1) conflicts in the recursive case to also be consistent. As an added bonus, this simplifies the code considerably. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-19merge-recursive: fix the refresh logic in update_file_flagsElijah Newren
If we need to delete a higher stage entry in the index to place the file at stage 0, then we'll lose that file's stat information. In such situations we may still be able to detect that the file on disk is the version we want (as noted by our comment in the code: /* do not overwrite file if already present */ ), but we do still need to update the mtime since we are creating a new cache_entry for that file. Update the logic used to determine whether we refresh a file's mtime. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-27merge-recursive: use subtraction to flip stageJunio C Hamano
The flip_stage() helper uses a bit-flipping xor to switch between "2" and "3". While clever, this relies on a property of those two numbers that is mostly coincidence. Let's write it as a subtraction; that's more clear and would extend to other numbers if somebody copies the logic. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-27merge-recursive: silence -Wxor-used-as-pow warningJeff King
The merge-recursive code uses stage number constants like this: add = &ci->ren1->dst_entry->stages[2 ^ 1]; ... add = &ci->ren2->dst_entry->stages[3 ^ 1]; The xor has the effect of flipping the "1" bit, so that "2 ^ 1" becomes "3" and "3 ^ 1" becomes "2", which correspond to the "ours" and "theirs" stages respectively. Unfortunately, clang-10 and up issue a warning for this code: merge-recursive.c:1759:40: error: result of '2 ^ 1' is 3; did you mean '1 << 1' (2)? [-Werror,-Wxor-used-as-pow] add = &ci->ren1->dst_entry->stages[2 ^ 1]; ~~^~~ 1 << 1 merge-recursive.c:1759:40: note: replace expression with '0x2 ^ 1' to silence this warning We could silence it by using 0x2, as the compiler mentions. Or by just using the constants "2" and "3" directly. But after digging into it, I do think this bit-flip is telling us something. If we just wrote: add = &ci->ren2->dst_entry->stages[2]; for the second one, you might think that "ren2" and "2" correspond. But they don't. The logic is: ren2 is theirs, which is stage 3, but we are interested in the opposite side's stage, so flip it to 2. So let's keep the bit-flipping, but let's also put it behind a named function, which will make its purpose a bit clearer. This also has the side effect of suppressing the warning (and an optimizing compiler should be able to easily turn it into a constant as before). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-06Merge branch 'en/merge-recursive-oid-eq-simplify'Junio C Hamano
Code cleanup. * en/merge-recursive-oid-eq-simplify: merge-recursive: remove unnecessary oid_eq function
2020-01-02merge-recursive: remove unnecessary oid_eq functionElijah Newren
Back when merge-recursive was first introduced in commit 6d297f8137 (Status update on merge-recursive in C, 2006-07-08), it created a sha_eq() function. This function pre-dated the introduction of hashcmp() to cache.h by about a month, but was switched over to using hashcmp() as part of commit 9047ebbc22 (Split out merge_recursive() to merge-recursive.c, 2008-08-12). In commit b4da9d62f9 (merge-recursive: convert leaf functions to use struct object_id, 2016-06-24), sha_eq() was renamed to oid_eq() and its hashcmp() call was switched to oideq(). oid_eq() is basically just a wrapper around oideq() that has some extra checks to protect against NULL arguments or to allow short-circuiting if one of the arguments is NULL. I don't know if any caller ever tried to call with NULL arguments, but certainly none do now which means the extra checks serve no purpose. (Also, if these checks were genuinely useful, then they probably should be added to the main oideq() so all callers could benefit from them.) Reduce the cognitive overhead of having both oid_eq() and oideq(), by getting rid of merge-recursive's special oid_eq() wrapper. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-10Merge branch 'en/merge-recursive-directory-rename-fixes'Junio C Hamano
When all files from some subdirectory were renamed to the root directory, the directory rename heuristics would fail to detect that as a rename/merge of the subdirectory to the root directory, which has been corrected. * en/merge-recursive-directory-rename-fixes: t604[236]: do not run setup in separate tests merge-recursive: fix merging a subdirectory into the root directory merge-recursive: clean up get_renamed_dir_portion()
2019-10-23merge-recursive: fix merging a subdirectory into the root directoryElijah Newren
We allow renaming all entries in e.g. a directory named z/ into a directory named y/ to be detected as a z/ -> y/ rename, so that if the other side of history adds any files to the directory z/ in the mean time, we can provide the hint that they should be moved to y/. There is no reason to not allow 'y/' to be the root directory, but the code did not handle that case correctly. Add a testcase and the necessary special checks to support this case. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-23merge-recursive: clean up get_renamed_dir_portion()Elijah Newren
Dscho noted a few things making this function hard to follow. Restructure it a bit and add comments to make it easier to follow. The restructurings include: * There was a special case if-check at the end of the function checking whether someone just renamed a file within its original directory, meaning that there could be no directory rename involved. That check was slightly convoluted; it could be done in a more straightforward fashion earlier in the function, and can be done more cheaply too (no call to strncmp). * The conditions for advancing end_of_old and end_of_new before calling strchr were both confusing and unnecessary. If either points at a '/', then they need to be advanced in order to find the next '/'. If either doesn't point at a '/', then advancing them one char before calling strchr() doesn't hurt. So, just rip out the if conditions and advance both before calling strchr(). Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-15Merge branch 'ew/hashmap'Junio C Hamano
Code clean-up of the hashmap API, both users and implementation. * ew/hashmap: hashmap_entry: remove first member requirement from docs hashmap: remove type arg from hashmap_{get,put,remove}_entry OFFSETOF_VAR macro to simplify hashmap iterators hashmap: introduce hashmap_free_entries hashmap: hashmap_{put,remove} return hashmap_entry * hashmap: use *_entry APIs for iteration hashmap_cmp_fn takes hashmap_entry params hashmap_get{,_from_hash} return "struct hashmap_entry *" hashmap: use *_entry APIs to wrap container_of hashmap_get_next returns "struct hashmap_entry *" introduce container_of macro hashmap_put takes "struct hashmap_entry *" hashmap_remove takes "const struct hashmap_entry *" hashmap_get takes "const struct hashmap_entry *" hashmap_add takes "struct hashmap_entry *" hashmap_get_next takes "const struct hashmap_entry *" hashmap_entry_init takes "struct hashmap_entry *" packfile: use hashmap_entry in delta_base_cache_entry coccicheck: detect hashmap_entry.hash assignment diff: use hashmap_entry_init on moved_entry.ent
2019-10-15Merge branch 'en/merge-recursive-cleanup'Junio C Hamano
The merge-recursive machiery is one of the most complex parts of the system that accumulated cruft over time. This large series cleans up the implementation quite a bit. * en/merge-recursive-cleanup: (26 commits) merge-recursive: fix the fix to the diff3 common ancestor label merge-recursive: fix the diff3 common ancestor label for virtual commits merge-recursive: alphabetize include list merge-recursive: add sanity checks for relevant merge_options merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_* merge-recursive: split internal fields into a separate struct merge-recursive: avoid losing output and leaking memory holding that output merge-recursive: comment and reorder the merge_options fields merge-recursive: consolidate unnecessary fields in merge_options merge-recursive: move some definitions around to clean up the header merge-recursive: rename merge_options argument to opt in header merge-recursive: rename 'mrtree' to 'result_tree', for clarity merge-recursive: use common name for ancestors/common/base_list merge-recursive: fix some overly long lines cache-tree: share code between functions writing an index as a tree merge-recursive: don't force external callers to do our logging merge-recursive: remove useless parameter in merge_trees() merge-recursive: exit early if index != head Ensure index matches head before invoking merge machinery, round N merge-recursive: remove another implicit dependency on the_repository ...
2019-10-08merge-recursive: fix the fix to the diff3 common ancestor labelElijah Newren
In commit 8e4ec337 ("merge-recursive: fix the diff3 common ancestor label for virtual commits", 2019-10-01), which was a fix to commit 743474cbfa8b ("merge-recursive: provide a better label for diff3 common ancestor", 2019-08-17), the label for the common ancestor was changed from always being "merged common ancestors" to instead be based on the number of merge bases and whether the merge base was a real commit or a virtual one: >=2: "merged common ancestors" 1, via merge_recursive_generic: "constructed merge base" 1, otherwise: <abbreviated commit hash> 0: "<empty tree>" The handling for "constructed merge base" worked by allowing opt->ancestor to be set in merge_recursive_generic(), so we paid attention to the setting of that variable in merge_recursive_internal(). Now, for the outer merge, the code flow was simply the following: ancestor_name = "merged merge bases" loop over merge_bases: merge_recursive_internal() The first merge base not needing recursion would determine its own ancestor_name however necessary and thus run ancestor_name = $SOMETHING empty loop over merge_bases... opt->ancestor = ancestor_name merge_trees_internal() Now, the next set of merge_bases that would need to be merged after this particular merge had completed would note that opt->ancestor has been set to something (to a local ancestor_name variable that has since been popped off the stack), and thus it would run: ... else if (opt->ancestor) { ancestor_name = opt->ancestor; /* OOPS! */ loop over merge_bases: merge_recursive_internal() opt->ancestor = ancestor_name merge_trees_internal() This resulted in garbage strings being printed for the virtual merge bases, which was visible in git.git by just merging commit b744c3af07 into commit 6d8cb22a4f. There are two ways to fix this: set opt->ancestor to NULL after using it to avoid re-use, or add a !opt->priv->call_depth check to the if block for using a pre-defined opt->ancestor. Apply both fixes. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: remove type arg from hashmap_{get,put,remove}_entryEric Wong
Since these macros already take a `keyvar' pointer of a known type, we can rely on OFFSETOF_VAR to get the correct offset without relying on non-portable `__typeof__' and `offsetof'. Argument order is also rearranged, so `keyvar' and `member' are sequential as they are used as: `keyvar->member' Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07OFFSETOF_VAR macro to simplify hashmap iteratorsEric Wong
While we cannot rely on a `__typeof__' operator being portable to use with `offsetof'; we can calculate the pointer offset using an existing pointer and the address of a member using pointer arithmetic for compilers without `__typeof__'. This allows us to simplify usage of hashmap iterator macros by not having to specify a type when a pointer of that type is already given. In the future, list iterator macros (e.g. list_for_each_entry) may also be implemented using OFFSETOF_VAR to save hackers the trouble of using container_of/list_entry macros and without relying on non-portable `__typeof__'. v3: use `__typeof__' to avoid clang warnings Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: introduce hashmap_free_entriesEric Wong
`hashmap_free_entries' behaves like `container_of' and passes the offset of the hashmap_entry struct to the internal `hashmap_free_' function, allowing the function to free any struct pointer regardless of where the hashmap_entry field is located. `hashmap_free' no longer takes any arguments aside from the hashmap itself. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap: use *_entry APIs for iterationEric Wong
Inspired by list_for_each_entry in the Linux kernel. Once again, these are somewhat compromised usability-wise by compilers lacking __typeof__ support. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_cmp_fn takes hashmap_entry paramsEric Wong
Another step in eliminating the requirement of hashmap_entry being the first member of a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_get{,_from_hash} return "struct hashmap_entry *"Eric Wong
Update callers to use hashmap_get_entry, hashmap_get_entry_from_hash or container_of as appropriate. This is another step towards eliminating the requirement of hashmap_entry being the first field in a struct. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_put takes "struct hashmap_entry *"Eric Wong
This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_remove takes "const struct hashmap_entry *"Eric Wong
This is less error-prone than "const void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_get takes "const struct hashmap_entry *"Eric Wong
This is less error-prone than "const void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_add takes "struct hashmap_entry *"Eric Wong
This is less error-prone than "void *" as the compiler now detects invalid types being passed. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-07hashmap_entry_init takes "struct hashmap_entry *"Eric Wong
C compilers do type checking to make life easier for us. So rely on that and update all hashmap_entry_init callers to take "struct hashmap_entry *" to avoid future bugs while improving safety and readability. Signed-off-by: Eric Wong <e@80x24.org> Reviewed-by: Derrick Stolee <stolee@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-02merge-recursive: fix the diff3 common ancestor label for virtual commitsElijah Newren
In commit 743474cbfa8b ("merge-recursive: provide a better label for diff3 common ancestor", 2019-08-17), the label for the common ancestor was changed from always being "merged common ancestors" to instead be based on the number of merge bases: >=2: "merged common ancestors" 1: <abbreviated commit hash> 0: "<empty tree>" Unfortunately, this did not take into account that when we have a single merge base, that merge base could be fake or constructed. In such cases, this resulted in a label of "00000000". Of course, the previous label of "merged common ancestors" was also misleading for this case. Since we have an API that is explicitly about creating fake merge base commits in merge_recursive_generic(), we should provide a better label when using that API with one merge base. So, when merge_recursive_generic() is called with one merge base, set the label to: "constructed merge base" Note that callers of merge_recursive_generic() include the builtin commands git-am (in combination with git apply --build-fake-ancestor), git-merge-recursive, and git-stash. Helped-by: Jeff King <peff@peff.net> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-09-20merge-recursive: symlink's descendants not in wayJonathan Tan
When the working tree has: - bar (directory) - bar/file (file) - foo (symlink to .) (note that lstat() for "foo/bar" would tell us that it is a directory) and the user merges a commit that deletes the foo symlink and instead contains: - bar (directory, as above) - bar/file (file, as above) - foo (directory) - foo/bar (file) the merge should happen without requiring user intervention. However, this does not happen. This is because dir_in_way(), when checking the working tree, thinks that "foo/bar" is a directory. But a symlink should be treated much the same as a file: since dir_in_way() is only checking to see if there is a directory in the way, we don't want symlinks in leading paths to sometimes cause dir_in_way() to return true. Teach dir_in_way() to also check for symlinks in leading paths before reporting whether a directory is in the way. Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: alphabetize include listElijah Newren
Other than cache.h which needs to appear first, and merge-recursive.h which I want to be second so that we are more likely to notice if merge-recursive.h has any missing includes, the rest of the list is long and easier to look through if it's alphabetical. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: add sanity checks for relevant merge_optionsElijah Newren
There are lots of options that callers can set, yet most have a limited range of valid values, some options are meant for output (e.g. opt->obuf, which is expected to start empty), and callers are expected to not set opt->priv. Add several sanity checks to ensure callers provide sane values. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: rename MERGE_RECURSIVE_* to MERGE_VARIANT_*Elijah Newren
I want to implement the same outward facing API as found within merge-recursive.h in a different merge strategy. However, that makes names like MERGE_RECURSIVE_{NORMAL,OURS,THEIRS} look a little funny; rename to MERGE_VARIANT_{NORMAL,OURS,THEIRS}. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: split internal fields into a separate structElijah Newren
merge_options has several internal fields that should not be set or read by external callers. This just complicates the API. Move them into an opaque merge_options_internal struct that is defined only in merge-recursive.c and keep these out of merge-recursive.h. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: avoid losing output and leaking memory holding that outputElijah Newren
If opt->buffer_output is less than 2, then merge_trees(), merge_recursive(), and merge_recursive_generic() are all supposed to flush the opt->obuf output buffer to stdout and release any memory it holds. merge_trees() did not do this. Move the logic that handles this for merge_recursive_internal() to merge_finalize() so that all three methods handle this requirement. Note that this bug didn't cause any problems currently, because there are only two callers of merge_trees() right now (a git grep for 'merge_trees(' is misleading because builtin/merge-tree.c also defines a 'merge_tree' function that is unrelated), and only one of those is called with buffer_output less than 2 (builtin/checkout.c), but it set opt->verbosity to 0, for which there is only currently one non-error message that would be shown: "Already up to date!". However, that one message can only occur when the merge is utterly trivial (the merge base tree exactly matches the merge tree), and builtin/checkout.c already attempts a trivial merge via unpack_trees() before falling back to merge_trees(). Also, if opt->buffer_output is 2, then the caller is responsible to handle showing any output in opt->obuf and for free'ing it. This requirement might be easy to overlook, so add a comment to merge-recursive.h pointing it out. (There are currently two callers that set buffer_output to 2, both in sequencer.c, and both of which handle this correctly.) Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: comment and reorder the merge_options fieldsElijah Newren
The merge_options struct had lots of fields, making it a little imposing, but the options naturally fall into multiple different groups. Grouping similar options and adding a comment or two makes it easier to read, easier for new folks to figure out which options are related, and thus easier for them to find the options they need. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: consolidate unnecessary fields in merge_optionsElijah Newren
We provided users with the ability to state whether they wanted rename detection, and to put a limit on how much CPU would be spent. Both of these fields had multiple configuration parameters for setting them, with one being a fallback and the other being an override. However, instead of implementing the logic for how to combine the multiple source locations into the appropriate setting at config loading time, we loaded and tracked both values and then made the code combine them every time it wanted to check the overall value. This had a few minor drawbacks: * it seems more complicated than necessary * it runs the risk of people using the independent settings in the future and breaking the intent of how the options are used together * it makes merge_options more complicated than necessary for other potential users of the API Fix these problems by moving the logic for combining the pairs of options into a single value; make it apply at time-of-config-loading instead of each-time-of-use. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-08-19merge-recursive: move some definitions around to clean up the headerElijah Newren
No substantive code changes (view this with diff --color-moved), but a few small code cleanups: * Move structs and an inline function only used by merge-recursive.c into merge-recursive.c * Re-order function declarations to be more logical * Add or fix some explanatory comments Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>