summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2019-12-09format-patch: move git_config() before repo_init_revisions()Denton Liu1-4/+7
In 13cdf78094 (format-patch: teach format.notes config option, 2019-05-16), the order in which git_config() and repo_init_revisions() were swapped so that `rev.notes_opt` would be initialized before git_config() was called. This is problematic, however, as git_config() should generally be called before repo_init_revisions(). Break this circular dependency by creating `show_notes` and `notes_opt` which git_config() reads into. Then, copy these values over to `rev.show_notes` and `rev.notes_opt` after repo_init_revisions() is called. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09format-patch: use --notes behavior for format.notesDenton Liu2-12/+33
When we had multiple `format.notes` config values where we had `<ref1>`, `false`, `<ref2>` (in that order), then we would print out the notes for both `<ref1>` and `<ref2>`. This doesn't make sense, however, since we parse the config in a top-down manner and a `false` should be able to override previous configurations, just like how `--no-notes` will override previous `--notes`. Duplicate the logic that handles the `--[no-]notes[=]` option to `format.notes` for consistency. As a result, when parsing the config from top to bottom, `format.notes = true` will behave like `--notes`, `format.notes = <ref>` will behave like `--notes=<ref>` and `format.notes = false` will behave like `--no-notes`. This change isn't strictly backwards compatible but since it is an edge case where a sane user would not mix notes refs with `false` and this feature is relatively new (released only in v2.23.0), this change should be harmless. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09notes: extract logic into set_display_notes()Denton Liu3-16/+38
Instead of open coding the logic that tweaks the variables in `struct display_notes_opt` within handle_revision_opt(), abstract away the logic into set_display_notes() so that it can be reused. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09notes: create init_display_notes() helperDenton Liu3-1/+12
We currently open code the initialization for revs->notes_opt. Abstract this away into a helper function so that the logic can be reused in a future commit. This is slightly wasteful as we memset the struct twice but this is only run once so it shouldn't have any major effect. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09notes: rename to load_display_notes()Denton Liu3-6/+6
According to the function comment, init_display_notes() was supposed to "Load the notes machinery for displaying several notes trees." Rename this function to load_display_notes() so that its use is more accurately represented. This is done because, in a future commit, we will reuse the name init_display_notes(). Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09name-rev: cleanup name_ref()SZEDER Gábor1-15/+16
Earlier patches in this series moved a couple of conditions from the recursive name_rev() function into its caller name_ref(), for no other reason than to make eliminating the recursion a bit easier to follow. Since the previous patch name_rev() is not recursive anymore, so let's move all those conditions back into name_rev(). Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09name-rev: eliminate recursion in name_rev()SZEDER Gábor2-37/+63
The name_rev() function calls itself recursively for each interesting parent of the commit it got as parameter, and, consequently, it can segfault when processing a deep history if it exhausts the available stack space. E.g. running 'git name-rev --all' and 'git name-rev HEAD~100000' in the gcc, gecko-dev, llvm, and WebKit repositories results in segfaults on my machine ('ulimit -s' reports 8192kB of stack size limit), and nowadays the former segfaults in the Linux repo as well (it reached the necessasry depth sometime between v5.3-rc4 and -rc5). Eliminate the recursion by inserting the interesting parents into a LIFO 'prio_queue' [1] and iterating until the queue becomes empty. Note that the parent commits must be added in reverse order to the LIFO 'prio_queue', so their relative order is preserved during processing, i.e. the first parent should come out first from the queue, because otherwise performance greatly suffers on mergy histories [2]. The stacksize-limited test 'name-rev works in a deep repo' in 't6120-describe.sh' demonstrated this issue and expected failure. Now the recursion is gone, so flip it to expect success. Also gone are the dmesg entries logging the segfault of that segfaulting 'git name-rev' process on every execution of the test suite. Note that this slightly changes the order of lines in the output of 'git name-rev --all', usually swapping two lines every 35 lines in git.git or every 150 lines in linux.git. This shouldn't matter in practice, because the output has always been unordered anyway. This patch is best viewed with '--ignore-all-space'. [1] Early versions of this patch used a 'commit_list', resulting in ~15% performance penalty for 'git name-rev --all' in 'linux.git', presumably because of the memory allocation and release for each insertion and removal. Using a LIFO 'prio_queue' has basically no effect on performance. [2] We prefer shorter names, i.e. 'v0.1~234' is preferred over 'v0.1^2~5', meaning that usually following the first parent of a merge results in the best name for its ancestors. So when later we follow the remaining parent(s) of a merge, and reach an already named commit, then we usually find that we can't give that commit a better name, and thus we don't have to visit any of its ancestors again. OTOH, if we were to follow the Nth parent of the merge first, then the name of all its ancestors would include a corresponding '^N'. Those are not the best names for those commits, so when later we reach an already named commit following the first parent of that merge, then we would have to update the name of that commit and the names of all of its ancestors as well. Consequently, we would have to visit many commits several times, resulting in a significant slowdown. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09name-rev: use 'name->tip_name' instead of 'tip_name'SZEDER Gábor1-4/+7
Following the previous patches in this series we can get the value of 'name_rev()'s 'tip_name' parameter from the 'struct rev_name' associated with the commit as well. So let's use 'name->tip_name' instead, which makes the patch eliminating the recursion of name_rev() a bit easier to follow. Note that at this point we could drop the 'tip_name' parameter as well, but that parameter will be necessary later, after the recursion is eliminated. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09archive-zip: use enum for compression methodRené Scharfe1-9/+14
Add an enumeration to assign names to the magic values that determine the ZIP compression method to use. Use those names to improve code readability. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-10git gui: fix branch name encoding errorKazuhiro Kato2-0/+3
After "git checkout -b '漢字'" to create a branch with UTF-8 character in it, "git gui" shows the branch name incorrectly, as it forgets to turn the bytes read from the "git for-each-ref" and read from "HEAD" file into Unicode characters. Signed-off-by: Kazuhiro Kato <kato-k@ksysllc.co.jp> Signed-off-by: Pratyush Yadav <me@yadavpratyush.com>
2019-12-09l10n: minor case fix in 'git branch' '--unset-upstream' descriptionDimitriy Ryazantcev1-1/+1
Signed-off-by: Dimitriy Ryazantcev <dimitriy.ryazantcev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09t3404: fix indentationJohannes Schindelin1-5/+5
This test case was added in 66ae9a57b88 (t3404: rebase -i: demonstrate short SHA-1 collision, 2013-08-23), and it is not indented in the way we usually indent sub-shell code in our test cases these days. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09patch-id: use oid_to_hex() to print multiple object IDsRené Scharfe1-7/+2
flush_current_id() prints the hexadecimal representation of two object IDs. When the code was added in f97672225b (Add "git-patch-id" program to generate patch ID's., 2005-06-23), sha1_to_hex() had only a single internal static buffer, so the result of one invocation had to be stored in a local buffer. Since dcb3450fd8 (sha1_to_hex() usage cleanup, 2006-05-03) it rotates through four buffers, which allows to print up to four object IDs at the same time. 1a876a69af6 (patch-id: convert to use struct object_id, 2015-03-13) replaced sha1_to_hex() with oid_to_hex(), which has the same feature. Use it to simplify the code. Signed-off-by: René Scharfe <l.s.r@web.de> Acked-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-09commit: use strbuf_add() to add a length-limited stringRené Scharfe1-1/+1
This is shorter and simpler. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06The fourth batchJunio C Hamano1-0/+18
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06Merge branch 'dl/lore-is-the-archive'Junio C Hamano14-17/+18
Publicize lore.kernel.org mailing list archive and use URLs pointing into it to refer to notable messages in the documentation. * dl/lore-is-the-archive: doc: replace LKML link with lore.kernel.org RelNotes: replace Gmane with real Message-IDs doc: replace MARC links with lore.kernel.org
2019-12-06Merge branch 'jk/lore-is-the-archive'Junio C Hamano13-30/+30
Doc update for the mailing list archiving and nntp service. * jk/lore-is-the-archive: doc: replace public-inbox links with lore.kernel.org doc: recommend lore.kernel.org over public-inbox.org
2019-12-06Merge branch 'tg/perf-remove-stale-result'Junio C Hamano2-11/+5
PerfTest fix to avoid stale result mixed up with the latest round of test results. * tg/perf-remove-stale-result: perf-lib: use a single filename for all measurement types
2019-12-06Merge branch 'jk/send-pack-check-negative-with-quick'Junio C Hamano1-1/+3
Performance tweak on "git push" into a repository with many refs that point at objects we have never heard of. * jk/send-pack-check-negative-with-quick: send-pack: use OBJECT_INFO_QUICK to check negative objects
2019-12-06Merge branch 'hi/grep-do-not-return-void'Junio C Hamano1-1/+1
Code cleanup. * hi/grep-do-not-return-void: grep: don't return an expression from pcre2_free()
2019-12-06Merge branch 'rs/use-skip-prefix-more'Junio C Hamano5-22/+16
Code cleanup. * rs/use-skip-prefix-more: name-rev: use skip_prefix() instead of starts_with() push: use skip_prefix() instead of starts_with() shell: use skip_prefix() instead of starts_with() fmt-merge-msg: use skip_prefix() instead of starts_with() fetch: use skip_prefix() instead of starts_with()
2019-12-06Merge branch 'rs/simplify-prepare-cmd'Junio C Hamano1-2/+1
Code cleanup. * rs/simplify-prepare-cmd: run-command: use prepare_git_cmd() in prepare_cmd()
2019-12-06Merge branch 'rs/test-cleanup'Junio C Hamano10-23/+16
Test cleanup. * rs/test-cleanup: t7811: don't create unused file t9300: don't create unused file test: use test_must_be_empty F instead of test_cmp empty F test: use test_must_be_empty F instead of test -z $(cat F) t1400: use test_must_be_empty t1410: use test_line_count t1512: use test_line_count
2019-12-06Merge branch 'sg/assume-no-todo-update-in-cherry-pick'Junio C Hamano2-1/+31
While running "revert" or "cherry-pick --edit" for multiple commits, a recent regression incorrectly detected "nothing to commit, working tree clean", instead of replaying the commits, which has been corrected. * sg/assume-no-todo-update-in-cherry-pick: sequencer: don't re-read todo for revert and cherry-pick
2019-12-06Merge branch 'sg/osx-force-gcc-9'Junio C Hamano2-4/+5
TravisCI update. * sg/osx-force-gcc-9: ci: build Git with GCC 9 in the 'osx-gcc' build job
2019-12-06name-rev: drop name_rev()'s 'generation' and 'distance' parametersSZEDER Gábor1-12/+13
Following the previous patches in this series we can get the values of name_rev()'s 'generation' and 'distance' parameters from the 'stuct rev_name' associated with the commit as well. Let's simplify the function's signature and remove these two unnecessary parameters. Note that at this point we could do the same with the 'tip_name', 'taggerdate' and 'from_tag' parameters as well, but those parameters will be necessary later, after the recursion is eliminated. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: restructure creating/updating 'struct rev_name' instancesSZEDER Gábor1-14/+21
At the beginning of the recursive name_rev() function it creates a new 'struct rev_name' instance for each previously unvisited commit or, if this visit results in better name for an already visited commit, then updates the 'struct rev_name' instance attached to the commit, or returns early. Restructure this so it's caller creates or updates the 'struct rev_name' instance associated with the commit to be passed as parameter, i.e. both name_ref() before calling name_rev() and name_rev() itself as it iterates over the parent commits. This makes eliminating the recursion a bit easier to follow, and the condition moved to name_ref() will be moved back to name_rev() after the recursion is eliminated. This change also plugs the memory leak that was temporarily unplugged in the earlier "name-rev: pull out deref handling from the recursion" patch in this series. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: restructure parsing commits and applying date cutoffSZEDER Gábor1-13/+16
At the beginning of the recursive name_rev() function it parses the commit it got as parameter, and returns early if the commit is older than a cutoff limit. Restructure this so the caller parses the commit and checks its date, and doesn't invoke name_rev() if the commit to be passed as parameter is older than the cutoff, i.e. both name_ref() before calling name_rev() and name_rev() itself as it iterates over the parent commits. This makes eliminating the recursion a bit easier to follow, and the condition moved to name_ref() will be moved back to name_rev() after the recursion is eliminated. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: pull out deref handling from the recursionSZEDER Gábor1-17/+10
The 'if (deref) { ... }' condition near the beginning of the recursive name_rev() function can only ever be true in the first invocation, because the 'deref' parameter is always 0 in the subsequent recursive invocations. Extract this condition from the recursion into name_rev()'s caller and drop the function's 'deref' parameter. This makes eliminating the recursion a bit easier to follow, and it will be moved back into name_rev() after the recursion is eliminated. Furthermore, drop the condition that die()s when both 'deref' and 'generation' are non-null (which should have been a BUG() to begin with). Note that this change reintroduces the memory leak that was plugged in in commit 5308224633 (name-rev: avoid leaking memory in the `deref` case, 2017-05-04), but a later patch (name-rev: restructure creating/updating 'struct rev_name' instances) in this series will plug it in again. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: extract creating/updating a 'struct name_rev' into a helperSZEDER Gábor1-13/+27
In a later patch in this series we'll want to do this in two places. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t6120: add a test to cover inner conditions in 'git name-rev's name_rev()SZEDER Gábor1-0/+41
In 'builtin/name-rev.c' in the name_rev() function there is a loop iterating over all parents of the given commit, and the loop body looks like this: if (parent_number > 1) { if (generation > 0) // branch #1 new_name = ... else // branch #2 new_name = ... name_rev(parent, new_name, ...); } else { // branch #3 name_rev(...); } These conditions are not covered properly in the test suite. As far as purely test coverage goes, they are all executed several times over in 't6120-describe.sh'. However, they don't directly influence the command's output, because the repository used in that test script contains several branches and tags pointing somewhere into the middle of the commit DAG, and thus result in a better name for the to-be-named commit. This can hide bugs: e.g. by replacing the 'new_name' parameter of the first recursive name_rev() call with 'tip_name' (effectively making both branch #1 and #2 a noop) 'git name-rev --all' shows thousands of bogus names in the Git repository, but the whole test suite still passes successfully. In an early version of a later patch in this series I managed to mess up all three branches (at once!), but the test suite still passed. So add a new test case that operates on the following history: A--------------master \ / \----------M2 \ / \---M1-C \ / B and names the commit 'B' to make sure that all three branches are crucial to determine 'B's name: - There is only a single ref, so all names are based on 'master', without any undesired interference from other refs. - Each time name_rev() follows the second parent of a merge commit, it appends "^2" to the name. Following 'master's second parent right at the start ensures that all commits on the ancestry path from 'master' to 'B' have a different base name from the original 'tip_name' of the very first name_rev() invocation. Currently, while name_rev() is recursive, it doesn't matter, but it will be necessary to properly cover all three branches after the recursion is eliminated later in this series. - Following 'M2's second parent makes sure that branch #2 (i.e. when 'generation = 0') affects 'B's name. - Following the only parent of the non-merge commit 'C' ensures that branch #3 affects 'B's name, and that it increments 'generation'. - Coming from 'C' 'generation' is 1, thus following 'M1's second parent makes sure that branch #1 affects 'B's name. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: use sizeof(*ptr) instead of sizeof(type) in allocationSZEDER Gábor1-1/+1
Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: avoid unnecessary cast in name_ref()SZEDER Gábor1-1/+1
Casting a 'struct object' to 'struct commit' is unnecessary there, because it's already available in the local 'commit' variable. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06name-rev: use strbuf_strip_suffix() in get_rev_name()René Scharfe1-4/+3
get_name_rev() basically open-codes strip_suffix() before adding a string to a strbuf. Let's use the strbuf right from the beginning, i.e. add the whole string to the strbuf and then use strbuf_strip_suffix(), making the code more idiomatic. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t6120-describe: modernize the 'check_describe' helperSZEDER Gábor1-6/+4
The 'check_describe' helper function runs 'git describe' outside of 'test_expect_success' blocks, with extra hand-rolled code to record and examine its exit code. Update this helper and move the 'git describe' invocation inside the 'test_expect_success' block. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06range-diff: clear `other_arg` at end of functionDenton Liu1-0/+1
We were leaking memory by not clearing `other_arg` after we were done using it. Clear it after we've finished using it. Note that this isn't strictly necessary since the memory will be reclaimed once the command exits. However, since we are releasing the strbufs, we should also clear `other_arg` for consistency. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06range-diff: mark pointers as constDenton Liu2-5/+5
The contents pointed to by `diffopt` and `other_arg` should not be modified. Mark these as `const` to indicate this. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3206: fix incorrect test nameDenton Liu1-1/+1
The name of the test used to indicate that it was testing the `--notes` option but it was really testing the `format.notes` configuration. Correct the test name to reflect this. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t9300-fast-import: don't hang if background fast-import exits too earlySZEDER Gábor1-2/+14
The five tests checking 'git fast-import's checkpoint handling in 't9300-fast-import.sh', all with the prefix "V:" in their test description, can hang indefinitely if 'git fast-import' unexpectedly dies early in any of these tests. These five tests run 'git fast-import' in the background, while feeding instructions to its standard input through a fifo (fd 8) from a background subshell, and reading and verifying its standard output through another fifo (fd 9) in the test script's main shell process. This "reading and verifying" is basically a 'while read ...' shell loop iterating until 'git fast-import' outputs the expected line, ignoring any other output. This doesn't work very well when 'git fast-import' dies before printing that particular line, because the 'read' builtin doesn't get EOF after the death of 'git fast-import', as their input and output are not connected directly but through a fifo. Consequently, that 'read' hangs waiting for the next line from the already dead 'git fast-import', leaving the test script and in turn the whole test suite hanging. Avoid this hang by checking whether the background 'git fast-import' process exited unexpectedly early, and interrupt the 'while read' loop if it did. We have to jump through some hoops to achive that, though: - Start the background 'git fast-import' in another background subshell, which then: - prints the PID of that 'git fast-import' process to the fifo, to be read by the main shell process, so it will know which process to kill when the test is finished. - waits until that 'git fast-import' process exits. If it does exit, then report its exit code, and write a message to the fifo used for 'git fast-import's standard output, thus un-block the 'read' builtin in the main shell process. - Modify that 'while read' loop to break the loop upon seeing that message, and fail the test in the usual way. - Once the test is finished kill that background subshell as well, and do so before killing the background 'git fast-import'. Otherwise the background 'git fast-import' and subshell processes would die racily, and if 'git fast-import' were to die sooner, then we might get some undesired and potentially confusing messages in the test's output. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t9300-fast-import: store the PID in a variable instead of pidfileSZEDER Gábor1-3/+3
The five tests running 'git fast-import' in the background in 't9300-fast-import.sh' store the PID of that background process in a pidfile, to be used to check whether that background process survived each test and then to kill it in test_when_finished commands. To achieve this all these five tests run three $(cat <pidfile>) command substitutions each. Store the PID of the background 'git fast-import' in a variable to avoid those extra processes. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06commit: give correct advice for empty commit during a rebasePhillip Wood5-19/+114
In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02), `git commit` learned to suggest to run `git cherry-pick --skip` when trying to cherry-pick an empty patch. However, it was overlooked that there are more conditions than just a `git cherry-pick` when this advice is printed (which originally suggested the neutral `git reset`): the same can happen during a rebase. Let's suggest the correct command, even during a rebase. While at it, we adjust more places in `builtin/commit.c` that incorrectly assumed that the presence of a `CHERRY_PICK_HEAD` meant that surely this must be a `cherry-pick` in progress. Note: we take pains to handle the situation when a user runs a `git cherry-pick` _during_ a rebase. This is quite valid (e.g. in an `exec` line in an interactive rebase). On the other hand, it is not possible to run a rebase during a cherry-pick, meaning: if both `rebase-merge/` and `sequencer/` exist or CHERRY_PICK_HEAD and REBASE_HEAD point to the same commit , we still want to advise to use `git cherry-pick --skip`. Original-patch-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06commit: encapsulate determine_whence() for sequencerPhillip Wood3-6/+15
Working out which command wants to create a commit requires detailed knowledge of the sequencer internals and that knowledge is going to increase in subsequent commits. With that in mind lets encapsulate that knowledge in sequencer.c rather than spreading it into builtin/commit.c. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06commit: use enum value for multiple cherry-picksPhillip Wood2-15/+19
Add FROM_CHERRY_PICK_MULTI for a sequence of cherry-picks rather than using a separate variable. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06sequencer: write CHERRY_PICK_HEAD for reword and editPhillip Wood2-1/+56
`git commit` relies on the presence of CHERRY_PICK_HEAD to show the correct error message in the case of an empty pick. This fixes a regression introduced by the conversion from shell to C. In the shell version everything was a cherry-pick as far as the sequencer code was concerned so it always wrote CHERRY_PICK_HEAD. The conversion to C forgot to update the code that creates CHERRY_PICK_HEAD. We do not want to create CHERRY_PICK_HEAD for fixup and squash commands as that would prevent `git commit --amend` from running. Note that the error message shown by `git commit` for an empty pick during a rebase is currently wrong as it talks about running `git cherry-pick --skip` rather than `git rebase --skip`. This will be fixed in a future commit which is why the tests are in t3403-rebase-skip.sh. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06cherry-pick: check commit error messagesPhillip Wood1-0/+23
We disallow partial commits and amending when CHERRY_PICK_HEAD exists. Add a couple of tests to check the error message printed in each case before we refactor the code responsible for this. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06cherry-pick: add test for `--skip` advice in `git commit`Johannes Schindelin1-1/+2
In dcb500dc16c (cherry-pick/revert: advise using --skip, 2019-07-02), `git commit` learned to suggest to run `git cherry-pick --skip` when trying to cherry-pick an empty patch, but that was never tested for. Here is a test that verifies that a message is given to the user that contains the correct invocation. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3404: use test_cmp_revPhillip Wood1-19/+19
There are a number of places where we compare two revisions with test $(git rev-parse rev1) = $(git rev-parse rev2) when these fail there's no indication what has gone wrong and you need to be running with `-x` to see where the test has failed. Lets use test_cmp_rev instead. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06apply --allow-overlap: fix a corner caseJohannes Schindelin1-0/+10
Yes, yes, this is supposed to be only a band-aid option for `git add -p` not Doing The Right Thing. But as long as we carry the `--allow-overlap` option, we might just as well get it right. This fixes the case where one hunk inserts a line before the first line, and is followed by a hunk whose context overlaps with the first one's and which appends a line at the end. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06git add -p: use non-zero exit code when the diff generation failedJohannes Schindelin2-4/+6
The first thing `git add -p` does is to generate a diff. If this diff cannot be generated, `git add -p` should not continue as if nothing happened, but instead fail. What we *actually* do here is much broader: we now verify for *every* `run_cmd_pipe()` call that the spawned process actually succeeded. Note that we have to change two callers in this patch, as we need to store the spawned process' output in a local variable, which means that the callers can no longer decide whether to interpret the `return <$fh>` in array or in scalar context. This bug was noticed while writing a test case for the diff.algorithm feature, and we let that test case double as a regression test for this fixed bug, too. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3701: verify that the diff.algorithm config setting is handledJohannes Schindelin1-0/+10
Without this patch, there is actually no test in Git's test suite that covers the diff.algorithm feature. Let's add one. We do this by passing a bogus value and then expecting `git diff-files` to produce the appropriate error message. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>