[RFC PATCH v2 5/7] git-rebase.txt: document behavioral inconsistencies between modes
There are a variety of aspects that are common to all rebases regardless of which backend is in use; however, the behavior for these different aspects varies in ways that could surprise users. (In fact, it's not clear -- to me at least -- that these differences were even desirable or intentional.) Document these differences. Signed-off-by: Elijah Newren --- Documentation/git-rebase.txt | 57 1 file changed, 57 insertions(+) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index adccc15284..0dbfab06d0 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -555,6 +555,63 @@ Other incompatible flag pairs: * --preserve-merges && --rebase-merges * --rebase-merges && --strategy +BEHAVIORAL INCONSISTENCIES +-- + + * --no-ff vs. --force-rebase + +These options are actually identical, though their description +leads people to believe they might not be. + + * empty commits: + +am-based rebase will drop any "empty" commits, whether the +commit started empty (had no changes relative to its parent to +start with) or ended empty (all changes were already applied +upstream in other commits). + +merge-based rebase does the same. + +interactive-based rebase will by default drop commits that +started empty and halt if it hits a commit that ended up empty. +The --keep-empty option exists for interactive rebases to allow +it to keep commits that started empty. + + * empty commit messages: + +am-based rebase will silently apply commits with empty commit +messages. + +merge-based and interactive-based rebases will by default halt +on any such commits. The --allow-empty-message option exists to +allow interactive-based rebases to apply such commits without +halting. + + * directory rename detection: + +merge-based and interactive-based rebases work fine with +directory rename detection. am-based rebases sometimes do not. + +git-am tries to avoid a full three way merge, instead calling +git-apply. That prevents us from detecting renames at all, +which may defeat the directory rename detection. There is a +fallback, though; if the initial git-apply fails and the user +has specified the -3 option, git-am will fall back to a three +way merge. However, git-am lacks the necessary information to +do a "real" three way merge. Instead, it has to use +build_fake_ancestor() to get a merge base that is missing files +whose rename may have been important to detect for directory +rename detection to function. + +Since am-based rebases work by first generating a bunch of +patches (which no longer record what the original commits were +and thus don't have the necessary info from which we can find a +real merge-base), and then calling git-am, this implies that +am-based rebases will not always successfully detect directory +renames either. merged-based rebases (rebase -m) and +cherry-pick-based rebases (rebase -i) are not affected by this +shortcoming. + include::merge-strategies.txt[] NOTES -- 2.18.0.rc2.1.g5453d3f70b.dirty
[RFC PATCH v2 4/7] git-rebase: error out when incompatible options passed
git rebase has three different types: am, merge, and interactive, all of
which are implemented in terms of separate scripts. am builds on git-am,
merge builds on git-merge-recursive, and interactive builds on
git-cherry-pick. We make use of features in those lower-level commands in
the different rebase types, but those features don't exist in all of the
lower level commands so we have a range of incompatibilities. Previously,
we just accepted nearly any argument and silently ignored whichever ones
weren't implemented for the type of rebase specified. Change this so the
incompatibilities are documented, included in the testsuite, and tested
for at runtime with an appropriate error message shown.
Some exceptions I left out:
* --merge and --interactive are technically incompatible since they are
supposed to run different underlying scripts, but with a few small
changes, --interactive can do everything that --merge can. In fact,
I'll shortly be sending another patch to remove git-rebase--merge and
reimplement it on top of git-rebase--interactive.
* One could argue that --interactive and --quiet are incompatible since
--interactive doesn't implement a --quiet mode (perhaps since
cherry-pick itself does not implement one). However, the interactive
mode is more quiet than the other modes in general with progress
messages, so one could argue that it's already quiet.
Signed-off-by: Elijah Newren
---
git-rebase.sh | 17 +
t/t3422-rebase-incompatible-options.sh | 10 +-
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index bf71b7fa20..5f891214fa 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -503,6 +503,23 @@ then
git_format_patch_opt="$git_format_patch_opt --progress"
fi
+if test -n "$git_am_opt"; then
+ incompatible_opts=$(echo "$git_am_opt" | sed -e 's/ -q//')
+ if test -n "$interactive_rebase"
+ then
+ if test -n "$incompatible_opts"
+ then
+ die "$(gettext "error: cannot combine interactive
options (--interactive, --exec, --rebase-merges, --preserve-merges,
--keep-empty, --root + --onto) with am options ($incompatible_opts)")"
+ fi
+ fi
+ if test -n "$do_merge"; then
+ if test -n "$incompatible_opts"
+ then
+ die "$(gettext "error: cannot combine merge options
(--merge, --strategy, --strategy-option) with am options ($incompatible_opts)")"
+ fi
+ fi
+fi
+
if test -n "$signoff"
then
test -n "$preserve_merges" &&
diff --git a/t/t3422-rebase-incompatible-options.sh
b/t/t3422-rebase-incompatible-options.sh
index 04cdae921b..66a83363bf 100755
--- a/t/t3422-rebase-incompatible-options.sh
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -34,27 +34,27 @@ test_expect_success 'setup' '
test_run_rebase () {
opt=$1
shift
- test_expect_failure "$opt incompatible with --merge" "
+ test_expect_success "$opt incompatible with --merge" "
git checkout B^0 &&
test_must_fail git rebase $opt --merge A
"
- test_expect_failure "$opt incompatible with --strategy=ours" "
+ test_expect_success "$opt incompatible with --strategy=ours" "
git checkout B^0 &&
test_must_fail git rebase $opt --strategy=ours A
"
- test_expect_failure "$opt incompatible with --strategy-option=ours" "
+ test_expect_success "$opt incompatible with --strategy-option=ours" "
git checkout B^0 &&
test_must_fail git rebase $opt --strategy=ours A
"
- test_expect_failure "$opt incompatible with --interactive" "
+ test_expect_success "$opt incompatible with --interactive" "
git checkout B^0 &&
test_must_fail git rebase $opt --interactive A
"
- test_expect_failure "$opt incompatible with --exec" "
+ test_expect_success "$opt incompatible with --exec" "
git checkout B^0 &&
test_must_fail git rebase $opt --exec 'true' A
"
--
2.18.0.rc2.1.g5453d3f70b.dirty
[RFC PATCH v2 6/7] git-rebase.txt: address confusion between --no-ff vs --force-rebase
rebase was taught the --force-rebase option in commit b2f82e05de ("Teach
rebase to rebase even if upstream is up to date", 2009-02-13). This flag
worked for the am and merge backends, but wasn't a valid option for the
interactive backend.
rebase was taught the --no-ff option for interactive rebases in commit
b499549401cb ("Teach rebase the --no-ff option.", 2010-03-24), to do the
exact same thing as --force-rebase does for non-interactive rebases. This
commit explicitly documented the fact that --force-rebase was incompatible
with --interactive, though it made --no-ff a synonym for --force-rebase
for non-interactive rebases. The choice of a new option was based on the
fact that "force rebase" didn't sound like an appropriate term for the
interactive machinery.
In commit 6bb4e485cff8 ("rebase: align variable names", 2011-02-06), the
separate parsing of command line options in the different rebase scripts
was removed, and whether on accident or because the author noticed that
these options did the same thing, the options became synonyms and both
were accepted by all three rebase types.
In commit 2d26d533a012 ("Documentation/git-rebase.txt: -f forces a rebase
that would otherwise be a no-op", 2014-08-12), which reworded the
description of the --force-rebase option, the (no-longer correct) sentence
stating that --force-rebase was incompatible with --interactive was
finally removed.
Finally, as explained at
https://public-inbox.org/git/98279912-0f52-969d-44a6-222420393...@xiplink.com
In the original discussion around this option [1], at one point I
proposed teaching rebase--interactive to respect --force-rebase
instead of adding a new option [2]. Ultimately --no-ff was chosen as
the better user interface design [3], because an interactive rebase
can't be "forced" to run.
We have accepted both --no-ff and --force-rebase as full synonyms for all
three rebase types for over seven years. Documenting them differently
and in ways that suggest they might not be quite synonyms simply leads to
confusion. Adjust the documentation to match reality.
Signed-off-by: Elijah Newren
---
Documentation/git-rebase.txt | 35 ++-
1 file changed, 10 insertions(+), 25 deletions(-)
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 0dbfab06d0..7a2ed9efdc 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -337,16 +337,18 @@ See also INCOMPATIBLE OPTIONS below.
+
See also INCOMPATIBLE OPTIONS below.
--f::
+--no-ff::
--force-rebase::
- Force a rebase even if the current branch is up to date and
- the command without `--force` would return without doing anything.
+-f::
+ Individually replay all rebased commits instead of fast-forwarding
+ over the unchanged ones. This ensures that the entire history of
+ the rebased branch is composed of new commits.
+
-You may find this (or --no-ff with an interactive rebase) helpful after
-reverting a topic branch merge, as this option recreates the topic branch with
-fresh commits so it can be remerged successfully without needing to "revert
-the reversion" (see the
-link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for
details).
+You may find this helpful after reverting a topic branch merge, as this option
+recreates the topic branch with fresh commits so it can be remerged
+successfully without needing to "revert the reversion" (see the
+link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for
+details).
--fork-point::
--no-fork-point::
@@ -498,18 +500,6 @@ See also INCOMPATIBLE OPTIONS below.
with care: the final stash application after a successful
rebase might result in non-trivial conflicts.
---no-ff::
- With --interactive, cherry-pick all rebased commits instead of
- fast-forwarding over the unchanged ones. This ensures that the
- entire history of the rebased branch is composed of new commits.
-+
-Without --interactive, this is a synonym for --force-rebase.
-+
-You may find this helpful after reverting a topic branch merge, as this option
-recreates the topic branch with fresh commits so it can be remerged
-successfully without needing to "revert the reversion" (see the
-link:howto/revert-a-faulty-merge.html[revert-a-faulty-merge How-To] for
details).
-
INCOMPATIBLE OPTIONS
@@ -558,11 +548,6 @@ Other incompatible flag pairs:
BEHAVIORAL INCONSISTENCIES
--
- * --no-ff vs. --force-rebase
-
-These options are actually identical, though their description
-leads people to believe they might not be.
-
* empty commits:
am-based rebase will drop any "empty" commits, whether the
--
2.18.0.rc2.1.g5453d3f70b.dirty
[RFC PATCH v2 7/7] git-rebase: make --allow-empty-message the default
am-based rebases already apply commits with an empty commit message without requiring the user to specify an extra flag. Make merge-based and interactive-based rebases behave the same. Signed-off-by: Elijah Newren --- Documentation/git-rebase.txt | 10 -- git-rebase.sh | 2 +- t/t3404-rebase-interactive.sh | 7 --- t/t3405-rebase-malformed.sh | 11 +++ 4 files changed, 8 insertions(+), 22 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 7a2ed9efdc..a5608f481f 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -562,16 +562,6 @@ BEHAVIORAL INCONSISTENCIES The --keep-empty option exists for interactive rebases to allow it to keep commits that started empty. - * empty commit messages: - -am-based rebase will silently apply commits with empty commit -messages. - -merge-based and interactive-based rebases will by default halt -on any such commits. The --allow-empty-message option exists to -allow interactive-based rebases to apply such commits without -halting. - * directory rename detection: merge-based and interactive-based rebases work fine with diff --git a/git-rebase.sh b/git-rebase.sh index 5f891214fa..bf033da4e5 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -95,7 +95,7 @@ rebase_cousins= preserve_merges= autosquash= keep_empty= -allow_empty_message= +allow_empty_message=--allow-empty-message signoff= test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t case "$(git config --bool commit.gpgsign)" in diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index c65826ddac..f84fa63b15 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -553,15 +553,16 @@ test_expect_success '--continue tries to commit, even for "edit"' ' ' test_expect_success 'aborted --continue does not squash commits after "edit"' ' + test_when_finished "git rebase --abort" && old=$(git rev-parse HEAD) && test_tick && set_fake_editor && FAKE_LINES="edit 1" git rebase -i HEAD^ && echo "edited again" > file7 && git add file7 && - test_must_fail env FAKE_COMMIT_MESSAGE=" " git rebase --continue && - test $old = $(git rev-parse HEAD) && - git rebase --abort + echo all the things >>conflict && + test_must_fail git rebase --continue && + test $old = $(git rev-parse HEAD) ' test_expect_success 'auto-amend only edited commits after "edit"' ' diff --git a/t/t3405-rebase-malformed.sh b/t/t3405-rebase-malformed.sh index cb7c6de84a..da94dddc86 100755 --- a/t/t3405-rebase-malformed.sh +++ b/t/t3405-rebase-malformed.sh @@ -77,19 +77,14 @@ test_expect_success 'rebase commit with diff in message' ' ' test_expect_success 'rebase -m commit with empty message' ' - test_must_fail git rebase -m master empty-message-merge && - git rebase --abort && - git rebase -m --allow-empty-message master empty-message-merge + git rebase -m master empty-message-merge ' test_expect_success 'rebase -i commit with empty message' ' git checkout diff-in-message && set_fake_editor && - test_must_fail env FAKE_COMMIT_MESSAGE=" " FAKE_LINES="reword 1" \ - git rebase -i HEAD^ && - git rebase --abort && - FAKE_COMMIT_MESSAGE=" " FAKE_LINES="reword 1" \ - git rebase -i --allow-empty-message HEAD^ + env FAKE_COMMIT_MESSAGE=" " FAKE_LINES="reword 1" \ + git rebase -i HEAD^ ' test_done -- 2.18.0.rc2.1.g5453d3f70b.dirty
[RFC PATCH v2 2/7] git-rebase.sh: update help messages a bit
signoff is not specific to the am-backend. Also, re-order a few options to make like things (e.g. strategy and strategy-option) be near each other. --- git-rebase.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 40be59ecc4..bf71b7fa20 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -20,23 +20,23 @@ onto=! rebase onto given branch instead of upstream r,rebase-merges? try to rebase merges instead of skipping them p,preserve-merges! try to recreate merges instead of ignoring them s,strategy=! use the given merge strategy +X,strategy-option=! pass the argument through to the merge strategy no-ff! cherry-pick all commits, even if unchanged +f,force-rebase!cherry-pick all commits, even if unchanged m,merge! use merging strategies to rebase i,interactive! let the user edit the list of commits to rebase x,exec=! add exec lines after each commit of the editable list k,keep-empty preserve empty commits during rebase allow-empty-message allow rebasing commits with empty messages -f,force-rebase!force rebase even if branch is up to date -X,strategy-option=! pass the argument through to the merge strategy stat! display a diffstat of what changed upstream n,no-stat! do not show diffstat of what changed upstream verify allow pre-rebase hook to run rerere-autoupdate allow rerere to update index with resolved conflicts root! rebase all reachable commits up to the root(s) autosquash move commits that begin with squash!/fixup! under -i +signoffadd a Signed-off-by: line to each commit committer-date-is-author-date! passed to 'git am' ignore-date! passed to 'git am' -signoffpassed to 'git am' whitespace=! passed to 'git apply' ignore-whitespace! passed to 'git apply' C=!passed to 'git apply' -- 2.18.0.rc2.1.g5453d3f70b.dirty
[RFC PATCH v2 1/7] git-rebase.txt: document incompatible options
git rebase has many options that only work with one of its three backends. It also has a few other pairs of incompatible options. Document these. Signed-off-by: Elijah Newren --- Documentation/git-rebase.txt | 84 1 file changed, 76 insertions(+), 8 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 0e20a66e73..adccc15284 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -243,11 +243,15 @@ leave out at most one of A and B, in which case it defaults to HEAD. --keep-empty:: Keep the commits that do not change anything from its parents in the result. ++ +See also INCOMPATIBLE OPTIONS below. --allow-empty-message:: By default, rebasing commits with an empty message will fail. This option overrides that behavior, allowing commits with empty messages to be rebased. ++ +See also INCOMPATIBLE OPTIONS below. --skip:: Restart the rebasing process by skipping the current patch. @@ -271,6 +275,8 @@ branch on top of the branch. Because of this, when a merge conflict happens, the side reported as 'ours' is the so-far rebased series, starting with , and 'theirs' is the working branch. In other words, the sides are swapped. ++ +See also INCOMPATIBLE OPTIONS below. -s :: --strategy=:: @@ -280,8 +286,10 @@ other words, the sides are swapped. + Because 'git rebase' replays each commit from the working branch on top of the branch using the given strategy, using -the 'ours' strategy simply discards all patches from the , +the 'ours' strategy simply empties all patches from the , which makes little sense. ++ +See also INCOMPATIBLE OPTIONS below. -X :: --strategy-option=:: @@ -289,6 +297,8 @@ which makes little sense. This implies `--merge` and, if no strategy has been specified, `-s recursive`. Note the reversal of 'ours' and 'theirs' as noted above for the `-m` option. ++ +See also INCOMPATIBLE OPTIONS below. -S[]:: --gpg-sign[=]:: @@ -324,6 +334,8 @@ which makes little sense. and after each change. When fewer lines of surrounding context exist they all must match. By default no context is ever ignored. ++ +See also INCOMPATIBLE OPTIONS below. -f:: --force-rebase:: @@ -355,19 +367,22 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`. --whitespace=:: These flag are passed to the 'git apply' program (see linkgit:git-apply[1]) that applies the patch. - Incompatible with the --interactive option. ++ +See also INCOMPATIBLE OPTIONS below. --committer-date-is-author-date:: --ignore-date:: These flags are passed to 'git am' to easily change the dates of the rebased commits (see linkgit:git-am[1]). - Incompatible with the --interactive option. ++ +See also INCOMPATIBLE OPTIONS below. --signoff:: Add a Signed-off-by: trailer to all the rebased commits. Note that if `--interactive` is given then only commits marked to be - picked, edited or reworded will have the trailer added. Incompatible - with the `--preserve-merges` option. + picked, edited or reworded will have the trailer added. ++ +See also INCOMPATIBLE OPTIONS below. -i:: --interactive:: @@ -378,6 +393,8 @@ default is `--no-fork-point`, otherwise the default is `--fork-point`. The commit list format can be changed by setting the configuration option rebase.instructionFormat. A customized instruction format will automatically have the long commit hash prepended to the format. ++ +See also INCOMPATIBLE OPTIONS below. -r:: --rebase-merges[=(rebase-cousins|no-rebase-cousins)]:: @@ -404,7 +421,7 @@ It is currently only possible to recreate the merge commits using the `recursive` merge strategy; Different merge strategies can be used only via explicit `exec git merge -s [...]` commands. + -See also REBASING MERGES below. +See also REBASING MERGES and INCOMPATIBLE OPTIONS below. -p:: --preserve-merges:: @@ -415,6 +432,8 @@ See also REBASING MERGES below. This uses the `--interactive` machinery internally, but combining it with the `--interactive` option explicitly is generally not a good idea unless you know what you are doing (see BUGS below). ++ +See also INCOMPATIBLE OPTIONS below. -x :: --exec :: @@ -437,6 +456,8 @@ squash/fixup series. + This uses the `--interactive` machinery internally, but it can be run without an explicit `--interactive`. ++ +See also INCOMPATIBLE OPTIONS below. --root:: Rebase all commits reachable from , instead of @@ -447,6 +468,8 @@ without an explicit `--interactive`. When used together with both --onto and --preserve-merges, 'all' root commits will be rewritten to have as parent instead. ++ +See also INCOMPATIBLE OPTIONS below. --autosquash:: --no-autosquash:: @@ -461,11 +484,11 @@ without an explicit `--interactive`. too.
[RFC PATCH v2 3/7] t3422: new testcases for checking when incompatible options passed
git rebase is split into three types: am, merge, and interactive. Various
options imply different types, and which mode we are using determine which
sub-script (git-rebase--$type) is executed to finish the work. Not all
options work with all types, so add tests for combinations where we expect
to receive an error rather than having options be silently ignored.
Signed-off-by: Elijah Newren
---
t/t3422-rebase-incompatible-options.sh | 69 ++
1 file changed, 69 insertions(+)
create mode 100755 t/t3422-rebase-incompatible-options.sh
diff --git a/t/t3422-rebase-incompatible-options.sh
b/t/t3422-rebase-incompatible-options.sh
new file mode 100755
index 00..04cdae921b
--- /dev/null
+++ b/t/t3422-rebase-incompatible-options.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+
+test_description='test if rebase detects and aborts on incompatible options'
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_seq 2 9 >foo &&
+ git add foo &&
+ git commit -m orig &&
+
+ git branch A &&
+ git branch B &&
+
+ git checkout A &&
+ test_seq 1 9 >foo &&
+ git add foo &&
+ git commit -m A &&
+
+ git checkout B &&
+ # This is indented with HT SP HT.
+ echo " foo();" >>foo &&
+ git add foo &&
+ git commit -m B
+'
+
+#
+# Rebase has lots of useful options like --whitepsace=fix, which are
+# actually all built in terms of flags to git-am. Since neither
+# --merge nor --interactive (nor any options that imply those two) use
+# git-am, using them together will result in flags like --whitespace=fix
+# being ignored. Make sure rebase warns the user and aborts instead.
+#
+
+test_run_rebase () {
+ opt=$1
+ shift
+ test_expect_failure "$opt incompatible with --merge" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --merge A
+ "
+
+ test_expect_failure "$opt incompatible with --strategy=ours" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --strategy=ours A
+ "
+
+ test_expect_failure "$opt incompatible with --strategy-option=ours" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --strategy=ours A
+ "
+
+ test_expect_failure "$opt incompatible with --interactive" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --interactive A
+ "
+
+ test_expect_failure "$opt incompatible with --exec" "
+ git checkout B^0 &&
+ test_must_fail git rebase $opt --exec 'true' A
+ "
+
+}
+
+test_run_rebase --whitespace=fix
+test_run_rebase --ignore-whitespace
+test_run_rebase --committer-date-is-author-date
+test_run_rebase -C4
+
+test_done
--
2.18.0.rc2.1.g5453d3f70b.dirty
[RFC PATCH v2 0/7] Document/fix/warn about rebase incompatibilities and inconsistences
git-rebase has lots of options that are mutually incompatible. Even among aspects of its behavior that is common to all rebase types, it has a number of inconsistencies. This series tries to document, fix, and/or warn users about many of these. I have a much higher than average expectation that folks will object to some of these patches. I've tried to divide them up so that any parts we decide to drop or redo can be more easily excised. No branch-diff; because it's a significant re-work; instead I'll comment briefly on the individual patches... Elijah Newren (7): git-rebase.txt: document incompatible options Both Dscho (on a related patch series) and Phillip suggested changing the documentation to avoid implementational details. I instead made a separate section with sets of incompatible options...but it still mentions the different backends while doing so. Does that seem alright? git-rebase.sh: update help messages a bit Minor tweaks to `git rebase -h` output. t3422: new testcases for checking when incompatible options passed The one unmodified patch from the first round. git-rebase: error out when incompatible options passed Almost the same as the first round, except: * Documentation pulled into a separate patch (patch 1) * $() instead of `` git-rebase.txt: document behavioral inconsistencies between modes Add another section to the documentation for aspects that ideally should be common between all modes but are handled differently. git-rebase.txt: address confusion between --no-ff vs --force-rebase This came up on the list not that long ago; fix the documentation. git-rebase: make --allow-empty-message the default Address the easiest of the inconsistencies, assuming the am-based backend has the correct default and the merge-based and interactive-based backends are the ones that need to change. Documentation/git-rebase.txt | 154 - git-rebase.sh | 25 +++- t/t3404-rebase-interactive.sh | 7 +- t/t3405-rebase-malformed.sh| 11 +- t/t3422-rebase-incompatible-options.sh | 69 +++ 5 files changed, 224 insertions(+), 42 deletions(-) create mode 100755 t/t3422-rebase-incompatible-options.sh -- 2.18.0.rc2.1.g5453d3f70b.dirty
[PATCH v2] 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. For any of those cases we want a new commit, so
we should not set the to_amend flag.
Signed-off-by: Elijah Newren
---
Differences since v1 (Thanks to Eric Sunshine for the suggestions):
* Add test_when_finished "reset_rebase" calls
* Remove unnecessary word from description of test
sequencer.c | 2 +-
t/t3423-rebase-reword.sh | 48
2 files changed, 49 insertions(+), 1 deletion(-)
create mode 100755 t/t3423-rebase-reword.sh
diff --git a/sequencer.c b/sequencer.c
index cca968043e..9e6d1ee368 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3217,7 +3217,7 @@ static int pick_commits(struct todo_list *todo_list,
struct replay_opts *opts)
} else if (res && is_rebase_i(opts) && item->commit)
return res | error_with_patch(item->commit,
item->arg, item->arg_len, opts, res,
- item->command == TODO_REWORD);
+ 0);
} else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(item->arg + item->arg_len);
int saved = *end_of_arg;
diff --git a/t/t3423-rebase-reword.sh b/t/t3423-rebase-reword.sh
new file mode 100755
index 00..6963750794
--- /dev/null
+++ b/t/t3423-rebase-reword.sh
@@ -0,0 +1,48 @@
+#!/bin/sh
+
+test_description='git rebase interactive with rewording'
+
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_expect_success 'setup' '
+ test_commit master file-1 test &&
+
+ git checkout -b stuff &&
+
+ test_commit feature_a file-2 aaa &&
+ test_commit feature_b file-2 ddd
+'
+
+test_expect_success 'reword without issues functions as intended' '
+ test_when_finished "reset_rebase" &&
+
+ git checkout stuff^0 &&
+
+ set_fake_editor &&
+ FAKE_LINES="pick 1 reword 2" FAKE_COMMIT_MESSAGE="feature_b_reworded" \
+ git rebase -i -v master &&
+
+ test "$(git log -1 --format=%B)" = "feature_b_reworded" &&
+ test $(git rev-list --count HEAD) = 3
+'
+
+test_expect_success 'reword after a conflict preserves commit' '
+ test_when_finished "reset_rebase" &&
+
+ git checkout stuff^0 &&
+
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="reword 2" \
+ git rebase -i -v master &&
+
+ git checkout --theirs file-2 &&
+ git add file-2 &&
+ FAKE_COMMIT_MESSAGE="feature_b_reworded" git rebase --continue &&
+
+ test "$(git log -1 --format=%B)" = "feature_b_reworded" &&
+ test $(git rev-list --count HEAD) = 2
+'
+
+test_done
--
2.18.0.rc2.1.g5453d3f70b.dirty
[PATCH v2 signed off] doc: fix typos in documentation and release notes
Signed-off-by: Karthikeyan Singaravelan
---
Documentation/RelNotes/1.7.11.7.txt | 2 +-
Documentation/RelNotes/2.17.0.txt | 2 +-
Documentation/RelNotes/2.18.0.txt | 2 +-
Documentation/diff-options.txt | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/RelNotes/1.7.11.7.txt
b/Documentation/RelNotes/1.7.11.7.txt
index e7e79d999bd38..e743a2a8e46eb 100644
--- a/Documentation/RelNotes/1.7.11.7.txt
+++ b/Documentation/RelNotes/1.7.11.7.txt
@@ -25,7 +25,7 @@ Fixes since v1.7.11.6
references" nor "Reload" did not update what is shown as the
contents of it, when the user overwrote the tag with "git tag -f".
- * "git for-each-ref" did not currectly support more than one --sort
+ * "git for-each-ref" did not correctly support more than one --sort
option.
* "git log .." errored out saying it is both rev range and a path
diff --git a/Documentation/RelNotes/2.17.0.txt
b/Documentation/RelNotes/2.17.0.txt
index d6db0e19cf17b..c2cf891f71adf 100644
--- a/Documentation/RelNotes/2.17.0.txt
+++ b/Documentation/RelNotes/2.17.0.txt
@@ -342,7 +342,7 @@ Fixes since v2.16
validate the data and connected-ness of objects in the received
pack; the code to perform this check has been taught about the
narrow clone's convention that missing objects that are reachable
- from objects in a pack that came from a promissor remote is OK.
+ from objects in a pack that came from a promisor remote is OK.
* There was an unused file-scope static variable left in http.c when
building for versions of libCURL that is older than 7.19.4, which
diff --git a/Documentation/RelNotes/2.18.0.txt
b/Documentation/RelNotes/2.18.0.txt
index 7c59bd92fbd99..1eb13ece53600 100644
--- a/Documentation/RelNotes/2.18.0.txt
+++ b/Documentation/RelNotes/2.18.0.txt
@@ -324,7 +324,7 @@ Fixes since v2.17
after giving an error message.
(merge 3bb0923f06 ps/contains-id-error-message later to maint).
- * "diff-highlight" filter (in contrib/) learned to undertand "git log
+ * "diff-highlight" filter (in contrib/) learned to understand "git log
--graph" output better.
(merge 4551fbba14 jk/diff-highlight-graph-fix later to maint).
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f466600972f86..bfa3808e49cc0 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -133,7 +133,7 @@ These parameters can also be set individually with
`--stat-width=`,
as file creations or deletions ("new" or "gone", optionally "+l"
if it's a symlink) and mode changes ("+x" or "-x" for adding
or removing executable bit respectively) in diffstat. The
- information is put betwen the filename part and the graph
+ information is put between the filename part and the graph
part. Implies `--stat`.
--numstat::
--
https://github.com/git/git/pull/510
Re: [PATCH] doc: fix typos in documentation and release notes
On Sat, Jun 16, 2018 at 11:36 PM Karthikeyan wrote: > On Sun, Jun 17, 2018, 8:55 AM Eric Sunshine wrote: >> Please sign-off[1] your patch so it can be included in the project. > > Thanks. I am a beginner using Gmail and I have used SubmitToGit for > this. Is it possible to do this using SubmitToGit or should I do git > commit --amend -s to sign off and make a force push to submit it > again from SubmitToGit as newer one? Amending the commit with sign-off and re-submit should work fine. Alternately, you can reply to this email thread with your sign off in-line, like this: Signed-off-by: Your Name
Re: Is NO_ICONV misnamed or is it broken?
On Sat, Jun 16, 2018 at 10:57 PM Christian Couder wrote: > On Fri, Jun 15, 2018 at 12:47 AM, Mahmoud Al-Qudsi > wrote: > > ~> make NO_ICONV=1 > > # ommitted > > LINK git-credential-store > > /usr/bin/ld: cannot find -liconv > I think 597c9cc540 (Flatten tools/ directory to make build procedure > simpler., 2005-09-07) which introduces NEEDS_LIBICONV is even older > than the commit that introduced NO_ICONV (see above), so you might > want to play with NEEDS_LIBICONV too and see if it works better for > you. > > I CC'ed the people involved in related commits. Maybe they can give > you a better answer. It might also help if you could tell us on which > OS/Platform and perhaps for which purpose you want to compile Git. For completeness, for those reading this thread, a patch fixing this issue has already been sent[1] to the list. [1]: https://public-inbox.org/git/20180615022503.34111-1-sunsh...@sunshineco.com/
Re: [BUG] git-rebase: reword squashes commits in case of merge-conflicts
On Sat, Jun 16, 2018 at 12:08 PM Elijah Newren wrote: > Subject: [PATCH] sequencer: do not squash 'reword' commits when we hit > conflicts > [...] > Signed-off-by: Elijah Newren > --- > diff --git a/t/t3423-rebase-reword.sh b/t/t3423-rebase-reword.sh > @@ -0,0 +1,44 @@ > +test_expect_success 'reword comes after a conflict preserves commit' ' s/comes// > + git checkout stuff^0 && > + > + set_fake_editor && > + test_must_fail env FAKE_LINES="reword 2" \ > + git rebase -i -v master && If some command fails between here and "git rebase --continue" below, then the test will exit with an in-progress (dangling) rebase, which could confuse tests added later to this script. I wonder, therefore, if it would make sense to insert the following cleanup before "git rebase -i": test_when_finished "reset_rebase" && > + git checkout --theirs file-2 && > + git add file-2 && > + FAKE_COMMIT_MESSAGE="feature_b_reworded" git rebase --continue && > + > + test "$(git log -1 --format=%B)" = "feature_b_reworded" && > + test $(git rev-list --count HEAD) = 2 > +'
Re: [PATCH] doc: fix typos in documentation and release notes
On Sat, Jun 16, 2018 at 2:08 PM Xtreak wrote: > doc: fix typos in documentation and release notes Thanks for the patch. All the fixes look "obviously correct". Please sign-off[1] your patch so it can be included in the project. [1]: https://github.com/git/git/blob/v2.8.1/Documentation/SubmittingPatches#L234-L286
Re: Is NO_ICONV misnamed or is it broken?
Hi, On Fri, Jun 15, 2018 at 12:47 AM, Mahmoud Al-Qudsi wrote: > Hello list, > > With regards to the Makefile define/variable `NO_ICONV` - the Makefile > comments imply that it should be used if "your libc doesn't properly support > iconv," which could mean anything from "a patch will be applied" to "iconv > won't be used." b6e56eca8a (Allow building Git in systems without iconv, 2006-02-16) which added NO_ICONV says: Systems using some uClibc versions do not properly support iconv stuff. This patch allows Git to be built on those systems by passing NO_ICONV=YesPlease to make. The only drawback is mailinfo won't do charset conversion in those systems. > Based off the name of the varibale, the assumption is that iconv is an > optional dependency that can be omitted if compiled with NO_ICONV. However, in > practice attempting to compile git with `make ... NO_ICONV=1` and libiconv not > installed results in linker errors as follows: > > ``` > ~> make clean > # omitted > ~> make NO_ICONV=1 > # ommitted > LINK git-credential-store > /usr/bin/ld: cannot find -liconv > cc: error: linker command failed with exit code 1 (use -v to see invocation) > gmake: *** [Makefile:2327: git-credential-store] Error 1 > ``` Yeah, this might be an issue with the Makefile options. > Am I misunderstanding the intended behavior when NO_ICONV is defined (i.e. it > does not remove the dependency on libiconv) or is this a bug and iconv should > not, in fact, be required? It's difficult to tell from reading the comments and commit messages. I think 597c9cc540 (Flatten tools/ directory to make build procedure simpler., 2005-09-07) which introduces NEEDS_LIBICONV is even older than the commit that introduced NO_ICONV (see above), so you might want to play with NEEDS_LIBICONV too and see if it works better for you. (I understand that "you might want to play with such and such other options" is perhaps not as helpful as what you expected, but I previously tried to tighten the way we handle dependencies in the Makefile and it was considered "too heavy handed". So yeah, we consider it ok if people have to tinker a bit when they want to build Git.) I CC'ed the people involved in related commits. Maybe they can give you a better answer. It might also help if you could tell us on which OS/Platform and perhaps for which purpose you want to compile Git. Best, Christian.
git@vger.kernel.org
Three tests in 't7406-submodule-update' contain broken &&-chains, but since they are all in subshells, chain-lint couldn't notice them. Signed-off-by: SZEDER Gábor --- t/t7406-submodule-update.sh | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh index 6f083c4d68..9e0d31700e 100755 --- a/t/t7406-submodule-update.sh +++ b/t/t7406-submodule-update.sh @@ -65,7 +65,7 @@ test_expect_success 'setup a submodule tree' ' git commit -m "none" ) && git clone . recursivesuper && - ( cd recursivesuper + ( cd recursivesuper && git submodule add ../super super ) ' @@ -245,13 +245,13 @@ test_expect_success 'submodule update --remote should fetch upstream changes wit ( cd super && git submodule update --remote --force submodule && - git -C submodule log -1 --oneline >actual - git -C ../submodule log -1 --oneline master >expect + git -C submodule log -1 --oneline >actual && + git -C ../submodule log -1 --oneline master >expect && test_cmp expect actual && git checkout -b test-branch && git submodule update --remote --force submodule && - git -C submodule log -1 --oneline >actual - git -C ../submodule log -1 --oneline test-branch >expect + git -C submodule log -1 --oneline >actual && + git -C ../submodule log -1 --oneline test-branch >expect && test_cmp expect actual && git checkout master && git branch -d test-branch && @@ -891,7 +891,7 @@ test_expect_success 'submodule update properly revives a moved submodule' ' rm -rf submodule2 && mkdir -p "moved/sub module" && git update-index --add --cacheinfo 16 $H "moved/sub module" && -git config -f .gitmodules submodule.submodule2.path "moved/sub module" +git config -f .gitmodules submodule.submodule2.path "moved/sub module" && git commit -am "post move" && git submodule update && git status | sed "s/$H2/XXX/" >actual && -- 2.18.0.rc0.207.ga6211da864
Re: [PATCH 2/3] submodule: ensure core.worktree is set after update
> +static int connect_gitdir_workingtree(int argc, const char **argv, const
> char *prefix)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + const char *name, *path;
> + char *sm_gitdir;
> +
> + if (argc != 3)
> + BUG("submodule--helper connect-gitdir-workingtree
> ");
So this aborts when it's invoked with the wrong number of cmdline
arguments.
> +
> + name = argv[1];
> + path = argv[2];
> +
> + strbuf_addf(&sb, "%s/modules/%s", get_git_dir(), name);
> + sm_gitdir = absolute_pathdup(sb.buf);
> +
> + connect_work_tree_and_git_dir(path, sm_gitdir, 0);
> +
> + strbuf_release(&sb);
> + free(sm_gitdir);
> +
> + return 0;
> +}
> +
> #define SUPPORT_SUPER_PREFIX (1<<0)
>
> struct cmd_struct {
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 78073cd87d1..6596a77c5ef 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -615,6 +615,11 @@ cmd_update()
> die "$(eval_gettext "Unable to find current
> \${remote_name}/\${branch} revision in submodule path '\$sm_path'")"
> fi
>
> + if ! $(git config -f "$(git rev-parse
> --git-common-dir)/modules/$name/config" core.worktree) 2>/dev/null
> + then
> + git submodule--helper connect-gitdir-workingtree $name
> $sm_path
The path to the submodule, $sm_path, may contain spaces, so it must
be quoted.
I'm sure you would have noticed this already, had you checked this
'submodule--helper's exit code :) In t7406 the test 'submodule update
properly revives a moved submodule' does update a submodule with a
space in its name, and thus executes 'submodule--helper' with one more
argument than expected, causing it to abort, but since there is no
error checking, 'git submodule update' continues anyway, and in the
end the test tends to pass[1].
I think it would be prudent to check the exit code of all
'submodule--helper' executions.
[1] I wrote "tends to", because e.g. on Travis CI the aborting
'submodule--helper' often dumps its core, and the extra 'core'
file shows up in the output of 'git status' and 'test_cmp' notices
it.
> + fi
> +
> if test "$subsha1" != "$sha1" || test -n "$force"
> then
> subforce=$force
> --
> 2.18.0.rc1.244.gcf134e6275-goog
>
>
Re: [PATCH 0/2] rebase --root: fix `reword` on a root commit
Hi Johannes, Johannes Schindelin via GitGitGadget wrote: > From: GitGitGadget > > Todd Zullinger reported this bug in > https://public-inbox.org/git/20180615043111.gs3...@zaya.teonanacatl.net/: > when calling git rebase --root and trying to reword the > root commit's message, a BUG is reported. > > This fixes that. > > IMO the bug fix is trivial enough to qualify for inclusion into v2.18.0, > still. It does indeed fix the issue. I agree it would be nice to see it in 2.18.0. As a fix for a minor regression introduced in this cycle, that seems reasonable. > Johannes Schindelin (1): > rebase --root: fix amending root commit messages > > Todd Zullinger (1): > rebase --root: demonstrate a bug while amending root commit messages > > sequencer.c | 2 +- > t/t3404-rebase-interactive.sh | 9 + > 2 files changed, 10 insertions(+), 1 deletion(-) > > > base-commit: 68372c88794aba15f853542008cda39def768372 -- Todd ~~ I don't mean to sound cold, or cruel, or vicious, but I am, so that's the way it comes out. -- Bill Hicks
[PATCH 2/2] rebase --root: fix amending root commit messages
From: Johannes Schindelin
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
---
sequencer.c | 2 +-
t/t3404-rebase-interactive.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index cca968043..4034c0461 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -784,7 +784,7 @@ static int run_git_commit(const char *defmsg, struct
replay_opts *opts,
struct child_process cmd = CHILD_PROCESS_INIT;
const char *value;
- if (flags & CREATE_ROOT_COMMIT) {
+ if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) {
struct strbuf msg = STRBUF_INIT, script = STRBUF_INIT;
const char *author = is_rebase_i(opts) ?
read_author_ident(&script) : NULL;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ca94c688d..e500d7c32 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -971,7 +971,7 @@ test_expect_success 'rebase -i --root fixup root commit' '
test 0 = $(git cat-file commit HEAD | grep -c ^parent\ )
'
-test_expect_failure 'rebase -i --root reword root commit' '
+test_expect_success 'rebase -i --root reword root commit' '
test_when_finished "test_might_fail git rebase --abort" &&
git checkout -b reword-root-branch master &&
set_fake_editor &&
--
2.17.0.windows.1
[PATCH 1/2] rebase --root: demonstrate a bug while amending root commit messages
From: Todd Zullinger 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 Signed-off-by: Johannes Schindelin --- t/t3404-rebase-interactive.sh | 9 + 1 file changed, 9 insertions(+) diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index c65826dda..ca94c688d 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -971,6 +971,15 @@ test_expect_success 'rebase -i --root fixup root commit' ' test 0 = $(git cat-file commit HEAD | grep -c ^parent\ ) ' +test_expect_failure 'rebase -i --root reword root commit' ' + test_when_finished "test_might_fail git rebase --abort" && + git checkout -b reword-root-branch master && + set_fake_editor && + FAKE_LINES="reword 1 2" FAKE_COMMIT_MESSAGE="A changed" \ + git rebase -i --root && + git show HEAD^ | grep "A changed" +' + test_expect_success C_LOCALE_OUTPUT 'rebase --edit-todo does not work on non-interactive rebase' ' git reset --hard && git checkout conflict-branch && -- 2.17.0.windows.1
[PATCH 0/2] rebase --root: fix `reword` on a root commit
From: GitGitGadget Todd Zullinger reported this bug in https://public-inbox.org/git/20180615043111.gs3...@zaya.teonanacatl.net/: when calling git rebase --root and trying to reword the root commit's message, a BUG is reported. This fixes that. IMO the bug fix is trivial enough to qualify for inclusion into v2.18.0, still. Johannes Schindelin (1): rebase --root: fix amending root commit messages Todd Zullinger (1): rebase --root: demonstrate a bug while amending root commit messages sequencer.c | 2 +- t/t3404-rebase-interactive.sh | 9 + 2 files changed, 10 insertions(+), 1 deletion(-) base-commit: 68372c88794aba15f853542008cda39def768372 -- 2.17.0.windows.1
Re: [PATCH 1/3] ewah_read_mmap: bounds-check mmap reads
On Sat, Jun 16, 2018 at 04:35:13PM +0200, SZEDER Gábor wrote: > > + head -c 512 <$bitmap >$bitmap.tmp && > > + mv $bitmap.tmp $bitmap && > > This line turns out to be problematic on OSX and ultimately causes the > test to fail. > > When OSX's 'mv's destination is read-only, it asks whether to replace > the destination even though in the test its stdin is not a terminal > (and thus doesn't conform to POSIX[1]). Since the '.bitmap' file is > read-only, and since 'mv' obviously doesn't get an affirmative > response from /dev/null, the original '.bitmap' file is not > overwritten, the subsequent 'git rev-list' doesn't print any error > message, and finally 'test_i18ngrep' causes the test to fail. Right, sorry, I should have remembered that we've run into this before. Using "mv -f" is the standard solution. E.g., c20d4d702f (t1450: use "mv -f" within loose object directory, 2017-01-24). Junio, can you squash this in to jk/ewah-bounds-check~1? diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh index c4ed88030c..b11bc392a8 100755 --- a/t/t5310-pack-bitmaps.sh +++ b/t/t5310-pack-bitmaps.sh @@ -338,7 +338,7 @@ test_expect_success 'truncated bitmap fails gracefully' ' bitmap=$(ls .git/objects/pack/*.bitmap) && test_when_finished "rm -f $bitmap" && head -c 512 <$bitmap >$bitmap.tmp && - mv $bitmap.tmp $bitmap && + mv -f $bitmap.tmp $bitmap && git rev-list --use-bitmap-index --count --all >actual 2>stderr && test_cmp expect actual && test_i18ngrep corrupt stderr -Peff
[PATCH] doc: fix typos in documentation and release notes
---
Documentation/RelNotes/1.7.11.7.txt | 2 +-
Documentation/RelNotes/2.17.0.txt | 2 +-
Documentation/RelNotes/2.18.0.txt | 2 +-
Documentation/diff-options.txt | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/Documentation/RelNotes/1.7.11.7.txt
b/Documentation/RelNotes/1.7.11.7.txt
index e7e79d999bd38..e743a2a8e46eb 100644
--- a/Documentation/RelNotes/1.7.11.7.txt
+++ b/Documentation/RelNotes/1.7.11.7.txt
@@ -25,7 +25,7 @@ Fixes since v1.7.11.6
references" nor "Reload" did not update what is shown as the
contents of it, when the user overwrote the tag with "git tag -f".
- * "git for-each-ref" did not currectly support more than one --sort
+ * "git for-each-ref" did not correctly support more than one --sort
option.
* "git log .." errored out saying it is both rev range and a path
diff --git a/Documentation/RelNotes/2.17.0.txt
b/Documentation/RelNotes/2.17.0.txt
index d6db0e19cf17b..c2cf891f71adf 100644
--- a/Documentation/RelNotes/2.17.0.txt
+++ b/Documentation/RelNotes/2.17.0.txt
@@ -342,7 +342,7 @@ Fixes since v2.16
validate the data and connected-ness of objects in the received
pack; the code to perform this check has been taught about the
narrow clone's convention that missing objects that are reachable
- from objects in a pack that came from a promissor remote is OK.
+ from objects in a pack that came from a promisor remote is OK.
* There was an unused file-scope static variable left in http.c when
building for versions of libCURL that is older than 7.19.4, which
diff --git a/Documentation/RelNotes/2.18.0.txt
b/Documentation/RelNotes/2.18.0.txt
index 7c59bd92fbd99..1eb13ece53600 100644
--- a/Documentation/RelNotes/2.18.0.txt
+++ b/Documentation/RelNotes/2.18.0.txt
@@ -324,7 +324,7 @@ Fixes since v2.17
after giving an error message.
(merge 3bb0923f06 ps/contains-id-error-message later to maint).
- * "diff-highlight" filter (in contrib/) learned to undertand "git log
+ * "diff-highlight" filter (in contrib/) learned to understand "git log
--graph" output better.
(merge 4551fbba14 jk/diff-highlight-graph-fix later to maint).
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index f466600972f86..bfa3808e49cc0 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -133,7 +133,7 @@ These parameters can also be set individually with
`--stat-width=`,
as file creations or deletions ("new" or "gone", optionally "+l"
if it's a symlink) and mode changes ("+x" or "-x" for adding
or removing executable bit respectively) in diffstat. The
- information is put betwen the filename part and the graph
+ information is put between the filename part and the graph
part. Implies `--stat`.
--numstat::
--
https://github.com/git/git/pull/510
Re: [BUG] git-rebase: reword squashes commits in case of merge-conflicts
On Mon, Jun 11, 2018 at 06:06:11PM +0200, ch wrote:
> During a recent rebase operation on one of my repositories a number of commits
> unexpectedly ended up getting squashed into other commits. After some
> experiments it turned out that the 'reword' instruction seems to squash the
> referenced commit into the preceding commit if there's a merge-conflict.
>
> Here's a small reproduction recipe to illustrate the behavior:
Indeed I can reproduce. It bisects back to when we switched to using the
rebase-helper builtin, so this started with git-2.13.0. I can also slim down
your reproduction testcase a bit; you don't need any additional commits after
the reword, and the only reason to have commits before it is to induce a merge
conflict by dropping them. The fact that there are lots of fixups floating
around is not necessary.
Anyway, patch below.
-- >8 --
Subject: [PATCH] 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. For any of those cases we want a new commit, so
we should not set the to_amend flag.
Reported-by: ch
Signed-off-by: Elijah Newren
---
sequencer.c | 2 +-
t/t3423-rebase-reword.sh | 44
2 files changed, 45 insertions(+), 1 deletion(-)
create mode 100755 t/t3423-rebase-reword.sh
diff --git a/sequencer.c b/sequencer.c
index cca968043e..9e6d1ee368 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3217,7 +3217,7 @@ static int pick_commits(struct todo_list *todo_list,
struct replay_opts *opts)
} else if (res && is_rebase_i(opts) && item->commit)
return res | error_with_patch(item->commit,
item->arg, item->arg_len, opts, res,
- item->command == TODO_REWORD);
+ 0);
} else if (item->command == TODO_EXEC) {
char *end_of_arg = (char *)(item->arg + item->arg_len);
int saved = *end_of_arg;
diff --git a/t/t3423-rebase-reword.sh b/t/t3423-rebase-reword.sh
new file mode 100755
index 00..31c09efd22
--- /dev/null
+++ b/t/t3423-rebase-reword.sh
@@ -0,0 +1,44 @@
+#!/bin/sh
+
+test_description='git rebase interactive with rewording'
+
+. ./test-lib.sh
+
+. "$TEST_DIRECTORY"/lib-rebase.sh
+
+test_expect_success 'setup' '
+ test_commit master file-1 test &&
+
+ git checkout -b stuff &&
+
+ test_commit feature_a file-2 aaa &&
+ test_commit feature_b file-2 ddd
+'
+
+test_expect_success 'reword without issues functions as intended' '
+ git checkout stuff^0 &&
+
+ set_fake_editor &&
+ FAKE_LINES="pick 1 reword 2" FAKE_COMMIT_MESSAGE="feature_b_reworded" \
+ git rebase -i -v master &&
+
+ test "$(git log -1 --format=%B)" = "feature_b_reworded" &&
+ test $(git rev-list --count HEAD) = 3
+'
+
+test_expect_success 'reword comes after a conflict preserves commit' '
+ git checkout stuff^0 &&
+
+ set_fake_editor &&
+ test_must_fail env FAKE_LINES="reword 2" \
+ git rebase -i -v master &&
+
+ git checkout --theirs file-2 &&
+ git add file-2 &&
+ FAKE_COMMIT_MESSAGE="feature_b_reworded" git rebase --continue &&
+
+ test "$(git log -1 --format=%B)" = "feature_b_reworded" &&
+ test $(git rev-list --count HEAD) = 2
+'
+
+test_done
--
2.18.0.rc2.3.g7fc0d4cb57
[L10N] Start l10n round 3 for one more l10n update from v2.18.0-rc2
Hi, Git v2.18.0-rc2 introduced a new l10n update, and it's easy to be fixed, even though there are only two days left for the release of Git 2.18.0. There are too many l10n updates for Git 2.18.0, and some l10n teams may not have enough time to update. Not worry about that, we can send one more pull request to the maint branch after Git 2.18.0 is released. Find and update your translations from the usual place: https://github.com/git-l10n/git-po/ As how to update your XX.po and help to translate Git, please see "Updating a XX.po file" and other sections in “po/README" file. -- Jiang Xin
Re: [PATCH 1/3] ewah_read_mmap: bounds-check mmap reads
> diff --git a/t/t5310-pack-bitmaps.sh b/t/t5310-pack-bitmaps.sh
> index 423c0a475f..237ee6e5fc 100755
> --- a/t/t5310-pack-bitmaps.sh
> +++ b/t/t5310-pack-bitmaps.sh
> @@ -331,4 +331,17 @@ test_expect_success 'pack reuse respects --incremental' '
> git show-index actual &&
> test_cmp expect actual
> '
> +
> +test_expect_success 'truncated bitmap fails gracefully' '
> + git repack -ad &&
> + git rev-list --use-bitmap-index --count --all >expect &&
> + bitmap=$(ls .git/objects/pack/*.bitmap) &&
> + test_when_finished "rm -f $bitmap" &&
> + head -c 512 <$bitmap >$bitmap.tmp &&
> + mv $bitmap.tmp $bitmap &&
This line turns out to be problematic on OSX and ultimately causes the
test to fail.
When OSX's 'mv's destination is read-only, it asks whether to replace
the destination even though in the test its stdin is not a terminal
(and thus doesn't conform to POSIX[1]). Since the '.bitmap' file is
read-only, and since 'mv' obviously doesn't get an affirmative
response from /dev/null, the original '.bitmap' file is not
overwritten, the subsequent 'git rev-list' doesn't print any error
message, and finally 'test_i18ngrep' causes the test to fail.
The relevant part of the '-x' test output on Travis CI:
++mv
.git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap.tmp
.git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap
override r--r--r-- travis/staff for
.git/objects/pack/pack-8886db3fce4f9657c1a43fee7d3ea4f2a4b5be2d.bitmap? (y/n
[n]) not overwritten
++git rev-list --use-bitmap-index --count --all
++test_cmp expect actual
++diff -u expect actual
++test_i18ngrep corrupt stderr
++eval 'last_arg=${2}'
+++last_arg=stderr
++test -f stderr
++test 2 -lt 2
++test 'x!' = xcorrupt
++test -n ''
++test 'x!' = xcorrupt
++grep corrupt stderr
++echo 'error: '\''grep corrupt' 'stderr'\'' didn'\''t find a match in:'
error: 'grep corrupt stderr' didn't find a match in:
++test -s stderr
++echo ''
++return 1
error: last command exited with $?=1
not ok 43 - truncated bitmap fails gracefully
As far as I can tell, 'mv -f' appears to make the test work on OSX as
well.
I've run a build job with an additional 'grep ^override
t/test-results/*.out' command following the tests to see whether there
are any other cases where OSX 'mv' doesn't overwrite a read-only file
without causing the tests to fail, but found nothing. (But note that
the OSX build jobs don't run all tests.)
[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/mv.html
> + git rev-list --use-bitmap-index --count --all >actual 2>stderr &&
> + test_cmp expect actual &&
> + test_i18ngrep corrupt stderr
> +'
> +
Re: [PATCH] unpack-trees: do not fail reset because of unmerged skipped entry
> I do not know offhand if "reset --merge" should force succeeding in such a case, but I agree that it is criminal to stop "reset --hard" with "not uptodate", as the whole point of "hard reset" is to get rid of the 'not up-to-date' modification. I originally had a fix just for "reset --hard". It was in verify_uptodate_1(), to move check for "->reset" earlier. But then I found that "merge --abort" does not use "reset --hard", but rather --merge, so I fixed that. Because --merge should work also, shouldn't it? Actually, I think that fix in verify_uptodate_1() was right, I just did not find what it affects, after the other fix
