summaryrefslogtreecommitdiff
path: root/merge-recursive.c
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'kw/merge-recursive-cleanup'Junio C Hamano2017-09-191-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
| * merge-recursive: change current file dir string_lists to hashmapkw/merge-recursive-cleanupKevin Willford2017-09-081-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>
| * merge-recursive: remove return value from get_files_dirsKevin Willford2017-09-061-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>
| * merge-recursive: fix memory leakKevin Willford2017-09-061-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>
* | treewide: correct several "up-to-date" to "up to date"ma/up-to-dateMartin Ågren2017-08-231-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>
* Merge branch 'sb/merge-recursive-code-cleanup'Junio C Hamano2017-07-061-3/+3
|\ | | | | | | | | | | | | Code clean-up. * sb/merge-recursive-code-cleanup: merge-recursive: use DIFF_XDL_SET macro
| * merge-recursive: use DIFF_XDL_SET macrosb/merge-recursive-code-cleanupStefan Beller2017-06-301-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>
* | Merge branch 'bw/config-h'Junio C Hamano2017-06-241-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
| * | config: don't include config.h by defaultBrandon Williams2017-06-151-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>
* | Merge branch 'bw/ls-files-sans-the-index'Junio C Hamano2017-06-241-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
| * | convert: convert renormalize_buffer to take an indexBrandon Williams2017-06-131-2/+2
| | | | | | | | | | | | | | | Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | diff-tree: convert diff_tree_sha1 to struct object_idBrandon Williams2017-06-051-1/+1
|/ / | | | | | | | | Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | object: convert parse_object* to take struct object_idbrian m. carlson2017-05-081-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>
* | Convert lookup_tree to struct object_idbrian m. carlson2017-05-081-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>
* | submodule: convert merge_submodule to use struct object_idbrian m. carlson2017-05-081-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>
* | Convert struct cache_tree to use struct object_idbrian m. carlson2017-05-021-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>
* Merge branch 'mm/merge-rename-delete-message'Junio C Hamano2017-02-271-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
| * merge-recursive: make "CONFLICT (rename/delete)" message show both pathsmm/merge-rename-delete-messageMatt McCutchen2017-01-301-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>
| * Merge branch 'nd/qsort-in-merge-recursive' into maintJunio C Hamano2017-01-171-9/+7
| |\ | | | | | | | | | | | | | | | | | | Code simplification. * nd/qsort-in-merge-recursive: merge-recursive.c: use string_list_sort instead of qsort
| * \ Merge branch 'jc/renormalize-merge-kill-safer-crlf' into maintJunio C Hamano2017-01-171-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
* | | | use SWAP macroRené Scharfe2017-01-301-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>
* | | | Merge branch 'jc/lock-report-on-error'Junio C Hamano2016-12-191-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
| * | | | hold_locked_index(): align error handling with hold_lockfile_for_update()Junio C Hamano2016-12-071-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.
* | | | | Merge branch 'jc/renormalize-merge-kill-safer-crlf'Junio C Hamano2016-12-191-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
| * | | | merge-recursive: handle NULL in add_cacheinfo() correctlyJohannes Schindelin2016-11-281-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>
* | | | | Merge branch 'nd/qsort-in-merge-recursive'Junio C Hamano2016-12-161-9/+7
|\ \ \ \ \ | |_|_|/ / |/| | | / | | |_|/ | |/| | | | | | | | | | Code simplification. * nd/qsort-in-merge-recursive: merge-recursive.c: use string_list_sort instead of qsort
| * | | merge-recursive.c: use string_list_sort instead of qsortnd/qsort-in-merge-recursiveNguyễn Thái Ngọc Duy2016-11-281-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>
* | | | submodules: allow empty working-tree dirs in merge/cherry-pickdt/empty-submodule-in-mergeDavid Turner2016-11-171-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>
* | | Merge branch 'rs/cocci'Junio C Hamano2016-10-171-3/+3
|\ \ \ | |/ / | | | | | | | | | | | | | | | | | | Code cleanup. * rs/cocci: use strbuf_add_unique_abbrev() for adding short hashes, part 3 remove unnecessary NULL check before free(3)
| * | use strbuf_add_unique_abbrev() for adding short hashes, part 3René Scharfe2016-10-101-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>
* | | Merge branch 'rs/cocci'Junio C Hamano2016-09-261-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()
| * | use strbuf_addstr() for adding constant strings to a strbuf, part 2René Scharfe2016-09-151-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>
* | | Convert read_mmblob to take struct object_id.brian m. carlson2016-09-071-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>
* | | cache: convert struct cache_entry to use struct object_idbrian m. carlson2016-09-071-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>
* | Merge branch 'rs/pull-signed-tag'Junio C Hamano2016-08-191-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()
| * | merge-recursive: fix verbose output for multiple base treesRené Scharfe2016-08-131-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>
* | | Merge branch 'rs/merge-recursive-string-list-init'Junio C Hamano2016-08-121-2/+1
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | A small code clean-up. * rs/merge-recursive-string-list-init: merge-recursive: use STRING_LIST_INIT_NODUP
| * | | merge-recursive: use STRING_LIST_INIT_NODUPrs/merge-recursive-string-list-initRené Scharfe2016-08-051-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>
* | | merge-recursive: flush output buffer even when erroring outjs/am-3-merge-recursive-directJohannes Schindelin2016-08-011-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>
* | | merge_trees(): ensure that the callers release output bufferJohannes Schindelin2016-08-011-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>
* | | merge-recursive: offer an option to retain the output in 'obuf'Johannes Schindelin2016-08-011-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>
* | | merge-recursive: write the commit title in one goJohannes Schindelin2016-08-011-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>
* | | merge-recursive: flush output buffer before printing error messagesJohannes Schindelin2016-08-011-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>
* | | merge-recursive: switch to returning errors instead of dyingJohannes Schindelin2016-07-261-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>
* | | merge-recursive: handle return values indicating errorsJohannes Schindelin2016-07-261-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>
* | | merge-recursive: allow write_tree_from_memory() to error outJohannes Schindelin2016-07-261-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>
* | | merge-recursive: avoid returning a wholesale structJohannes Schindelin2016-07-261-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>
* | | merge_recursive: abort properly upon errorsJohannes Schindelin2016-07-261-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>
* | | merge-recursive: clarify code in was_tracked()Johannes Schindelin2016-07-261-16/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | It can be puzzling to see that was_tracked() asks to get an index entry by name, but does not take a negative return value for an answer. The reason we have to do this is that cache_name_pos() only looks for entries in stage 0, even if nobody asked for any stage in particular. Let's rewrite the logic a little bit, to handle the easy case early: if cache_name_pos() returned a non-negative position, we know it is a match, and we do not even have to compare the name again (cache_name_pos() did that for us already). We can say right away: yes, this file was tracked. Only if there was no exact match do we need to look harder for any matching entry in stage 2. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | die(_("BUG")): avoid translating bug messagesJohannes Schindelin2016-07-261-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While working on the patch series that avoids die()ing in recursive merges, the issue came up that bug reports (i.e. die("BUG: ...") constructs) should never be translated, as the target audience is the Git developer community, not necessarily the current user, and hence a translated message would make it *harder* to address the problem. So let's stop translating the obvious ones. As it is really, really outside the purview of this patch series to see whether there are more die() statements that report bugs and are currently translated, that task is left for another day and patch. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>