summaryrefslogtreecommitdiff
path: root/merge-recursive.c
AgeCommit message (Collapse)AuthorFilesLines
2017-12-22merge-recursive: avoid incorporating uncommitted changes in a mergeLibravatar Elijah Newren1-0/+7
builtin/merge.c contains this important requirement for merge strategies: /* * At this point, we need a real merge. No matter what strategy * we use, it would operate on the index, possibly affecting the * working tree, and when resolved cleanly, have the desired * tree in the index -- this means that the index must be in * sync with the head commit. The strategies are responsible * to ensure this. */ merge-recursive does not do this check directly, instead it relies on unpack_trees() to do it. However, merge_trees() has a special check for the merge branch exactly matching the merge base; when it detects that situation, it returns early without calling unpack_trees(), because it knows that the HEAD commit already has the correct result. Unfortunately, it didn't check that the index matched HEAD, so after it returned, the outer logic ended up creating a merge commit that included something other than HEAD. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-15merge-recursive: handle addition of submodule on our side of historyLibravatar Elijah Newren1-2/+3
The code for a newly added path assumed that the path was a normal file, and thus checked for there being a directory still being in the way of the file. Note that since unpack_trees() does path-in-the-way checks already, the only way for there to be a directory in the way at this point in the code, is if there is some kind of D/F conflict in the merge. For a submodule addition on HEAD's side of history, the submodule would have already been present. This means that we do expect there to be a directory present but should not consider it to be "in the way"; instead, it's the expected submodule. So, when there's a submodule addition from HEAD's side, don't bother checking the working copy for a directory in the way. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-19Merge branch 'kw/merge-recursive-cleanup'Libravatar Junio C Hamano1-20/+56
A leakfix and code clean-up. * kw/merge-recursive-cleanup: merge-recursive: change current file dir string_lists to hashmap merge-recursive: remove return value from get_files_dirs merge-recursive: fix memory leak
2017-09-08merge-recursive: change current file dir string_lists to hashmapLibravatar Kevin Willford1-11/+45
The code was using two string_lists, one for the directories and one for the files. The code never checks the lists independently so we should be able to only use one list. The string_list also is a O(log n) for lookup and insertion. Switching this to use a hashmap will give O(1) which will save some time when there are millions of paths that will be checked. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-06merge-recursive: remove return value from get_files_dirsLibravatar Kevin Willford1-6/+2
The return value of the get_files_dirs call is never being used. Looking at the history of the file and it was originally only being used for debug output statements. Also when read_tree_recursive return value is non zero it is changed to zero. This leads me to believe that it doesn't matter if read_tree_recursive gets an error. Since the debug output has been removed and the caller isn't checking the return value there is no reason to keep calculating and returning a value. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-09-06merge-recursive: fix memory leakLibravatar Kevin Willford1-3/+9
In merge_trees if process_renames or process_entry returns less than zero, the method will just return and not free re_merge, re_head, or entries. This change cleans up the allocated variables before returning to the caller. Signed-off-by: Kevin Willford <kewillf@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-23treewide: correct several "up-to-date" to "up to date"Libravatar Martin Ågren1-1/+1
Follow the Oxford style, which says to use "up-to-date" before the noun, but "up to date" after it. Don't change plumbing (specifically send-pack.c, but transport.c (git push) also has the same string). This was produced by grepping for "up-to-date" and "up to date". It turned out we only had to edit in one direction, removing the hyphens. Fix a typo in Documentation/git-diff-index.txt while we're there. Reported-by: Jeffrey Manian <jeffrey.manian@gmail.com> Reported-by: STEVEN WHITE <stevencharleswhitevoices@gmail.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-06Merge branch 'sb/merge-recursive-code-cleanup'Libravatar Junio C Hamano1-3/+3
Code clean-up. * sb/merge-recursive-code-cleanup: merge-recursive: use DIFF_XDL_SET macro
2017-06-30merge-recursive: use DIFF_XDL_SET macroLibravatar Stefan Beller1-3/+3
Instead of implementing this on our own, just use a convenience macro. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-24Merge branch 'bw/config-h'Libravatar Junio C Hamano1-0/+1
Fix configuration codepath to pay proper attention to commondir that is used in multi-worktree situation, and isolate config API into its own header file. * bw/config-h: config: don't implicitly use gitdir or commondir config: respect commondir setup: teach discover_git_directory to respect the commondir config: don't include config.h by default config: remove git_config_iter config: create config.h
2017-06-24Merge branch 'bw/ls-files-sans-the-index'Libravatar Junio C Hamano1-2/+2
Code clean-up. * bw/ls-files-sans-the-index: ls-files: factor out tag calculation ls-files: factor out debug info into a function ls-files: convert show_files to take an index ls-files: convert show_ce_entry to take an index ls-files: convert prune_cache to take an index ls-files: convert ce_excluded to take an index ls-files: convert show_ru_info to take an index ls-files: convert show_other_files to take an index ls-files: convert show_killed_files to take an index ls-files: convert write_eolinfo to take an index ls-files: convert overlay_tree_on_cache to take an index tree: convert read_tree to take an index parameter convert: convert renormalize_buffer to take an index convert: convert convert_to_git to take an index convert: convert convert_to_git_filter_fd to take an index convert: convert crlf_to_git to take an index convert: convert get_cached_convert_stats_ascii to take an index
2017-06-15config: don't include config.h by defaultLibravatar Brandon Williams1-0/+1
Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-13convert: convert renormalize_buffer to take an indexLibravatar Brandon Williams1-2/+2
Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-05diff-tree: convert diff_tree_sha1 to struct object_idLibravatar Brandon Williams1-1/+1
Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08object: convert parse_object* to take struct object_idLibravatar brian m. carlson1-1/+1
Make parse_object, parse_object_or_die, and parse_object_buffer take a pointer to struct object_id. Remove the temporary variables inserted earlier, since they are no longer necessary. Transform all of the callers using the following semantic patch: @@ expression E1; @@ - parse_object(E1.hash) + parse_object(&E1) @@ expression E1; @@ - parse_object(E1->hash) + parse_object(E1) @@ expression E1, E2; @@ - parse_object_or_die(E1.hash, E2) + parse_object_or_die(&E1, E2) @@ expression E1, E2; @@ - parse_object_or_die(E1->hash, E2) + parse_object_or_die(E1, E2) @@ expression E1, E2, E3, E4, E5; @@ - parse_object_buffer(E1.hash, E2, E3, E4, E5) + parse_object_buffer(&E1, E2, E3, E4, E5) @@ expression E1, E2, E3, E4, E5; @@ - parse_object_buffer(E1->hash, E2, E3, E4, E5) + parse_object_buffer(E1, E2, E3, E4, E5) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08Convert lookup_tree to struct object_idLibravatar brian m. carlson1-3/+3
Convert the lookup_tree function to take a pointer to struct object_id. The commit was created with manual changes to tree.c, tree.h, and object.c, plus the following semantic patch: @@ @@ - lookup_tree(EMPTY_TREE_SHA1_BIN) + lookup_tree(&empty_tree_oid) @@ expression E1; @@ - lookup_tree(E1.hash) + lookup_tree(&E1) @@ expression E1; @@ - lookup_tree(E1->hash) + lookup_tree(E1) Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-08submodule: convert merge_submodule to use struct object_idLibravatar brian m. carlson1-4/+4
This is a caller of lookup_commit_reference, which we will convert later. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-02Convert struct cache_tree to use struct object_idLibravatar brian m. carlson1-1/+1
Convert the sha1 member of struct cache_tree to struct object_id by changing the definition and applying the following semantic patch, plus the standard object_id transforms: @@ struct cache_tree E1; @@ - E1.sha1 + E1.oid.hash @@ struct cache_tree *E1; @@ - E1->sha1 + E1->oid.hash Fix up one reference to active_cache_tree which was not automatically caught by Coccinelle. These changes are prerequisites for converting parse_object. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-27Merge branch 'mm/merge-rename-delete-message'Libravatar Junio C Hamano1-54/+63
When "git merge" detects a path that is renamed in one history while the other history deleted (or modified) it, it now reports both paths to help the user understand what is going on in the two histories being merged. * mm/merge-rename-delete-message: merge-recursive: make "CONFLICT (rename/delete)" message show both paths
2017-01-30use SWAP macroLibravatar René Scharfe1-4/+1
Apply the semantic patch swap.cocci to convert hand-rolled swaps to use the macro SWAP. The resulting code is shorter and easier to read, the object code is effectively unchanged. The patch for object.c had to be hand-edited in order to preserve the comment before the change; Coccinelle tried to eat it for some reason. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-30merge-recursive: make "CONFLICT (rename/delete)" message show both pathsLibravatar Matt McCutchen1-54/+63
The current message printed by "git merge-recursive" for a rename/delete conflict is like this: CONFLICT (rename/delete): new-path deleted in HEAD and renamed in other-branch. Version other-branch of new-path left in tree. To be more helpful, the message should show both paths of the rename and state that the deletion occurred at the old path, not the new path. So change the message to the following format: CONFLICT (rename/delete): old-path deleted in HEAD and renamed to new-path in other-branch. Version other-branch of new-path left in tree. Since this doubles the number of cases in handle_change_delete (modify vs. rename), refactor the code to halve the number of cases again by merging the cases where o->branch1 has the change and o->branch2 has the delete with the cases that are the other way around. Also add a simple test of the new conflict message. Signed-off-by: Matt McCutchen <matt@mattmccutchen.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-17Merge branch 'nd/qsort-in-merge-recursive' into maintLibravatar Junio C Hamano1-9/+7
Code simplification. * nd/qsort-in-merge-recursive: merge-recursive.c: use string_list_sort instead of qsort
2017-01-17Merge branch 'jc/renormalize-merge-kill-safer-crlf' into maintLibravatar Junio C Hamano1-0/+2
Fix a corner case in merge-recursive regression that crept in during 2.10 development cycle. * jc/renormalize-merge-kill-safer-crlf: convert: git cherry-pick -Xrenormalize did not work merge-recursive: handle NULL in add_cacheinfo() correctly cherry-pick: demonstrate a segmentation fault
2016-12-19Merge branch 'jc/lock-report-on-error'Libravatar Junio C Hamano1-1/+1
Git 2.11 had a minor regression in "merge --ff-only" that competed with another process that simultanously attempted to update the index. We used to explain what went wrong with an error message, but the new code silently failed. The error message has been resurrected. * jc/lock-report-on-error: lockfile: LOCK_REPORT_ON_ERROR hold_locked_index(): align error handling with hold_lockfile_for_update() wt-status: implement opportunisitc index update correctly
2016-12-19Merge branch 'jc/renormalize-merge-kill-safer-crlf'Libravatar Junio C Hamano1-0/+2
Fix a corner case in merge-recursive regression that crept in during 2.10 development cycle. * jc/renormalize-merge-kill-safer-crlf: convert: git cherry-pick -Xrenormalize did not work merge-recursive: handle NULL in add_cacheinfo() correctly cherry-pick: demonstrate a segmentation fault
2016-12-16Merge branch 'nd/qsort-in-merge-recursive'Libravatar Junio C Hamano1-9/+7
Code simplification. * nd/qsort-in-merge-recursive: merge-recursive.c: use string_list_sort instead of qsort
2016-12-07hold_locked_index(): align error handling with hold_lockfile_for_update()Libravatar Junio C Hamano1-1/+1
Callers of the hold_locked_index() function pass 0 when they want to prepare to write a new version of the index file without wishing to die or emit an error message when the request fails (e.g. somebody else already held the lock), and pass 1 when they want the call to die upon failure. This option is called LOCK_DIE_ON_ERROR by the underlying lockfile API, and the hold_locked_index() function translates the paramter to LOCK_DIE_ON_ERROR when calling the hold_lock_file_for_update(). Replace these hardcoded '1' with LOCK_DIE_ON_ERROR and stop translating. Callers other than the ones that are replaced with this change pass '0' to the function; no behaviour change is intended with this patch. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Among the callers of hold_locked_index() that passes 0: - diff.c::refresh_index_quietly() at the end of "git diff" is an opportunistic update; it leaks the lockfile structure but it is just before the program exits and nobody should care. - builtin/describe.c::cmd_describe(), builtin/commit.c::cmd_status(), sequencer.c::read_and_refresh_cache() are all opportunistic updates and they are OK. - builtin/update-index.c::cmd_update_index() takes a lock upfront but we may end up not needing to update the index (i.e. the entries may be fully up-to-date), in which case we do not need to issue an error upon failure to acquire the lock. We do diagnose and die if we indeed need to update, so it is OK. - wt-status.c::require_clean_work_tree() IS BUGGY. It asks silence, does not check the returned value. Compare with callsites like cmd_describe() and cmd_status() to notice that it is wrong to call update_index_if_able() unconditionally.
2016-11-28merge-recursive.c: use string_list_sort instead of qsortLibravatar Nguyễn Thái Ngọc Duy1-9/+7
Merge-recursive sorts a string list using a raw qsort(), where it feeds the "items" from one struct but the "nr" and size fields from another struct. This isn't a bug because one list is a copy of the other, but it's unnecessarily confusing (and also caused our recent QSORT() cleanups via coccinelle to miss this call site). Let's use string_list_sort() instead, which is more concise and harder to get wrong. Note that we need to adjust our comparison function, which gets fed only the strings now, not the string_list_items. That's OK because we don't use the "util" field as part of our sort. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-28merge-recursive: handle NULL in add_cacheinfo() correctlyLibravatar Johannes Schindelin1-0/+2
1335d76e45 ("merge: avoid "safer crlf" during recording of merge results", 2016-07-08) tried to split make_cache_entry() call made with CE_MATCH_REFRESH into a call to make_cache_entry() without one, followed by a call to add_cache_entry(), refresh_cache() and another add_cache_entry() as needed. However the conversion was botched in that it forgot that refresh_cache() can return NULL, which was handled correctly in make_cache_entry() but in the updated code. This fixes https://github.com/git-for-windows/git/issues/952 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-17submodules: allow empty working-tree dirs in merge/cherry-pickLibravatar David Turner1-6/+15
When a submodule is being merged or cherry-picked into a working tree that already contains a corresponding empty directory, do not record a conflict. One situation where this bug appears is: - Commit 1 adds a submodule - Commit 2 removes that submodule and re-adds it into a subdirectory (sub1 to sub1/sub1). - Commit 3 adds an unrelated file. Now the user checks out commit 1 (first deinitializing the submodule), and attempts to cherry-pick commit 3. Previously, this would fail, because the incoming submodule sub1/sub1 would falsely conflict with the empty sub1 directory. This patch ignores the empty sub1 directory, fixing the bug. We only ignore the empty directory if the object being emplaced is a submodule, which expects an empty directory. Signed-off-by: David Turner <dturner@twosigma.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-10-17Merge branch 'rs/cocci'Libravatar Junio C Hamano1-3/+3
Code cleanup. * rs/cocci: use strbuf_add_unique_abbrev() for adding short hashes, part 3 remove unnecessary NULL check before free(3)
2016-10-10use strbuf_add_unique_abbrev() for adding short hashes, part 3Libravatar René Scharfe1-3/+3
Call strbuf_add_unique_abbrev() to add abbreviated hashes to strbufs instead of taking detours through find_unique_abbrev() and its static buffer. This is shorter in most cases and a bit more efficient. The changes here are not easily handled by a semantic patch because they involve removing temporary variables and deconstructing format strings for strbuf_addf(). Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-26Merge branch 'rs/cocci'Libravatar Junio C Hamano1-1/+1
Code cleanup. * rs/cocci: use strbuf_addstr() for adding constant strings to a strbuf, part 2 add coccicheck make target contrib/coccinelle: fix semantic patch for oid_to_hex_r()
2016-09-15use strbuf_addstr() for adding constant strings to a strbuf, part 2Libravatar René Scharfe1-1/+1
Replace uses of strbuf_addf() for adding strings with more lightweight strbuf_addstr() calls. This makes the intent clearer and avoids potential issues with printf format specifiers. 02962d36845b89145cd69f8bc65e015d78ae3434 already converted six cases, this patch covers eleven more. A semantic patch for Coccinelle is included for easier checking for new cases that might be introduced in the future. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-07Convert read_mmblob to take struct object_id.Libravatar brian m. carlson1-3/+3
Since all of its callers have been updated, convert read_mmblob to take a pointer to struct object_id. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-07cache: convert struct cache_entry to use struct object_idLibravatar brian m. carlson1-1/+1
Convert struct cache_entry to use struct object_id by applying the following semantic patch and the object_id transforms from contrib, plus the actual change to the struct: @@ struct cache_entry E1; @@ - E1.sha1 + E1.oid.hash @@ struct cache_entry *E1; @@ - E1->sha1 + E1->oid.hash Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-19Merge branch 'rs/pull-signed-tag'Libravatar Junio C Hamano1-4/+1
When "git merge-recursive" works on history with many criss-cross merges in "verbose" mode, the names the command assigns to the virtual merge bases could have overwritten each other by unintended reuse of the same piece of memory. * rs/pull-signed-tag: commit: use FLEX_ARRAY in struct merge_remote_desc merge-recursive: fix verbose output for multiple base trees commit: factor out set_merge_remote_desc() commit: use xstrdup() in get_merge_parent()
2016-08-13merge-recursive: fix verbose output for multiple base treesLibravatar René Scharfe1-4/+1
One of the indirect callers of make_virtual_commit() passes the result of oid_to_hex() as the name, i.e. a pointer to a static buffer. Since the function uses that string pointer directly in building a struct merge_remote_desc, multiple entries can end up sharing the same name inadvertently. Fix that by calling set_merge_remote_desc(), which creates a copy of the string, instead of building the struct by hand. Signed-off-by: Rene Scharfe <l.s.r@web.de> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-12Merge branch 'rs/merge-recursive-string-list-init'Libravatar Junio C Hamano1-2/+1
A small code clean-up. * rs/merge-recursive-string-list-init: merge-recursive: use STRING_LIST_INIT_NODUP
2016-08-05merge-recursive: use STRING_LIST_INIT_NODUPLibravatar René Scharfe1-2/+1
Initialize a string_list right when it's defined. That's shorter, saves a function call and makes it more obvious that we're using the NODUP variant here. Signed-off-by: Rene Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-01merge-recursive: flush output buffer even when erroring outLibravatar Johannes Schindelin1-1/+3
Ever since 66a155b (Enable output buffering in merge-recursive., 2007-01-14), we had a problem: When the merge failed in a fatal way, all regular output was swallowed because we called die() and did not get a chance to drain the output buffers. To fix this, several modifications were necessary: - we needed to stop die()ing, to give callers a chance to do something when an error occurred (in this case, flush the output buffers), - we needed to delay printing the error message so that the caller can print the buffered output before that, and - we needed to make sure that the output buffers are flushed even when the return value indicates an error. The first two changes were introduced through earlier commits in this patch series, and this commit addresses the third one. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-01merge_trees(): ensure that the callers release output bufferLibravatar Johannes Schindelin1-0/+2
The recursive merge machinery accumulates its output in an output buffer, to be flushed at the end of merge_recursive(). At this point, we forgot to release the output buffer. When calling merge_trees() (i.e. the non-recursive part of the recursive merge) directly, the output buffer is never flushed because the caller may be merge_recursive() which wants to flush the output itself. For the same reason, merge_trees() cannot release the output buffer: it may still be needed. Forgetting to release the output buffer did not matter much when running git-checkout, or git-merge-recursive, because we exited after the operation anyway. Ever since cherry-pick learned to pick a commit range, however, this memory leak had the potential of becoming a problem. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-01merge-recursive: offer an option to retain the output in 'obuf'Libravatar Johannes Schindelin1-4/+13
Since 66a155b (Enable output buffering in merge-recursive., 2007-01-14), we already accumulate the output in a buffer. The idea was to avoid interfering with the progress output that goes to stderr, which is unbuffered, when we write to stdout, which is buffered. We extend that buffering to allow the caller to handle the output (possibly suppressing it). This will help us when extending the sequencer to do rebase -i's brunt work: it does not want the picks to print anything by default but instead determine itself whether to print the output or not. Note that we also redirect the error messages into the output buffer when the caller asked not to flush the output buffer, for two reasons: 1) to retain the correct output order, and 2) to allow the caller to suppress *all* output. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-01merge-recursive: write the commit title in one goLibravatar Johannes Schindelin1-8/+9
In 66a155b (Enable output buffering in merge-recursive., 2007-01-14), we changed the code such that it prints the output in one go, to avoid interfering with the progress output. Let's make sure that the same holds true when outputting the commit title: previously, we used several printf() statements to stdout and assumed that stdout's buffer is large enough to hold the entire commit title. Apart from making that speculation unnecessary, we change the code to add the message to the output buffer before flushing for another reason: the next commit will introduce a new level of output buffering, where the caller can request the output not to be flushed, but to be retained for further processing. This latter feature will be needed when teaching the sequencer to do rebase -i's brunt work: it wants to control the output of the cherry-picks (i.e. recursive merges). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-01merge-recursive: flush output buffer before printing error messagesLibravatar Johannes Schindelin1-48/+68
The data structure passed to the recursive merge machinery has a feature where the caller can ask for the output to be buffered into a strbuf, by setting the field 'buffer_output'. Previously, we died without flushing, losing accumulated output. With this patch, we show the output first, and only then print the error message. Currently, the only user of that buffering is merge_recursive() itself, to avoid the progress output to interfere. In the next patches, we will introduce a new buffer_output mode that forces merge_recursive() to retain the output buffer for further processing by the caller. If the caller asked for that, we will then also write the error messages into the output buffer. This is necessary to give the caller more control not only how to react in case of errors but also control how/if to display the error messages. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-26merge-recursive: switch to returning errors instead of dyingLibravatar Johannes Schindelin1-27/+35
The recursive merge machinery is supposed to be a library function, i.e. it should return an error when it fails. Originally the functions were part of the builtin "merge-recursive", though, where it was simpler to call die() and be done with error handling. The existing callers were already prepared to detect negative return values to indicate errors and to behave as previously: exit with code 128 (which is the same thing that die() does, after printing the message). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-26merge-recursive: handle return values indicating errorsLibravatar Johannes Schindelin1-102/+150
We are about to libify the recursive merge machinery, where we only die() in case of a bug or memory contention. To that end, we must heed negative return values as indicating errors. This requires our functions to be careful to pass through error conditions in call chains, and for quite a few functions this means that they have to return values to begin with. The next step will be to convert the places where we currently die() to return negative values (read: -1) instead. Note that we ignore errors reported by make_room_for_path(), consistent with the previous behavior (update_file_flags() used the return value of make_room_for_path() only to indicate an early return, but not a fatal error): if the error is really a fatal error, we will notice later; If not, it was not that serious a problem to begin with. (Witnesses in favor of this reasoning are t4151-am-abort and t7610-mergetool, which would start failing if we stopped on errors reported by make_room_for_path()). Also note: while this patch makes the code slightly less readable in update_file_flags() (we introduce a new "goto free_buf;" instead of an explicit "free(buf); return;"), it is a preparatory change for the next patch where we will convert all of the die() calls in the same function to go through the free_buf return path instead. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-26merge-recursive: allow write_tree_from_memory() to error outLibravatar Johannes Schindelin1-2/+2
It is possible that a tree cannot be written (think: disk full). We will want to give the caller a chance to clean up instead of letting the program die() in such a case. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-26merge-recursive: avoid returning a wholesale structLibravatar Johannes Schindelin1-50/+56
It is technically allowed, as per C89, for functions' return type to be complete structs (i.e. *not* just pointers to structs). However, it was just an oversight of this developer when converting Python code to C code in 6d297f8 (Status update on merge-recursive in C, 2006-07-08) which introduced such a return type. Besides, by converting this construct to pass in the struct, we can now start returning a value that can indicate errors in future patches. This will help the current effort to libify merge-recursive.c. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-26merge_recursive: abort properly upon errorsLibravatar Johannes Schindelin1-5/+12
There are a couple of places where return values never indicated errors before, as we simply died instead of returning. But now negative return values mean that there was an error and we have to abort the operation. Let's do exactly that. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>