summaryrefslogtreecommitdiff
path: root/t/t3404-rebase-interactive.sh
AgeCommit message (Collapse)AuthorFilesLines
2019-02-13rebase: fix regression in rebase.useBuiltin=false test modeLibravatar Ævar Arnfjörð Bjarmason1-3/+6
Fix a recently introduced regression in c762aada1a ("rebase -x: sanity check command", 2019-01-29) triggered when running the tests with GIT_TEST_REBASE_USE_BUILTIN=false. See 62c23938fa ("tests: add a special setup where rebase.useBuiltin is off", 2018-11-14) for how that test mode works. As discussed on-list[1] it's not worth it to implement the sanity check in the legacy rebase code, we plan to remove it after the 2.21 release. So let's do the bare minimum to make the tests pass under the GIT_TEST_REBASE_USE_BUILTIN=false special setup. 1. https://public-inbox.org/git/xmqqva1nbeno.fsf@gitster-ct.c.googlers.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-02-06Merge branch 'pw/rebase-x-sanity-check'Libravatar Junio C Hamano1-0/+19
"git rebase -x $cmd" did not reject multi-line command, even though the command is incapable of handling such a command. It now is rejected upfront. * pw/rebase-x-sanity-check: rebase -x: sanity check command
2019-01-29rebase -x: sanity check commandLibravatar Phillip Wood1-0/+19
If the user gives an empty argument to --exec then git creates a todo list that it cannot parse. The rebase starts to run before erroring out with error: missing arguments for exec error: invalid line 2: exec You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'. Or you can abort the rebase with 'git rebase --abort'. Instead check for empty commands before starting the rebase. Also check that the command does not contain any newlines as the todo-list format is unable to cope with multiline commands. Note that this changes the behavior, before this change one could do git rebase --exec='echo one exec echo two' and it would insert two exec lines in the todo list, now it will error out. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-01-28implicit interactive rebase: don't run sequence editorLibravatar Phillip Wood1-0/+5
If GIT_SEQUENCE_EDITOR is set then rebase runs it when executing implicit interactive rebases which are supposed to appear non-interactive to the user. Fix this by setting GIT_SEQUENCE_EDITOR=: rather than GIT_EDITOR=:. A couple of tests relied on the old behavior so they are updated to work with the new regime. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-13Merge branch 'js/rebase-p-tests'Libravatar Junio C Hamano1-11/+12
In preparation to the day when we can deprecate and remove the "rebase -p", make sure we can skip and later remove tests for it. * js/rebase-p-tests: tests: optionally skip `git rebase -p` tests t3418: decouple test cases from a previous `rebase -p` test case t3404: decouple some test cases from outcomes of previous test cases
2018-11-06Merge branch 'sg/test-rebase-editor-fix'Libravatar Junio C Hamano1-5/+5
* sg/test-rebase-editor-fix: t3404-rebase-interactive: test abbreviated commands
2018-11-02tests: optionally skip `git rebase -p` testsLibravatar Johannes Schindelin1-4/+4
The `--preserve-merges` mode of the `rebase` command is slated to be deprecated soon, as the more powerful `--rebase-merges` mode is available now, and the latter was designed with the express intent to address the shortcomings of `--preserve-merges`' design (e.g. the inability to reorder commits in an interactive rebase). As such, we will eventually even remove the `--preserve-merges` support, and along with it, its tests. In preparation for this, and also to allow the Windows phase of our automated tests to save some well-needed time when running the test suite, this commit introduces a new prerequisite REBASE_P, which can be forced to being unmet by setting the environment variable `GIT_TEST_SKIP_REBASE_P` to any non-empty string. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-02t3404: decouple some test cases from outcomes of previous test casesLibravatar Johannes Schindelin1-7/+8
Originally, the `--preserve-merges` option of the `git rebase` command piggy-backed on top of the `--interactive` feature. For that reason, the early test cases were added to the very same test script that contains the `git rebase -i` tests: `t3404-rebase-interactive.sh`. However, since c42abfe7857 (rebase: introduce a dedicated backend for --preserve-merges, 2018-05-28), the `--preserve-merges` feature got its own backend, in preparation for converting the rest of the `--interactive` code to built-in code, written in C rather than shell. The reason why the `--preserve-merges` feature was not converted at the same time is that we have something much better now: `--rebase-merges`. That option intends to supersede `--preserve-merges`, and we will probably deprecate the latter soon. Once `--preserve-merges` has been deprecated for a good amount of time, it will be time to remove it, and along with it, its tests. In preparation for that, let's make the rest of the test cases in `t3404-rebase-interactive.sh` independent of the test cases dedicated to `--preserve-merges`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-11-02Merge branch 'ag/rebase-i-in-c'Libravatar Junio C Hamano1-0/+10
Rewrite of the remaining "rebase -i" machinery in C. * ag/rebase-i-in-c: rebase -i: move rebase--helper modes to rebase--interactive rebase -i: remove git-rebase--interactive.sh rebase--interactive2: rewrite the submodes of interactive rebase in C rebase -i: implement the main part of interactive rebase as a builtin rebase -i: rewrite init_basic_state() in C rebase -i: rewrite write_basic_state() in C rebase -i: rewrite the rest of init_revisions_and_shortrevisions() in C rebase -i: implement the logic to initialize $revisions in C rebase -i: remove unused modes and functions rebase -i: rewrite complete_action() in C t3404: todo list with commented-out commands only aborts sequencer: change the way skip_unnecessary_picks() returns its result sequencer: refactor append_todo_help() to write its message to a buffer rebase -i: rewrite checkout_onto() in C rebase -i: rewrite setup_reflog_action() in C sequencer: add a new function to silence a command, except if it fails rebase -i: rewrite the edit-todo functionality in C editor: add a function to launch the sequence editor rebase -i: rewrite append_todo_help() in C sequencer: make three functions and an enum from sequencer.c public
2018-10-29t3404-rebase-interactive: test abbreviated commandsLibravatar Johannes Sixt1-5/+5
Make sure that each short command is tested at least once. To not exacerbate the runtime of the test script, do not add new tests, but modify existing ones according to these criteria: - The test does not have a prerequisite. - The 'git rebase' command is not guarded by test_must_fail. The pick commands are optional in the FAKE_LINES variable, but when used, they do end up in the insn sheet. Test them, too. Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-24Merge branch 'en/sequencer-empty-edit-result-aborts'Libravatar Junio C Hamano1-4/+3
"git rebase" etc. in Git 2.19 fails to abort when given an empty commit log message as result of editing, which has been corrected. * en/sequencer-empty-edit-result-aborts: sequencer: fix --allow-empty-message behavior, make it smarter
2018-09-13sequencer: fix --allow-empty-message behavior, make it smarterLibravatar Elijah Newren1-4/+3
In commit b00bf1c9a8dd ("git-rebase: make --allow-empty-message the default", 2018-06-27), several arguments were given for transplanting empty commits without halting and asking the user for confirmation on each commit. These arguments were incomplete because the logic clearly assumed the only cases under consideration were transplanting of commits with empty messages (see the comment about "There are two sources for commits with empty messages). It didn't discuss or even consider rewords, squashes, etc. where the user is explicitly asked for a new commit message and provides an empty one. (My bad, I totally should have thought about that at the time, but just didn't.) Rewords and squashes are significantly different, though, as described by SZEDER: Let's suppose you start an interactive rebase, choose a commit to squash, save the instruction sheet, rebase fires up your editor, and then you notice that you mistakenly chose the wrong commit to squash. What do you do, how do you abort? Before [that commit] you could clear the commit message, exit the editor, and then rebase would say "Aborting commit due to empty commit message.", and you get to run 'git rebase --abort', and start over. But [since that commit, ...] saving the commit message as is would let rebase continue and create a bunch of unnecessary objects, and then you would have to use the reflog to return to the pre-rebase state. Also, he states: The instructions in the commit message template, which is shown for 'reword' and 'squash', too, still say... # Please enter the commit message for your changes. Lines starting # with '#' will be ignored, and an empty message aborts the commit. These are sound arguments that when editing commit messages during a sequencer operation, that if the commit message is empty then the operation should halt and ask the user to correct. The arguments in commit b00bf1c9a8dd (referenced above) still apply when transplanting previously created commits with empty commit messages, so the sequencer should not halt for those. Furthermore, all rationale so far applies equally for cherry-pick as for rebase. Therefore, make the code default to --allow-empty-message when transplanting an existing commit, and to default to halting when the user is asked to edit a commit message and provides an empty one -- for both rebase and cherry-pick. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-04Merge branch 'pw/rebase-i-author-script-fix'Libravatar Junio C Hamano1-3/+15
Recent "git rebase -i" update started to write bogusly formatted author-script, with a matching broken reading code. These are fixed. * pw/rebase-i-author-script-fix: sequencer: fix quoting in write_author_script sequencer: handle errors from read_author_ident()
2018-08-29rebase -i: rewrite complete_action() in CLibravatar Alban Gruin1-1/+1
This rewrites complete_action() from shell to C. A new mode is added to rebase--helper (`--complete-action`), as well as a new flag (`--autosquash`). Finally, complete_action() is stripped from git-rebase--interactive.sh. The original complete_action() would return the code 2 when the todo list contained no actions. This was a special case for rebase -i and -p; git-rebase.sh would then apply the autostash, delete the state directory, and die with the message "Nothing to do". This cleanup is rewritten in C instead of returning 2. As rebase -i no longer returns 2, the comment describing this behaviour in git-rebase.sh is updated to reflect this change. The message "Nothing to do" is now printed with error(), and so becomes "error: nothing to do". Some tests in t3404 check this value, so they are updated to fit this change. The first check might seem useless as we write "noop" to the todo list if it is empty. Actually, the todo list might contain commented commands (ie. empty commits). In this case, complete_action() won’t write "noop", and will abort without starting the editor. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-27Merge branch 'sg/test-must-be-empty'Libravatar Junio C Hamano1-3/+2
Test fixes. * sg/test-must-be-empty: tests: use 'test_must_be_empty' instead of 'test_cmp <empty> <out>' tests: use 'test_must_be_empty' instead of 'test_cmp /dev/null <out>' tests: use 'test_must_be_empty' instead of 'test ! -s' tests: use 'test_must_be_empty' instead of '! test -s'
2018-08-21tests: use 'test_must_be_empty' instead of 'test_cmp <empty> <out>'Libravatar SZEDER Gábor1-3/+2
Using 'test_must_be_empty' is shorter and more idiomatic than >empty && test_cmp empty out as it saves the creation of an empty file. Furthermore, sometimes the expected empty file doesn't have such a descriptive name like 'empty', and its creation is far away from the place where it's finally used for comparison (e.g. in 't7600-merge.sh', where two expected empty files are created in the 'setup' test, but are used only about 500 lines later). These cases were found by instrumenting 'test_cmp' to error out the test script when it's used to compare empty files, and then converted manually. Note that even after this patch there still remain a lot of cases where we use 'test_cmp' to check empty files: - Sometimes the expected output is not hard-coded in the test, but 'test_cmp' is used to ensure that two similar git commands produce the same output, and that output happens to be empty, e.g. the test 'submodule update --merge - ignores --merge for new submodules' in 't7406-submodule-update.sh'. - Repetitive common tasks, including preparing the expected results and running 'test_cmp', are often extracted into a helper function, and some of this helper's callsites expect no output. - For the same reason as above, the whole 'test_expect_success' block is within a helper function, e.g. in 't3070-wildmatch.sh'. - Or 'test_cmp' is invoked in a loop, e.g. the test 'cvs update (-p)' in 't9400-git-cvsserver-server.sh'. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-17Merge branch 'es/rebase-i-author-script-fix'Libravatar Junio C Hamano1-1/+9
The "author-script" file "git rebase -i" creates got broken when we started to move the command away from shell script, which is getting fixed now. * es/rebase-i-author-script-fix: sequencer: don't die() on bogus user-edited timestamp sequencer: fix "rebase -i --root" corrupting author header timestamp sequencer: fix "rebase -i --root" corrupting author header timezone sequencer: fix "rebase -i --root" corrupting author header
2018-08-10t3404: todo list with commented-out commands only abortsLibravatar Alban Gruin1-0/+10
If the todo list generated by `--make-script` is empty, complete_action() writes a noop, but if it has only commented-out commands, it will abort with the message "Nothing to do", and does not launch the editor. This adds a new test to ensure that complete_action() behaves this way. Signed-off-by: Alban Gruin <alban.gruin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-08-07sequencer: fix quoting in write_author_scriptLibravatar Phillip Wood1-3/+15
Single quotes should be escaped as \' not \\'. The bad quoting breaks the interactive version of 'rebase --root' (which is used when there is no '--onto' even if the user does not specify --interactive) for authors that contain "'" as sq_dequote() called by read_author_ident() errors out on the bad quoting. For other interactive rebases this only affects external scripts that read the author script and users whose git is upgraded from the shell version of rebase -i while rebase was stopped when the author contains "'". This is because the parsing in read_env_script() expected the broken quoting. This patch includes code to handle the broken quoting when git has been upgraded while rebase was stopped. It does this by detecting the missing "'" at the end of the GIT_AUTHOR_DATE line to see if it should dequote \\' as "'". Note this is only implemented for normal picks, not for creating a new root commit (rebase will stop with an error complaining out bad quoting in that case). The fallback code has been manually tested by reverting both the quoting fixes in write_author_script() and the previous fix for the missing "'" at the end of the GIT_AUTHOR_DATE line and running t3404-rebase-interactive.sh. Ideally rebase and am would share the same code for reading and writing the author script, but this commit just fixes the immediate bug. Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-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>
2018-08-02Merge branch 'bc/sequencer-export-work-tree-as-well'Libravatar Junio C Hamano1-0/+9
"git rebase" started exporting GIT_DIR environment variable and exposing it to hook scripts when part of it got rewritten in C. Instead of matching the old scripted Porcelains' behaviour, compensate by also exporting GIT_WORK_TREE environment as well to lessen the damage. This can harm existing hooks that want to operate on different repository, but the current behaviour is already broken for them anyway. * bc/sequencer-export-work-tree-as-well: sequencer: pass absolute GIT_WORK_TREE to exec commands
2018-08-02Merge branch 'es/test-fixes'Libravatar Junio C Hamano1-3/+3
Test clean-up and corrections. * es/test-fixes: (26 commits) t5608: fix broken &&-chain t9119: fix broken &&-chains t9000-t9999: fix broken &&-chains t7000-t7999: fix broken &&-chains t6000-t6999: fix broken &&-chains t5000-t5999: fix broken &&-chains t4000-t4999: fix broken &&-chains t3030: fix broken &&-chains t3000-t3999: fix broken &&-chains t2000-t2999: fix broken &&-chains t1000-t1999: fix broken &&-chains t0000-t0999: fix broken &&-chains t9814: simplify convoluted check that command correctly errors out t9001: fix broken "invoke hook" test t7810: use test_expect_code() instead of hand-rolled comparison t7400: fix broken "submodule add/reconfigure --force" test t7201: drop pointless "exit 0" at end of subshell t6036: fix broken "merge fails but has appropriate contents" tests t5505: modernize and simplify hard-to-digest test t5406: use write_script() instead of birthing shell script manually ...
2018-07-31sequencer: fix "rebase -i --root" corrupting author header timestampLibravatar Eric Sunshine1-1/+1
When "git rebase -i --root" creates a new root commit, it corrupts the "author" header's timestamp by prepending a "@": author A U Thor <author@example.com> @1112912773 -0700 The commit parser is very strict about the format of the "author" header, and does not allow a "@" in that position. The "@" comes from GIT_AUTHOR_DATE in "rebase-merge/author-script", signifying a Unix epoch-based timestamp, however, read_author_ident() incorrectly allows it to slip into the commit's "author" header, thus corrupting it. One possible fix would be simply to filter out the "@" when constructing the "author" header timestamp, however, a more correct fix is to parse the GIT_AUTHOR_DATE date (via parse_date()) and format the parsed result into the "author" header. Since "rebase-merge/author-script" may be edited by the user, this approach has the extra benefit of catching other potential timestamp corruption due to hand-editing. We can do better than calling parse_date() ourselves and constructing the "author" header manually, however, by instead taking advantage of fmt_ident() which does this work for us. The benefits of using fmt_ident() are twofold. First, it simplifies the logic considerably by allowing us to avoid the complexity of building the "author" header in parallel with and in the same buffer from which "rebase-merge/author-script" is being parsed. Instead, fmt_ident() is invoked to compose the header after parsing is complete. Second, fmt_ident() is careful to prevent "crud" from polluting the composed ident. As with validating GIT_AUTHOR_DATE, this "crud" avoidance prevents other (possibly hand-edited) bogus author information from "rebase-merge/author-script" from corrupting the commit object. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31sequencer: fix "rebase -i --root" corrupting author header timezoneLibravatar Eric Sunshine1-1/+1
When "git rebase -i --root" creates a new root commit, it corrupts the "author" header's timezone by repeating the last digit: author A U Thor <author@example.com> @1112912773 -07000 This is due to two bugs. First, write_author_script() neglects to add the closing quote to the value of GIT_AUTHOR_DATE when generating "rebase-merge/author-script". Second, although sq_dequote() correctly diagnoses the missing closing quote, read_author_ident() ignores sq_dequote()'s return value and blindly uses the result of the aborted dequote. sq_dequote() performs dequoting in-place by removing quoting and shifting content downward. When it detects misquoting (lack of closing quote, in this case), it gives up and returns an error without inserting a NUL-terminator at the end of the shifted content, which explains the duplicated last digit in the timezone. (Note that the "@" preceding the timestamp is a separate bug which will be fixed subsequently.) Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-31sequencer: fix "rebase -i --root" corrupting author headerLibravatar Eric Sunshine1-1/+9
When "git rebase -i --root" creates a new root commit (say, by swapping in a different commit for the root), it corrupts the commit's "author" header with trailing garbage: author A U Thor <author@example.com> @1112912773 -07000or@example.com This is a result of read_author_ident() neglecting to NUL-terminate the buffer into which it composes the "author" header. (Note that the "@" preceding the timestamp and the extra "0" in the timezone are separate bugs which will be fixed subsequently.) Security considerations: Construction of the "author" header by read_author_ident() happens in-place and in parallel with parsing the content of "rebase-merge/author-script" which occupies the same buffer. This is possible because the constructed "author" header is always smaller than the content of "rebase-merge/author-script". Despite neglecting to NUL-terminate the constructed "author" header, memory is never accessed (either by read_author_ident() or its caller) beyond the allocated buffer since a NUL-terminator is present at the end of the loaded "rebase-merge/author-script" content, and additional NUL's are inserted as part of the parsing process. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-24Merge branch 'jc/t3404-one-shot-export-fix'Libravatar Junio C Hamano1-1/+8
Correct a broken use of "VAR=VAL shell_func" in a test. * jc/t3404-one-shot-export-fix: t3404: fix use of "VAR=VAL cmd" with a shell function
2018-07-24Merge branch 'en/rebase-consistency'Libravatar Junio C Hamano1-3/+4
"git rebase" behaved slightly differently depending on which one of the three backends gets used; this has been documented and an effort to make them more uniform has begun. * en/rebase-consistency: git-rebase: make --allow-empty-message the default t3401: add directory rename testcases for rebase and am git-rebase.txt: document behavioral differences between modes directory-rename-detection.txt: technical docs on abilities and limitations git-rebase.txt: address confusion between --no-ff vs --force-rebase git-rebase: error out when incompatible options passed t3422: new testcases for checking when incompatible options passed git-rebase.sh: update help messages a bit git-rebase.txt: document incompatible options
2018-07-16sequencer: pass absolute GIT_WORK_TREE to exec commandsLibravatar brian m. carlson1-0/+9
The sequencer currently passes GIT_DIR, but not GIT_WORK_TREE, to exec commands. In that configuration, we assume that whatever directory we're in is the top level of the work tree, and git rev-parse --show-toplevel responds accordingly. However, when we're in a subdirectory, that isn't correct: we respond with the subdirectory as the top level, resulting in unexpected behavior. Ensure that we pass GIT_WORK_TREE as well as GIT_DIR so that git operations within subdirectories work correctly. Note that we are guaranteed to have a work tree in this case: the relevant sequencer functions are called only from revert, cherry-pick, and rebase--helper; all of these commands require a working tree. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-12t3404: fix use of "VAR=VAL cmd" with a shell functionLibravatar Junio C Hamano1-1/+8
Bash may take it happily but running test with dash reveals a breakage. This was not discovered for a long time as no tests after this test depended on GIT_AUTHOR_NAME to be reverted correctly back to the original value after this step is done. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03t: use test_write_lines() instead of series of 'echo' commandsLibravatar Eric Sunshine1-3/+3
These tests employ a noisy subshell (with missing &&-chain) to feed input into Git commands or files: (echo a; echo b; echo c) | git some-command ... Simplify by taking advantage of test_write_lines(): test_write_lines a b c | git some-command ... Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-27git-rebase: make --allow-empty-message the defaultLibravatar Elijah Newren1-3/+4
rebase backends currently behave differently with empty commit messages, largely as a side-effect of the different underlying commands on which they are based. am-based rebases apply commits with an empty commit message without stopping or requiring the user to specify an extra flag. (It is interesting to note that am-based rebases are the default rebase type, and no one has ever requested a --no-allow-empty-message flag to change this behavior.) merge-based and interactive-based rebases (which are ultimately based on git-commit), will currently halt on any such commits and require the user to manually specify what to do with the commit and continue. One possible rationale for the difference in behavior is that the purpose of an "am" based rebase is solely to transplant an existing history, while an "interactive" rebase is one whose purpose is to polish a series before making it publishable. Thus, stopping and asking for confirmation for a possible problem is more appropriate in the latter case. However, there are two problems with this rationale: 1) merge-based rebases are also non-interactive and there are multiple types of rebases that use the interactive machinery but are not explicitly interactive (e.g. when either --rebase-merges or --keep-empty are specified without --interactive). These rebases are also used solely to transplant an existing history, and thus also should default to --allow-empty-message. 2) this rationale only says that the user is more accepting of stopping in the case of an explicitly interactive rebase, not that stopping for this particular reason actually makes sense. Exploring whether it makes sense, requires backing up and analyzing the underlying commands... If git-commit did not error out on empty commits by default, accidental creation of commits with empty messages would be a very common occurrence (this check has caught me many times). Further, nearly all such empty commit messages would be considered an accidental error (as evidenced by a huge amount of documentation across version control systems and in various blog posts explaining how important commit messages are). A simple check for what would otherwise be a common error thus made a lot of sense, and git-commit gained an --allow-empty-message flag for special case overrides. This has made commits with empty messages very rare. There are two sources for commits with empty messages for rebase (and cherry-pick): (a) commits created in git where the user previously specified --allow-empty-message to git-commit, and (b) commits imported into git from other version control systems. In case (a), the user has already explicitly specified that there is something special about this commit that makes them not want to specify a commit message; forcing them to re-specify with every cherry-pick or rebase seems more likely to be infuriating than helpful. In case (b), the commit is highly unlikely to have been authored by the person who has imported the history and is doing the rebase or cherry-pick, and thus the user is unlikely to be the appropriate person to write a commit message for it. Stopping and expecting the user to modify the commit before proceeding thus seems counter-productive. Further, note that while empty commit messages was a common error case for git-commit to deal with, it is a rare case for rebase (or cherry-pick). The fact that it is rare raises the question of why it would be worth checking and stopping on this particular condition and not others. For example, why doesn't an interactive rebase automatically stop if the commit message's first line is 2000 columns long, or is missing a blank line after the first line, or has every line indented with five spaces, or any number of other myriad problems? Finally, note that if a user doing an interactive rebase does have the necessary knowledge to add a message for any such commit and wants to do so, it is rather simple for them to change the appropriate line from 'pick' to 'reword'. The fact that the subject is empty in the todo list that the user edits should even serve as a way to notify them. As far as I can tell, the fact that merge-based and interactive-based rebases stop on commits with empty commit messages is solely a by-product of having been based on git-commit. It went without notice for a long time precisely because such cases are rare. The rareness of this situation made it difficult to reason about, so when folks did eventually notice this behavior, they assumed it was there for a good reason and just added an --allow-empty-message flag. In my opinion, stopping on such messages not desirable in any of these cases, even the (explicitly) interactive case. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-19sequencer: do not squash 'reword' commits when we hit conflictsLibravatar Phillip Wood1-0/+28
Ever since commit 18633e1a22 ("rebase -i: use the rebase--helper builtin", 2017-02-09), when a commit marked as 'reword' in an interactive rebase has conflicts and fails to apply, when the rebase is resumed that commit will be squashed into its parent with its commit message taken. The issue can be understood better by looking at commit 56dc3ab04b ("sequencer (rebase -i): implement the 'edit' command", 2017-01-02), which introduced error_with_patch() for the edit command. For the edit command, it needs to stop the rebase whether or not the patch applies cleanly. If the patch does apply cleanly, then when it resumes it knows it needs to amend all changes into the previous commit. If it does not apply cleanly, then the changes should not be amended. Thus, it passes !res (success of applying the 'edit' commit) to error_with_patch() for the to_amend flag. The problematic line of code actually came from commit 04efc8b57c ("sequencer (rebase -i): implement the 'reword' command", 2017-01-02). Note that to get to this point in the code: * !!res (i.e. patch application failed) * item->command < TODO_SQUASH * item->command != TODO_EDIT * !is_fixup(item->command) [i.e. not squash or fixup] So that means this can only be a failed patch application that is either a pick, revert, or reword. We only need to amend HEAD when rewording the root commit or a commit that has been fast-forwarded, for any of the other cases we want a new commit, so we should not set the to_amend flag. Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Original-patch-by: Elijah Newren <newren@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-19t3404: check root commit in 'rebase -i --root reword root commit'Libravatar Todd Zullinger1-1/+2
When testing a reworded root commit, ensure that the squash-onto commit which is created and amended is still the root commit. Suggested-by: Phillip Wood <phillip.wood@talktalk.net> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-18rebase --root: fix amending root commit messagesLibravatar Johannes Schindelin1-1/+1
The code path that triggered that "BUG" really does not want to run without an explicit commit message. In the case where we want to amend a commit message, we have an *implicit* commit message, though: the one of the commit to amend. Therefore, this code path should not even be entered. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-18rebase --root: demonstrate a bug while amending root commit messagesLibravatar Todd Zullinger1-0/+9
When splitting a repository, running `git rebase -i --root` to reword the initial commit, Git dies with BUG: sequencer.c:795: root commit without message. Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-05-06rebase -i --root: let the sequencer handle even the initial partLibravatar Johannes Schindelin1-6/+13
In this developer's earlier attempt to accelerate interactive rebases by converting large parts from Unix shell script into portable, performant C, the --root handling was specifically excluded (to simplify the task a little bit; it still took over a year to get that reduced set of patches into Git proper). This patch ties up that loose end: now only --preserve-merges uses the slow Unix shell script implementation to perform the interactive rebase. As the rebase--helper reports progress to stderr (unlike the scripted interactive rebase, which reports it to stdout, of all places), we have to adjust a couple of tests that did not expect that for `git rebase -i --root`. This patch fixes -- at long last! -- the really old bug reported in 6a6bc5bdc4d (add tests for rebasing root, 2013-06-06) that rebasing with --root *always* rewrote the root commit, even if there were no changes. The bug still persists in --preserve-merges mode, of course, but that mode will be deprecated as soon as the new --rebase-merges mode stabilizes, anyway. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-25Merge branch 'ps/test-chmtime-get'Libravatar Junio C Hamano1-1/+1
Test cleanup. * ps/test-chmtime-get: t/helper: 'test-chmtime (--get|-g)' to print only the mtime
2018-04-11Merge branch 'nd/combined-test-helper'Libravatar Junio C Hamano1-2/+2
Small test-helper programs have been consolidated into a single binary. * nd/combined-test-helper: (36 commits) t/helper: merge test-write-cache into test-tool t/helper: merge test-wildmatch into test-tool t/helper: merge test-urlmatch-normalization into test-tool t/helper: merge test-subprocess into test-tool t/helper: merge test-submodule-config into test-tool t/helper: merge test-string-list into test-tool t/helper: merge test-strcmp-offset into test-tool t/helper: merge test-sigchain into test-tool t/helper: merge test-sha1-array into test-tool t/helper: merge test-scrap-cache-tree into test-tool t/helper: merge test-run-command into test-tool t/helper: merge test-revision-walking into test-tool t/helper: merge test-regex into test-tool t/helper: merge test-ref-store into test-tool t/helper: merge test-read-cache into test-tool t/helper: merge test-prio-queue into test-tool t/helper: merge test-path-utils into test-tool t/helper: merge test-online-cpus into test-tool t/helper: merge test-mktemp into test-tool t/helper: merge (unused) test-mergesort into test-tool ...
2018-04-10Merge branch 'ps/contains-id-error-message'Libravatar Junio C Hamano1-4/+2
"git tag --contains no-such-commit" gave a full list of options after giving an error message. * ps/contains-id-error-message: parse-options: do not show usage upon invalid option value
2018-04-09t/helper: 'test-chmtime (--get|-g)' to print only the mtimeLibravatar Paul-Sebastian Ungureanu1-1/+1
Compared to 'test-chmtime -v +0 file' which prints the mtime and and the file name, 'test-chmtime --get file' displays only the mtime. If it is used in combination with (+|=|=+|=-|-)seconds, it changes and prints the new value. test-chmtime -v +0 file | sed 's/[^0-9].*$//' is now equivalent to: test-chmtime --get file Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-27t/helper: merge test-chmtime into test-toolLibravatar Nguyễn Thái Ngọc Duy1-2/+2
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-22parse-options: do not show usage upon invalid option valueLibravatar Paul-Sebastian Ungureanu1-4/+2
Usually, the usage should be shown only if the user does not know what options are available. If the user specifies an invalid value, the user is already aware of the available options. In this case, there is no point in displaying the usage anymore. This patch applies to "git tag --contains", "git branch --contains", "git branch --points-at", "git for-each-ref --contains" and many more. Signed-off-by: Paul-Sebastian Ungureanu <ungureanupaulsebastian@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-06Merge branch 'nd/rebase-show-current-patch'Libravatar Junio C Hamano1-0/+8
The new "--show-current-patch" option gives an end-user facing way to get the diff being applied when "git rebase" (and "git am") stops with a conflict. * nd/rebase-show-current-patch: rebase: introduce and use pseudo-ref REBASE_HEAD rebase: add --show-current-patch am: add --show-current-patch
2018-02-13Merge branch 'pw/sequencer-in-process-commit'Libravatar Junio C Hamano1-0/+14
The sequencer infrastructure is shared across "git cherry-pick", "git rebase -i", etc., and has always spawned "git commit" when it needs to create a commit. It has been taught to do so internally, when able, by reusing the codepath "git commit" itself uses, which gives performance boost for a few tens of percents in some sample scenarios. * pw/sequencer-in-process-commit: sequencer: run 'prepare-commit-msg' hook t7505: add tests for cherry-pick and rebase -i/-p t7505: style fixes sequencer: assign only free()able strings to gpg_sign sequencer: improve config handling t3512/t3513: remove KNOWN_FAILURE_CHERRY_PICK_SEES_EMPTY_COMMIT=1 sequencer: try to commit without forking 'git commit' sequencer: load commit related config sequencer: simplify adding Signed-off-by: trailer commit: move print_commit_summary() to libgit commit: move post-rewrite code to libgit Add a function to update HEAD after creating a commit commit: move empty message checks to libgit t3404: check intermediate squash messages
2018-02-12rebase: introduce and use pseudo-ref REBASE_HEADLibravatar Nguyễn Thái Ngọc Duy1-1/+4
The new command `git rebase --show-current-patch` is useful for seeing the commit related to the current rebase state. Some however may find the "git show" command behind it too limiting. You may want to increase context lines, do a diff that ignores whitespaces... For these advanced use cases, the user can execute any command they want with the new pseudo ref REBASE_HEAD. This also helps show where the stopped commit is from, which is hard to see from the previous patch which implements --show-current-patch. Helped-by: Tim Landscheidt <tim@tim-landscheidt.de> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-12rebase: add --show-current-patchLibravatar Nguyễn Thái Ngọc Duy1-0/+5
It is useful to see the full patch while resolving conflicts in a rebase. The only way to do it now is less .git/rebase-*/patch which could turn out to be a lot longer to type if you are in a linked worktree, or not at top-dir. On top of that, an ordinary user should not need to peek into .git directory. The new option is provided to examine the patch. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-22sequencer: assign only free()able strings to gpg_signLibravatar Johannes Schindelin1-0/+10
The gpg_sign member of the replay_opts structure is of type `char *`, meaning that the sequencer deems the string to which gpg_sign points to be under its custody, i.e. it needs to be free()d by the sequencer. Therefore, let's only assign malloc()ed buffers to it. Reported-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-12-05t3404: add test case for abbreviated commandsLibravatar Liam Beguin1-0/+22
Make sure the todo list ends up using single-letter command abbreviations when the rebase.abbreviateCommands is enabled. This configuration option should not change anything else. Signed-off-by: Liam Beguin <liambeguin@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-11t3404: check intermediate squash messagesLibravatar Phillip Wood1-0/+4
When there is more than one squash/fixup command in a row check the intermediate messages are correct. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-06Merge branch 'jk/rebase-i-exec-gitdir-fix'Libravatar Junio C Hamano1-0/+11
A recent regression in "git rebase -i" that broke execution of git commands from subdirectories via "exec" insn has been fixed. * jk/rebase-i-exec-gitdir-fix: sequencer: pass absolute GIT_DIR to exec commands
2017-11-02sequencer: pass absolute GIT_DIR to exec commandsLibravatar Jacob Keller1-0/+11
When we replaced the old shell script based interactive rebase in commmit 18633e1a22a6 ("rebase -i: use the rebase--helper builtin", 2017-02-09) we introduced a regression of functionality in that the GIT_DIR would be sent to the environment of the exec command as-is. This generally meant that it would be passed as "GIT_DIR=.git", which causes problems for any exec command that wants to run git commands in a subdirectory. This isn't a very large regression, since it is not that likely that the exec command will run a git command, and even less likely that it will need to do so in a subdir. This regression was discovered by a build system which uses git-describe to find the current version of the build system, and happened to do so from the src/ sub directory of the project. Fix this by passing in the absolute path of the git directory into the child environment. Signed-off-by: Jacob Keller <jacob.keller@gmail.com> Acked-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>