Commit Graph

212 Commits

Author SHA1 Message Date
Junio C Hamano
6e31fa9cc2 Merge branch 'js/rebase-p-tests'
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-13 22:37:24 +09:00
Junio C Hamano
b3635c15b5 Merge branch 'sg/test-rebase-editor-fix'
* sg/test-rebase-editor-fix:
  t3404-rebase-interactive: test abbreviated commands
2018-11-06 15:50:22 +09:00
Johannes Schindelin
11aad46432 tests: optionally skip git rebase -p tests
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-02 11:27:30 +09:00
Johannes Schindelin
6c8fbae619 t3404: decouple some test cases from outcomes of previous test cases
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 c42abfe785 (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-02 11:27:30 +09:00
Junio C Hamano
b49ef560ed Merge branch 'ag/rebase-i-in-c'
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-11-02 11:04:53 +09:00
Johannes Sixt
8c64bc9420 t3404-rebase-interactive: test abbreviated commands
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-10-29 09:50:20 +09:00
Junio C Hamano
4af130af0c Merge branch 'en/sequencer-empty-edit-result-aborts'
"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-24 10:30:52 -07:00
Elijah Newren
a3ec9eaf38 sequencer: fix --allow-empty-message behavior, make it smarter
In commit b00bf1c9a8 ("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 b00bf1c9a8 (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-13 13:25:08 -07:00
Junio C Hamano
064e0b2d4c Merge branch 'pw/rebase-i-author-script-fix'
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-09-04 14:31:38 -07:00
Alban Gruin
b97e187364 rebase -i: rewrite complete_action() in C
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-29 13:38:18 -07:00
Junio C Hamano
986c518107 Merge branch 'sg/test-must-be-empty'
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-27 14:33:43 -07:00
SZEDER Gábor
1c5e94f459 tests: use 'test_must_be_empty' instead of 'test_cmp <empty> <out>'
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-21 11:48:36 -07:00
Junio C Hamano
1bc505b476 Merge branch 'es/rebase-i-author-script-fix'
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-17 13:09:54 -07:00
Alban Gruin
d078c39106 t3404: todo list with commented-out commands only aborts
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-10 11:56:22 -07:00
Phillip Wood
4aa5ff9409 sequencer: fix quoting in write_author_script
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-07 14:52:07 -07:00
Junio C Hamano
cd3f067815 Merge branch 'bc/sequencer-export-work-tree-as-well'
"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-02 15:30:45 -07:00
Junio C Hamano
7a135475d3 Merge branch 'es/test-fixes'
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-08-02 15:30:40 -07:00
Eric Sunshine
67f16e3d3f sequencer: fix "rebase -i --root" corrupting author header timestamp
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-31 11:32:12 -07:00
Eric Sunshine
0f16c09aae sequencer: fix "rebase -i --root" corrupting author header timezone
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-31 11:31:42 -07:00
Eric Sunshine
ca3e1826a0 sequencer: fix "rebase -i --root" corrupting author header
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-31 11:31:08 -07:00
Junio C Hamano
18a86f32ab Merge branch 'jc/t3404-one-shot-export-fix'
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-24 14:50:49 -07:00
Junio C Hamano
0ce5a698c6 Merge branch 'en/rebase-consistency'
"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-24 14:50:43 -07:00
brian m. carlson
ab5e67d751 sequencer: pass absolute GIT_WORK_TREE to exec commands
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-16 11:16:45 -07:00
Junio C Hamano
650161a277 t3404: fix use of "VAR=VAL cmd" with a shell function
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-12 13:31:57 -07:00
Eric Sunshine
0590ff26c4 t: use test_write_lines() instead of series of 'echo' commands
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-07-03 12:38:04 -07:00
Elijah Newren
b00bf1c9a8 git-rebase: make --allow-empty-message the default
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-27 11:23:22 -07:00
Phillip Wood
a9279c6785 sequencer: do not squash 'reword' commits when we hit conflicts
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-19 10:26:41 -07:00
Todd Zullinger
878810552b t3404: check root commit in 'rebase -i --root reword root commit'
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-19 09:14:33 -07:00
Johannes Schindelin
76fda6ebbc rebase --root: fix amending root commit messages
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-18 09:36:58 -07:00
Todd Zullinger
3a36ca0881 rebase --root: demonstrate a bug while amending root commit messages
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-06-18 09:22:18 -07:00
Johannes Schindelin
21d0764c82 rebase -i --root: let the sequencer handle even the initial part
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
6a6bc5bdc4 (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-05-06 13:21:58 +09:00
Junio C Hamano
deb9845a0a Merge branch 'ps/test-chmtime-get'
Test cleanup.

* ps/test-chmtime-get:
  t/helper: 'test-chmtime (--get|-g)' to print only the mtime
2018-04-25 13:29:00 +09:00
Junio C Hamano
27f25845cf Merge branch 'nd/combined-test-helper'
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-11 13:09:56 +09:00
Junio C Hamano
62c0fd46a8 Merge branch 'ps/contains-id-error-message'
"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-10 16:28:20 +09:00
Paul-Sebastian Ungureanu
decf711fc1 t/helper: 'test-chmtime (--get|-g)' to print only the mtime
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-04-09 11:33:19 +09:00
Nguyễn Thái Ngọc Duy
0e496492d2 t/helper: merge test-chmtime into test-tool
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-27 08:45:47 -07:00
Paul-Sebastian Ungureanu
3bb0923f06 parse-options: do not show usage upon invalid option value
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-22 12:10:08 -07:00
Junio C Hamano
9ca488c04b Merge branch 'nd/rebase-show-current-patch'
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-03-06 14:54:02 -08:00
Junio C Hamano
0f57f731ea Merge branch 'pw/sequencer-in-process-commit'
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-13 13:39:15 -08:00
Nguyễn Thái Ngọc Duy
fbd7a23237 rebase: introduce and use pseudo-ref REBASE_HEAD
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-12 14:07:59 -08:00
Nguyễn Thái Ngọc Duy
66335298a4 rebase: add --show-current-patch
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>
2018-02-12 14:07:59 -08:00
Johannes Schindelin
ed1e52822e sequencer: assign only free()able strings to gpg_sign
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-22 13:34:28 -08:00
Liam Beguin
1795993488 t3404: add test case for abbreviated commands
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-12-05 10:20:51 -08:00
Phillip Wood
60b6158886 t3404: check intermediate squash messages
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-11 03:33:26 +09:00
Junio C Hamano
e4db47e6a0 Merge branch 'jk/rebase-i-exec-gitdir-fix'
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-06 13:11:28 +09:00
Jacob Keller
09d7b6c6fa sequencer: pass absolute GIT_DIR to exec commands
When we replaced the old shell script based interactive rebase in
commmit 18633e1a22 ("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>
2017-11-02 10:53:23 +09:00
Johannes Schindelin
47d4ac019b t3404: relax rebase.missingCommitsCheck tests
These tests were a bit anal about the *exact* warning/error message
printed by git rebase. But those messages are intended for the *end
user*, therefore it does not make sense to test so rigidly for the
*exact* wording.

In the following, we will reimplement the missing commits check in
the sequencer, with slightly different words.

So let's just test for the parts in the warning/error message that
we *really* care about, nothing more, nothing less.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-27 15:35:05 -07:00
Junio C Hamano
5452224710 Merge branch 'pw/rebase-i-regression-fix-tests'
Fix a recent regression to "git rebase -i" and add tests that would
have caught it and others.

* pw/rebase-i-regression-fix-tests:
  t3420: fix under GETTEXT_POISON build
  rebase: add more regression tests for console output
  rebase: add regression tests for console output
  rebase -i: add test for reflog message
  sequencer: print autostash messages to stderr
2017-06-30 13:45:21 -07:00
Phillip Wood
1ceb9dfab7 rebase -i: add test for reflog message
Check that the reflog message written to the branch reflog when the
rebase is completed is correct

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-19 11:19:00 -07:00
Ævar Arnfjörð Bjarmason
0d75bfe67b tests: fix tests broken under GETTEXT_POISON=YesPlease
The GETTEXT_POISON=YesPlease compile-time testing option added in my
bb946bba76 ("i18n: add GETTEXT_POISON to simulate unfriendly
translator", 2011-02-22) has been slowly bitrotting as strings have
been marked for translation, and new tests have been added without
running it.

I brought this up on the list ("[BUG] test suite broken with
GETTEXT_POISON=YesPlease", [1]) asking whether this mode was useful at
all anymore. At least one person occasionally uses it, and Lars
Schneider offered to change one of the the Travis builds to run in
this mode, so fix up the failing ones.

My test setup runs most of the tests, with the notable exception of
skipping all the p4 tests, so it's possible that there's still some
lurking regressions I haven't fixed.

1. <CACBZZX62+acvi1dpkknadTL827mtCm_QesGSZ=6+UnyeMpg8+Q@mail.gmail.com>

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-11 18:44:38 +09:00