Commit Graph

345 Commits

Author SHA1 Message Date
Liam Beguin
0cce4a2756 rebase -i -x: add exec commands via the rebase--helper
Recent work on `git-rebase--interactive` aims to convert shell code to
C. Even if this is most likely not a big performance enhancement, let's
convert it too since a coming change to abbreviate command names
requires it to be updated.

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
Junio C Hamano
f68337d1ed Merge branch 'ad/rebase-i-serie-typofix'
* ad/rebase-i-serie-typofix:
  rebase -i: fix comment typo
2017-11-15 12:14:33 +09:00
Adam Dinwoodie
3dc5433fd5 rebase -i: fix comment typo
Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-11-09 12:32:25 +09:00
Junio C Hamano
5f3108b7b6 Merge branch 'js/rebase-i-final'
The final batch to "git rebase -i" updates to move more code from
the shell script to C.

* js/rebase-i-final:
  rebase -i: rearrange fixup/squash lines using the rebase--helper
  t3415: test fixup with wrapped oneline
  rebase -i: skip unnecessary picks using the rebase--helper
  rebase -i: check for missing commits in the rebase--helper
  t3404: relax rebase.missingCommitsCheck tests
  rebase -i: also expand/collapse the SHA-1s via the rebase--helper
  rebase -i: do not invent onelines when expanding/collapsing SHA-1s
  rebase -i: remove useless indentation
  rebase -i: generate the script via rebase--helper
  t3415: verify that an empty instructionFormat is handled as before
2017-10-03 15:42:47 +09:00
Phillip Wood
9b6d7a6245 rebase -i: honor --rerere-autoupdate
Interactive rebase was ignoring '--rerere-autoupdate'. Fix this by
reading it appropriate file when restoring the sequencer state for an
interactive rebase and passing '--rerere-autoupdate' to merge and
cherry-pick when rebasing with '--preserve-merges'.

Reported-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-08-02 15:16:09 -07:00
Johannes Schindelin
c44a4c650c rebase -i: rearrange fixup/squash lines using the rebase--helper
This operation has quadratic complexity, which is especially painful
on Windows, where shell scripts are *already* slow (mainly due to the
overhead of the POSIX emulation layer).

Let's reimplement this with linear complexity (using a hash map to
match the commits' subject lines) for the common case; Sadly, the
fixup/squash feature's design neglected performance considerations,
allowing arbitrary prefixes (read: `fixup! hell` will match the
commit subject `hello world`), which means that we are stuck with
quadratic performance in the worst case.

The reimplemented logic also happens to fix a bug where commented-out
lines (representing empty patches) were dropped by the previous code.

While at it, clarify how the fixup/squash feature works in `git rebase
-i`'s man page.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-07-27 15:35:06 -07:00
Johannes Schindelin
cdac2b01ff rebase -i: skip unnecessary picks using the rebase--helper
In particular on Windows, where shell scripts are even more expensive
than on MacOSX or Linux, it makes sense to move a loop that forks
Git at least once for every line in the todo list into a builtin.

Note: The original code did not try to skip unnecessary picks of root
commits but punts instead (probably --root was not considered common
enough of a use case to bother optimizing). We do the same, for now.

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
Johannes Schindelin
943999493f rebase -i: check for missing commits in the rebase--helper
In particular on Windows, where shell scripts are even more expensive
than on MacOSX or Linux, it makes sense to move a loop that forks
Git at least once for every line in the todo list into a builtin.

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
Johannes Schindelin
3546c8d927 rebase -i: also expand/collapse the SHA-1s via the rebase--helper
This is crucial to improve performance on Windows, as the speed is now
mostly dominated by the SHA-1 transformation (because it spawns a new
rev-parse process for *every* line, and spawning processes is pretty
slow from Git for Windows' MSYS2 Bash).

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
Johannes Schindelin
1f4044af7f rebase -i: do not invent onelines when expanding/collapsing SHA-1s
To avoid problems with short SHA-1s that become non-unique during the
rebase, we rewrite the todo script with short/long SHA-1s before and
after letting the user edit the script. Since SHA-1s are not intuitive
for humans, rebase -i also provides the onelines (commit message
subjects) in the script, purely for the user's convenience.

It is very possible to generate a todo script via different means than
rebase -i and then to let rebase -i run with it; In this case, these
onelines are not required.

And this is where the expand/collapse machinery has a bug: it *expects*
that oneline, and failing to find one reuses the previous SHA-1 as
"oneline".

It was most likely an oversight, and made implementation in the (quite
limiting) shell script language less convoluted. However, we are about
to reimplement performance-critical parts in C (and due to spawning a
git.exe process for every single line of the todo script, the
expansion/collapsing of the SHA-1s *is* performance-hampering on
Windows), therefore let's fix this bug to make cross-validation with the
C version of that functionality possible.

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
Johannes Schindelin
4b8b65d706 rebase -i: remove useless indentation
The commands used to be indented, and it is nice to look at, but when we
transform the SHA-1s, the indentation is removed. So let's do away with it.

For the moment, at least: when we will use the upcoming rebase--helper
to transform the SHA-1s, we *will* keep the indentation and can
reintroduce it. Yet, to be able to validate the rebase--helper against
the output of the current shell script version, we need to remove the
extra indentation.

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
Johannes Schindelin
62db524779 rebase -i: generate the script via rebase--helper
The first step of an interactive rebase is to generate the so-called "todo
script", to be stored in the state directory as "git-rebase-todo" and to
be edited by the user.

Originally, we adjusted the output of `git log <options>` using a simple
sed script. Over the course of the years, the code became more
complicated. We now use shell scripting to edit the output of `git log`
conditionally, depending whether to keep "empty" commits (i.e. commits
that do not change any files).

On platforms where shell scripting is not native, this can be a serious
drag. And it opens the door for incompatibilities between platforms when
it comes to shell scripting or to Unix-y commands.

Let's just re-implement the todo script generation in plain C, using the
revision machinery directly.

This is substantially faster, improving the speed relative to the
shell script version of the interactive rebase from 2x to 3x on Windows.

Note that the rearrange_squash() function in git-rebase--interactive
relied on the fact that we set the "format" variable to the config setting
rebase.instructionFormat. Relying on a side effect like this is no good,
hence we explicitly perform that assignment (possibly again) in
rearrange_squash().

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
5840eb9d14 doc: replace more gmane links
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-09 21:13:13 -07:00
Johannes Schindelin
18633e1a22 rebase -i: use the rebase--helper builtin
Now that the sequencer learned to process a "normal" interactive rebase,
we use it. The original shell script is still used for "non-normal"
interactive rebases, i.e. when --root or --preserve-merges was passed.

Please note that the --root option (via the $squash_onto variable) needs
special handling only for the very first command, hence it is still okay
to use the helper upon continue/skip.

Also please note that the --no-ff setting is volatile, i.e. when the
interactive rebase is interrupted at any stage, there is no record of
it. Therefore, we have to pass it from the shell script to the
rebase--helper.

Note: the test t3404 had to be adjusted because the the error messages
produced by the sequencer comply with our current convention to start with
a lower-case letter.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-09 14:55:40 -08:00
Junio C Hamano
5c924692cd Merge branch 'jk/rebase-i-squash-count-fix'
"git rebase -i" with a recent update started showing an incorrect
count when squashing more than 10 commits.

* jk/rebase-i-squash-count-fix:
  rebase--interactive: count squash commits above 10 correctly
2017-01-18 15:12:13 -08:00
Jeff King
356b8ecff1 rebase--interactive: count squash commits above 10 correctly
We generate the squash commit message incrementally running
a sed script once for each commit. It parses "This is
a combination of <N> commits" from the first line of the
existing message, adds one to <N>, and uses the result as
the number of our current message.

Since f2d17068fd (i18n: rebase-interactive: mark comments of
squash for translation, 2016-06-17), the first line may be
localized, and sed uses a pretty liberal regex, looking for:

  /^#.*([0-9][0-9]*)/

The "[0-9][0-9]*" tries to match double digits, but it
doesn't quite work.  The first ".*" is greedy, so if you
have:

  This is a combination of 10 commits.

it will eat up "This is a combination of 1", leaving "0" to
match the first "[0-9]" digit, and then skipping the
optional match of "[0-9]*".

As a result, the count resets every 10 commits, and a
15-commit squash would end up as:

  # This is a combination of 5 commits.
  # This is the 1st commit message:
  ...
  # This is the commit message #2:
  ... and so on ..
  # This is the commit message #10:
  ...
  # This is the commit message #1:
  ...
  # This is the commit message #2:
  ... etc, up to 5 ...

We can fix this by making the ".*" less greedy. Instead of
depending on ".*?" working portably, we can just limit the
match to non-digit characters, which accomplishes the same
thing.

Reported-by: Brandon Tolsch <btolsch@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-07 19:26:53 -08:00
Junio C Hamano
eef32a0653 Merge branch 'va/i18n-even-more'
* va/i18n-even-more:
  i18n: fix misconversion in shell scripts
2016-12-27 00:11:45 -08:00
Junio C Hamano
acefe2be28 i18n: fix misconversion in shell scripts
An earlier series that was merged at 2703572b3a ("Merge branch
'va/i18n-even-more'", 2016-07-13) failed to use $(eval_gettext
"string with \$variable interpolation") and instead used gettext in
a few places, and ended up showing the variable names in the
message, e.g.

    $ git submodule
    fatal: $program_name cannot be used without a working tree.

Catch these mistakes with

    $ git grep -n '[^_]gettext .*\\\$'

and fix them all to use eval_gettext instead.

Reported-by: Josh Bleecher Snyder
Acked-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-20 09:36:23 -08:00
Junio C Hamano
bd53f38d52 Merge branch 'js/rebase-i-commentchar-fix'
"git rebase -i" did not work well with core.commentchar
configuration variable for two reasons, both of which have been
fixed.

* js/rebase-i-commentchar-fix:
  rebase -i: handle core.commentChar=auto
  stripspace: respect repository config
  rebase -i: highlight problems with core.commentchar
2016-11-23 11:23:17 -08:00
Johannes Schindelin
882cd23777 rebase -i: handle core.commentChar=auto
When 84c9dc2 (commit: allow core.commentChar=auto for character auto
selection, 2014-05-17) extended the core.commentChar functionality to
allow for the value 'auto', it forgot that rebase -i was already taught to
handle core.commentChar, and in turn forgot to let rebase -i handle that
new value gracefully.

Reported by Taufiq Hoven.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-11-21 11:00:41 -08:00
Junio C Hamano
cec5f0bf80 Merge branch 'rt/rebase-i-broken-insn-advise' into maint
When "git rebase -i" is given a broken instruction, it told the
user to fix it with "--edit-todo", but didn't say what the step
after that was (i.e. "--continue").

* rt/rebase-i-broken-insn-advise:
  rebase -i: improve advice on bad instruction lines
2016-09-29 16:49:46 -07:00
Junio C Hamano
e25e6f3947 Merge branch 'jk/rebase-i-drop-ident-check' into maint
Even when "git pull --rebase=preserve" (and the underlying "git
rebase --preserve") can complete without creating any new commit
(i.e. fast-forwards), it still insisted on having a usable ident
information (read: user.email is set correctly), which was less
than nice.  As the underlying commands used inside "git rebase"
would fail with a more meaningful error message and advice text
when the bogus ident matters, this extra check was removed.

* jk/rebase-i-drop-ident-check:
  rebase-interactive: drop early check for valid ident
2016-09-29 16:49:41 -07:00
Junio C Hamano
104a93a329 Merge branch 'rt/rebase-i-broken-insn-advise'
When "git rebase -i" is given a broken instruction, it told the
user to fix it with "--edit-todo", but didn't say what the step
after that was (i.e. "--continue").

* rt/rebase-i-broken-insn-advise:
  rebase -i: improve advice on bad instruction lines
2016-09-26 16:09:21 -07:00
Junio C Hamano
ae1ae600db Merge branch 'jk/rebase-i-drop-ident-check'
Even when "git pull --rebase=preserve" (and the underlying "git
rebase --preserve") can complete without creating any new commit
(i.e. fast-forwards), it still insisted on having a usable ident
information (read: user.email is set correctly), which was less
than nice.  As the underlying commands used inside "git rebase"
would fail with a more meaningful error message and advice text
when the bogus ident matters, this extra check was removed.

* jk/rebase-i-drop-ident-check:
  rebase-interactive: drop early check for valid ident
2016-09-21 15:15:28 -07:00
Alex Henrie
7c406bd8a7 git-rebase--interactive: fix English grammar
Signed-off-by: Alex Henrie <alexhenrie24@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-08 12:17:03 -07:00
Ralf Thielow
37875b4733 rebase -i: improve advice on bad instruction lines
If we found bad instruction lines in the instruction sheet
of interactive rebase, we give the user advice on how to
fix it.  However, we don't tell the user what to do afterwards.
Give the user advice to run 'git rebase --continue' after
the fix.

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-09-07 11:56:05 -07:00
Jean-Noel Avila
8aa6dc1d9e i18n: fix git rebase interactive commit messages
For proper i18n, the logic cannot embed english specific processing.

Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-08-24 08:43:27 -07:00
Junio C Hamano
6b9114c649 Merge branch 'js/rebase-i-progress-tidy'
Regression fix for an i18n topic already in 'master'.

* js/rebase-i-progress-tidy:
  rebase-interactive: trim leading whitespace from progress count
2016-08-08 14:48:38 -07:00
Jeff King
1e461c4f1f rebase-interactive: drop early check for valid ident
Since the very inception of interactive-rebase in 1b1dce4
(Teach rebase an interactive mode, 2007-06-25), there has
been a preemptive check, before looking at any commits, to
see whether the user has a valid name/email combination.

This is convenient, because it means that we abort the
operation before even beginning (rather than just
complaining that we are unable to pick a particular commit).

However, it does the wrong thing when the rebase does not
actually need to generate any new commits (e.g., a
fast-forward with no commits to pick, or one where the base
stays the same, and we just pick the same commits without
rewriting anything). In this case it may complain about the
lack of ident, even though one would not be needed to
complete the operation.

This may seem like mere nit-picking, but because interactive
rebase underlies the "preserve-merges" rebase, somebody who
has set "pull.rebase" to "preserve" cannot make even a
fast-forward pull without a valid ident, as we bail before
even realizing the fast-forward nature.

This commit drops the extra ident check entirely. This means
we rely on individual commands that generate commit objects
to complain. So we will continue to notice and prevent cases
that actually do create commits, but with one important
difference: we fail while actually executing the "pick"
operations, and leave the rebase in a conflicted, half-done
state.

In some ways this is less convenient, but in some ways it is
more so; the user can then manually commit or even "git
rebase --continue" after setting up their ident (or
providing it as a one-off on the command line).

Reported-by: Dakota Hawkins <dakotahawkins@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-29 15:47:06 -07:00
Johannes Sixt
d949751859 rebase-interactive: trim leading whitespace from progress count
Interactive rebase uses 'wc -l' to write the current patch number
in a progress report. Some implementations of 'wc -l' produce spaces
before the number, leading to ugly output such as

  Rebasing (     3/8)

Remove the spaces using a trivial arithmetic evaluation.

Before 9588c52 (i18n: rebase-interactive: mark strings for
translation) this was not a problem because printf was used to
generate the text. Since that commit, the count is interpolated
directly from a shell variable into the text, where the spaces
remain. The total number of patches does not have this problem
even though it is interpolated from a shell variable in the same
manner, because the variable is set by an arithmetic evaluation.

Later in the script, there is a virtually identical case where
leading spaces are trimmed, but it uses a pattern substitution:

todocount=$(git stripspace --strip-comments <"$todo" | wc -l)
todocount=${todocount##* }

I did not choose this idiom because it adds a line of code, and
there is already an arithmetic evaluation in the vicinity of the
line that is changed here.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-07-28 13:22:46 -07:00
Junio C Hamano
5eb1e9f1a0 Merge branch 'ps/rebase-i-auto-unstash-upon-abort'
"git rebase -i --autostash" did not restore the auto-stashed change
when the operation was aborted.

* ps/rebase-i-auto-unstash-upon-abort:
  rebase -i: restore autostash on abort
2016-07-13 11:24:17 -07:00
Junio C Hamano
2703572b3a Merge branch 'va/i18n-even-more'
More markings of messages for i18n, with updates to various tests
to pass GETTEXT_POISON tests.

One patch from the original submission dropped due to conflicts
with jk/upload-pack-hook, which is still in flux.

* va/i18n-even-more: (38 commits)
  t5541: become resilient to GETTEXT_POISON
  i18n: branch: mark comment when editing branch description for translation
  i18n: unmark die messages for translation
  i18n: submodule: escape shell variables inside eval_gettext
  i18n: submodule: join strings marked for translation
  i18n: init-db: join message pieces
  i18n: remote: allow translations to reorder message
  i18n: remote: mark URL fallback text for translation
  i18n: standardise messages
  i18n: sequencer: add period to error message
  i18n: merge: change command option help to lowercase
  i18n: merge: mark messages for translation
  i18n: notes: mark options for translation
  i18n: notes: mark strings for translation
  i18n: transport-helper.c: change N_() call to _()
  i18n: bisect: mark strings for translation
  t5523: use test_i18ngrep for negation
  t4153: fix negated test_i18ngrep call
  t9003: become resilient to GETTEXT_POISON
  tests: unpack-trees: update to use test_i18n* functions
  ...
2016-07-13 11:24:10 -07:00
Patrick Steinhardt
33ba9c648b rebase -i: restore autostash on abort
When we abort an interactive rebase we do so by calling
`die_abort`, which cleans up after us by removing the rebase
state directory. If the user has requested to use the autostash
feature, though, the state directory may also contain a reference
to the autostash, which will now be deleted.

Fix the issue by trying to re-apply the autostash in `die_abort`.
This will also handle the case where the autostash does not apply
cleanly anymore by recording it in a user-visible stash.

Reported-by: Daniel Hahler <git@thequod.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-29 09:51:00 -07:00
Junio C Hamano
3ec9150a8c Merge branch 'em/newer-freebsd-shells-are-fine-with-returns'
Comments about misbehaving FreeBSD shells have been clarified with
the version number (9.x and before are broken, newer ones are OK).

* em/newer-freebsd-shells-are-fine-with-returns:
  rebase: update comment about FreeBSD /bin/sh
2016-06-27 09:56:52 -07:00
Vasco Almeida
9588c52b75 i18n: rebase-interactive: mark strings for translation
Mark strings in git-rebase--interactive.sh for translation. There is no
need to source git-sh-i18n since git-rebase.sh already does so.

Add git-rebase--interactive.sh to LOCALIZED_SH in Makefile in order to
enable extracting strings marked for translation by xgettext.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 15:45:48 -07:00
Vasco Almeida
f2d17068fd i18n: rebase-interactive: mark comments of squash for translation
Mark comment messages of squash/fixup file ($squash_msg) for
translation.

Helper functions this_nth_commit_message and skip_nth_commit_message
replace the previous method of making the comment messages (such as
"This is the 2nd commit message:") aided by nth_string helper function.
This step was taken as a workaround to enabled translation of entire
sentences. However, doesn't change any text seen in English by the user,
except for string "The first commit's message is:" which was changed to
match the style of other instances.

The test t3404-rebase-interactive.sh resorts to set_fake_editor which
didn't account for GETTEXT_POISON. Fix it by assuming success when we
find dummy gettext poison output where was supposed to find the first
comment line "This is a combination of $count commits.".

For that same message, use plural aware eval_ngettext instead of
eval_gettext, since other languages have more complex plural forms.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 15:45:48 -07:00
Vasco Almeida
b8fc9e43a7 i18n: rebase-interactive: mark here-doc strings for translation
Use pipe to send gettext output to git stripspace instead of the
original method of using shell here-document, because command
substitution '$(...)' would not take place inside the here-documents.
The exception is the case of the last here-document redirecting to cat,
in which commands substitution works and, thus, is preserved in this
commit.

t3404: adapt test to the strings newly marked for translation
Test t3404-rebase-interactive.sh would fail under GETTEXT_POISON unless
using test_i18ngrep.

Add eval_ngettext fallback functions to be called when running, for
instance, under GETTEXT_POISON. Otherwise, tests would fail under
GETTEXT_POISON, or other build that doesn't support the GNU gettext,
because that function could not be found.

Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 15:45:48 -07:00
Ed Maste
9b35cadc2c rebase: update comment about FreeBSD /bin/sh
Commit 9f50d32 introduced a fix for FreeBSD /bin/sh misbehaviour
when dot-sourcing a file containing "return" statements outside of
any function, from a function in another shell script. That issue
affects FreeBSD 9.x, and is not present in the /bin/sh in FreeBSD
10.3 and later. Update the comment to clarify this.

The example from 9f50d32's commit message produces the expected output
on FreeBSD 10.3 and -CURRENT (the upcoming 11.0):

% sh script1.sh
only this line should show
%

Signed-off-by: Ed Maste <emaste@freebsd.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-06-17 11:04:38 -07:00
Junio C Hamano
7267404dc5 Merge branch 'js/rebase-i-dedup-call-to-rerere'
"git rebase -i", after it fails to auto-resolve the conflict, had
an unnecessary call to "git rerere" from its very early days, which
was spotted recently; the call has been removed.

* js/rebase-i-dedup-call-to-rerere:
  rebase -i: remove an unnecessary 'rerere' invocation
2016-06-03 14:38:01 -07:00
Johannes Sixt
7063693d51 rebase -i: remove an unnecessary 'rerere' invocation
Interactive rebase uses 'git cherry-pick' and 'git merge' to replay
commits. Both invoke the 'rerere' machinery when they fail due to merge
conflicts. Note that all code paths with these two commands also invoke
the shell function die_with_patch when the commands fail.

Since commit 629716d2 ("rerere: do use multiple variants") the second
operation of the rerere machinery can be observed by a duplicated
message "Recorded preimage for 'file'". This second operation records
the same preimage as the first one and, hence, only wastes cycles.
Remove the 'git rerere' invocation from die_with_patch.

Shell function die_with_patch can be called after the failure of
"git commit", too, which also calls into the rerere machinery, but it
does so only after a successful commit to record the resolution.
Therefore, it is wrong to call 'git rerere' from die_with_patch after
"git commit" fails.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-31 13:47:18 -07:00
Junio C Hamano
dca05bb591 Merge branch 'jk/test-z-n-unquoted'
t9xxx series has been updated primarily for readability, while
fixing small bugs in it.  A few scripted Porcelains have also been
updated to fix possible bugs around their use of "test -z" and
"test -n".

* jk/test-z-n-unquoted:
  always quote shell arguments to test -z/-n
  t9103: modernize test style
  t9107: switch inverted single/double quotes in test
  t9107: use "return 1" instead of "exit 1"
  t9100,t3419: enclose all test code in single-quotes
  t/lib-git-svn: drop $remote_git_svn and $git_svn_id
2016-05-23 14:54:35 -07:00
Junio C Hamano
fd704b16f1 Merge branch 'jk/rebase-interative-eval-fix'
Portability enhancement for "rebase -i" to help platforms whose
shell does not like "for i in <empty>" (which is not POSIX-kosher).

* jk/rebase-interative-eval-fix:
  rebase--interactive: avoid empty list in shell for-loop
2016-05-17 14:38:35 -07:00
Jeff King
268ef4d3d0 always quote shell arguments to test -z/-n
In shell code like:

  test -z $foo
  test -n $foo

that does not quote its arguments, it's easy to think that
it is actually looking at the contents of $foo in each case.
But if $foo is empty, then "test" does not see any argument
at all! The results are quite subtle.

POSIX specifies that test's behavior depends on the number
of arguments it sees, and if $foo is empty, it sees only
one. The behavior in this case is:

  1 argument: Exit true (0) if $1 is not null; otherwise,
              exit false.

So in the "-z $foo" case, if $foo is empty, then we check
that "-z" is non-null, and it returns success. Which happens
to match what we expected.  But for "-n $foo", if $foo is
empty, we'll see that "-n" is non-null and still return
success. That's the opposite of what we intended!

Furthermore, if $foo contains whitespace, we'll end up with
more than 2 arguments. The results in this case are
generally unspecified (unless the first part of $foo happens
to be a valid binary operator, in which case the results are
specified but certainly not what we intended).

And on top of this, even though "test -z $foo" _should_ work
for the empty case, some older shells (reportedly ksh88)
complain about the missing argument.

So let's make sure we consistently quote our variable
arguments to "test". After this patch, the results of:

  git grep 'test -[zn] [^"]'

are empty.

Reported-by: Armin Kunaschik <megabreit@googlemail.com>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-14 10:37:29 -07:00
Jeff King
8e98b35f87 rebase--interactive: avoid empty list in shell for-loop
The $strategy_opts variable contains a space-separated list
of strategy options, each individually shell-quoted. To loop
over each, we "unwrap" them by doing an eval like:

  eval '
    for opt in '"$strategy_opts"'
    do
       ...
    done
  '

Note the quoting that means we expand $strategy_opts inline
in the code to be evaluated (which is the right thing
because we want the IFS-split and de-quoting). If the
variable is empty, however, we ask the shell to eval the
following code:

  for opt in
  do
     ...
  done

without anything between "in" and "do".  Most modern shells
are happy to treat that like a noop, but reportedly ksh88 on
AIX considers it a syntax error. So let's catch the case
that the variable is empty and skip the eval altogether
(since we know the loop would be a noop anyway).

Reported-by: Armin Kunaschik <megabreit@googlemail.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-05-10 14:11:27 -07:00
Junio C Hamano
5c788e7746 Merge branch 'rt/rebase-i-shorten-stop-report'
The commit object name reported when "rebase -i" stops has been
shortened.

* rt/rebase-i-shorten-stop-report:
  rebase-i: print an abbreviated hash when stop for editing
2016-04-13 14:12:30 -07:00
Ralf Thielow
14c793e8f4 rebase-i: print an abbreviated hash when stop for editing
The message that is shown when rebase-i stops for editing prints
the full hash of the commit where it stopped which makes the message
overflow to the next line on smaller terminal windows.  Print an
abbreviated hash instead.

Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-28 10:49:40 -07:00
Junio C Hamano
2df13639e7 Merge branch 'jc/sane-grep'
Recent versions of GNU grep is pickier than before to decide if a
file is "binary" and refuse to give line-oriented hits when we
expect it to, unless explicitly told with "-a" option.  As our
scripted Porcelains use sane_grep wrapper for line-oriented data,
even when the line may contain non-ASCII payload we took from
end-user data, use "grep -a" to implement sane_grep wrapper when
using an implementation of "grep" that takes the "-a" option.

* jc/sane-grep:
  rebase-i: clarify "is this commit relevant?" test
  sane_grep: pass "-a" if grep accepts it
2016-03-16 13:16:54 -07:00
Junio C Hamano
214123c645 rebase-i: clarify "is this commit relevant?" test
While I was checking all the call sites of sane_grep and sane_egrep,
I noticed this one is somewhat strangely written.  The lines in the
file sane_grep works on all begin with 40-hex object name, so there
is no real risk of confusing "test $(...) = ''" by finding something
that begins with a dash, but using the status from sane_grep makes
it a lot clearer what is going on.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-03-10 15:35:48 -08:00
Fabian Ruch
040fd39e67 rebase -i: remember merge options beyond continue actions
If the user explicitly specified a merge strategy or strategy
options, continue to use that strategy/option after
"rebase --continue".  Add a test of the corrected behavior.

If --merge is specified or implied by -s or -X, then "strategy and
"strategy_opts" are set to values from which "strategy_args" can be
derived; otherwise they are set to empty strings.  Either way,
their values are propagated from one step of an interactive rebase
to the next via state files.

"do_merge", on the other hand, is *not* propagated to later steps of
an interactive rebase.  Therefore, making the initialization of
"strategy_args" conditional on "do_merge" being set prevents later
steps of an interactive rebase from setting it correctly.

Luckily, we don't need the "do_merge" guard at all.  If the rebase
was started without --merge, then "strategy" and "strategy_opts"
are both the empty string, which results in "strategy_args" also
being set to the empty string, which is just what we want in that
situation.  So remove the "do_merge" guard and derive
"strategy_args" from "strategy" and "strategy_opts" every time.

Reported-by: Diogo de Campos <campos@esss.com.br>
Signed-off-by: Fabian Ruch <bafain@gmail.com>
Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2015-12-11 12:44:44 -08:00
Fredrik Medley
fe9394ad3e rebase-i-exec: Allow space in SHELL_PATH
On Windows, when Git is installed under "C:\Program Files\Git",
SHELL_PATH will include a space. Fix "git rebase --interactive --exec"
so that it works with spaces in SHELL_PATH.

Signed-off-by: Fredrik Medley <fredrik.medley@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
2015-11-13 17:51:39 -05:00