* [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
@ 2017-07-26 10:27 Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 1/5] rebase --continue: add --autostage to stage unstaged changes Phillip Wood
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Phillip Wood @ 2017-07-26 10:27 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Philip Oakley, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
These patches add an '--autostage' option (and corresponding config
variable) to 'rebase --continue' that will stage any unstaged changes
before continuing. This saves the user having to type 'git add' before
running 'git rebase --continue'.
Phillip Wood (5):
rebase --continue: add --autostage to stage unstaged changes
rebase -i: improve --continue --autostage
Unify rebase amend message when HEAD has changed
Add tests for rebase --continue --autostage
Add rebase.continue.autostage config setting
git-rebase--am.sh | 1 +
git-rebase--interactive.sh | 111 ++++++++++++++++++++++++++++++++----------
git-rebase--merge.sh | 1 +
git-rebase.sh | 76 ++++++++++++++++++++++++++---
sequencer.c | 22 +++++++--
t/t3404-rebase-interactive.sh | 2 +-
t/t3418-rebase-continue.sh | 50 ++++++++++++++++++-
7 files changed, 222 insertions(+), 41 deletions(-)
--
2.13.3
^ permalink raw reply [flat|nested] 19+ messages in thread
* [RFC PATCH 1/5] rebase --continue: add --autostage to stage unstaged changes
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
@ 2017-07-26 10:27 ` Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 2/5] rebase -i: improve --continue --autostage Phillip Wood
` (4 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Phillip Wood @ 2017-07-26 10:27 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Philip Oakley, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
After resolving conflicts during a rebase it is a pain to have to run
'git add' before 'git rebase --continue'. Passing --autostage to 'git
rebase --continue' will stage them automatically so long as 'git diff
--check' says they are ok. This is a safety measure to avoid
accidentally staging files containing unresolved conflicts.
Continuing an interactive rebase after a failed exec or if HEAD has
changed since rebase stopped with --autostage will stage changes that
wont be committed as rebase --continue will bail out. This will be
fixed in the next commit.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
Using diff check is not ideal as it will error out on whitespace
changes (which should be check in a commit hook if the user is worried
about them) as well as merge markers. Looking through diff.c it should
be possible to add a --check=merge-markers optional argument as the
two checks are enabled independently in the code. As Junio pointed out
in a previous message [1] the absence of conflict markers does not
indicate that a file is fully resolved but checking for them does at
least avoid the case of the user blindly continuing when they have
forgotten to look at a conflicted file.
The autostaging behaviour is opt-in so users who like the additional
safety of having to do git add before git rebase --continue can
continue using their current workflow.
I wonder if check_unstaged() should give different error messages
depending on the presence of unmerged paths rather than saying
'unstaged changes' all the time. Also it should probably have some
message after the check for merge markers fails rather than just the
raw output from diff --check.
[1] https://public-inbox.org/git/xmqqmv7td0a5.fsf@gitster.mtv.corp.google.com/
git-rebase--am.sh | 1 +
git-rebase--interactive.sh | 1 +
git-rebase--merge.sh | 1 +
git-rebase.sh | 76 +++++++++++++++++++++++++++++++++++++++++-----
4 files changed, 71 insertions(+), 8 deletions(-)
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 375239341fbfe885e51a25e9e0dc2d4fee791345..30faa8c24cce2149a883c0055e3f6e93dabd2dd0 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -17,6 +17,7 @@ git_rebase__am () {
case "$action" in
continue)
+ check_unstaged
git am --resolved --resolvemsg="$resolvemsg" \
${gpg_sign_opt:+"$gpg_sign_opt"} &&
move_to_original_branch
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 90b1fbe9cf6e8dfb2f4331916809fa40bf9050d2..4c037a3f7a9e01406c4205bf8786a3da5060381f 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1069,6 +1069,7 @@ git_rebase__interactive () {
case "$action" in
continue)
+ check_unstaged
if test ! -d "$rewritten"
then
exec git rebase--helper ${force_rebase:+--no-ff} --continue
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4db3db74ea17ace60d824e83cdee25e9..81723fc485882750c3ed7214b880d49cf55c6d68 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -16,6 +16,7 @@ read_state () {
continue_merge () {
test -d "$state_dir" || die "$state_dir directory does not exist"
+ check_unstaged
unmerged=$(git ls-files -u)
if test -n "$unmerged"
then
diff --git a/git-rebase.sh b/git-rebase.sh
index 2cf73b88e8e83ca34b9eb319dbc2b0a220139b0f..9ca387349b1cde440c4244de9125446fd35a7b67 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -9,7 +9,7 @@ OPTIONS_STUCKLONG=t
OPTIONS_SPEC="\
git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] [<upstream>] [<branch>]
git rebase [-i] [options] [--exec <cmd>] [--onto <newbase>] --root [<branch>]
-git-rebase --continue | --abort | --skip | --edit-todo
+git-rebase --continue [--autostage] | --abort | --skip | --edit-todo
--
Available options are
v,verbose! display a diffstat of what changed upstream
@@ -32,6 +32,7 @@ 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
+a,autostage add unstaged changes to the index when continuing
committer-date-is-author-date! passed to 'git am'
ignore-date! passed to 'git am'
signoff passed to 'git am'
@@ -69,6 +70,7 @@ merge_dir="$GIT_DIR"/rebase-merge
apply_dir="$GIT_DIR"/rebase-apply
verbose=
diffstat=
+autostage=false
test "$(git config --bool rebase.stat)" = true && diffstat=t
autostash="$(git config --bool rebase.autostash || echo false)"
fork_point=auto
@@ -213,6 +215,67 @@ run_pre_rebase_hook () {
fi
}
+check_autostage () {
+ # If the user has already staged files that contain whitespace
+ # errors or merge markers then we want ignore them so rebase
+ # --continue behaves consistency with and without --autostage
+ git diff-index --diff-filter=U --cached --name-only -z HEAD |
+ xargs -0 git diff-index --check HEAD -- &&
+ git diff-files --diff-filter=MA --check &&
+ git add -u ||
+ exit $?
+}
+
+autostage_advice () {
+ gettext "\
+Unable to continue rebasing as there are unstaged changes.
+Please stage or reset the changes before continuing with:
+
+ git rebase --continue
+
+or run:
+
+ git rebase --continue --autostage
+
+to stage them automatically.
+"
+}
+
+check_unstaged () {
+ git update-index --ignore-submodules --refresh >/dev/null
+ if ! git diff-files --quiet --ignore-submodules
+ then
+ if test $autostage = true
+ then
+ check_autostage
+ else
+ die "$(autostage_advice)"
+ fi
+ fi
+}
+
+parse_continue () {
+ action=continue
+ shift
+ while test $# != 0
+ do
+ case "$1" in
+ --autostage)
+ autostage=true
+ ;;
+ --no-autostage)
+ autostage=false
+ ;;
+ --)
+ ;;
+ *)
+ usage
+ ;;
+ esac
+ shift
+ done
+}
+
test -f "$apply_dir"/applying &&
die "$(gettext "It looks like git-am is in progress. Cannot rebase.")"
@@ -243,7 +306,10 @@ do
--verify)
ok_to_skip_pre_rebase=
;;
- --continue|--skip|--abort|--quit|--edit-todo)
+ --continue)
+ parse_continue "$@"
+ ;;
+ --skip|--abort|--quit|--edit-todo)
test $total_argc -eq 2 || usage
action=${1##--}
;;
@@ -374,12 +440,6 @@ continue)
# Sanity check
git rev-parse --verify HEAD >/dev/null ||
die "$(gettext "Cannot read HEAD")"
- git update-index --ignore-submodules --refresh &&
- git diff-files --quiet --ignore-submodules || {
- echo "$(gettext "You must edit all merge conflicts and then
-mark them as resolved using git add")"
- exit 1
- }
read_basic_state
run_specific_rebase
;;
--
2.13.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 2/5] rebase -i: improve --continue --autostage
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 1/5] rebase --continue: add --autostage to stage unstaged changes Phillip Wood
@ 2017-07-26 10:27 ` Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 3/5] Unify rebase amend message when HEAD has changed Phillip Wood
` (3 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Phillip Wood @ 2017-07-26 10:27 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Philip Oakley, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If HEAD has changed since the rebase stopped or rebase stopped due to
a failed exec then 'git rebase --continue --autostage' will autostage
changes that cannot be commited. Fix this by reordering some of the
checks so that 'git rebase --continue --autostage' never stages
changes unless they can be committed. Also reword some error
messages to account of --autostage and try and make them clearer.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
This could do with some tests the check the correct error message is
given in each failure case. However I'm expecting to change the error
messages based on the feedback I get to this patch so I'll add the
tests once the messages are finalized. I've tried to give a bit more
explanation in the error messages as I found some of the previous
messages didn't explain why rebase couldn't continue. Splitting the
advice out means it is consistent between different code paths and
should make it easier to optionally disable it in future if anyone
wanted to add that. As for the formatting of the messages I wonder if
they would be better without so many empty lines (I find the current
messages a bit intimating as they seem so long). e.g.
If you wish to squash the unstaged changes into the last commit, run:
git add -u
git commit --amend \$gpg_sign_opt_quoted
If they are meant to go into a new commit, run:
git add -u
git commit \$gpg_sign_opt_quoted
In both cases, once you're done, continue with:
git rebase --continue
Instead of
If you wish to squash the unstaged changes into the last commit, run:
git add -u
git commit --amend \$gpg_sign_opt_quoted
If they are meant to go into a new commit, run:
git add -u
git commit \$gpg_sign_opt_quoted
In both cases, once you're done, continue with:
git rebase --continue
git-rebase--interactive.sh | 104 ++++++++++++++++++++++++++++++++----------
t/t3404-rebase-interactive.sh | 2 +-
2 files changed, 81 insertions(+), 25 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 4c037a3f7a9e01406c4205bf8786a3da5060381f..8140c88839b4f3a86f53faaaa2ba4433ecc7f58b 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1056,6 +1056,42 @@ The possible behaviours are: ignore, warn, error.")"
fi
}
+unstaged_advice () {
+ gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
+ eval_gettext "\
+If you wish to squash the unstaged changes into the last commit, run:
+
+ git add -u
+ git commit --amend \$gpg_sign_opt_quoted
+
+If they are meant to go into a new commit, run:
+
+ git add -u
+ git commit \$gpg_sign_opt_quoted
+
+In both cases, once you're done, continue with:
+
+ git rebase --continue
+"
+}
+
+staged_advice () {
+ gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
+ eval_gettext "\
+If you wish to squash the staged changes into the last commit, run:
+
+ git commit --amend \$gpg_sign_opt_quoted
+
+If they are meant to go into a new commit, run:
+
+ git commit \$gpg_sign_opt_quoted
+
+In both cases, once you're done, continue with:
+
+ git rebase --continue
+"
+}
+
# The whole contents of this file is run by dot-sourcing it from
# inside a shell function. It used to be that "return"s we see
# below were not inside any function, and expected to return
@@ -1067,9 +1103,50 @@ The possible behaviours are: ignore, warn, error.")"
# here, and immediately call it after defining it.
git_rebase__interactive () {
+amend_head='' amend_ok=''
case "$action" in
continue)
- check_unstaged
+ test -f "$amend" &&
+ amend_head=$(cat "$amend") &&
+ test $amend_head = $(git rev-parse HEAD) &&
+ amend_ok=1
+ git update-index --refresh --ignore-submodules >/dev/null
+ git diff-files --quiet --ignore-submodules
+ unstaged=$?
+ if ! test -f "$author_script"
+ then
+ if test $unstaged = 1
+ then
+ die "$(gettext "Not expecting unstaged changes.")
+$(unstaged_advice)"
+ elif ! git diff-index --cached --quiet --ignore-submodules HEAD --
+ then
+ die "$(gettext "Not expecting staged changes.")
+$(staged_advice)"
+ fi
+ fi
+ if test $unstaged = 1 && test $autostage = true
+ then
+ if test -n "$amend_head" && test -z "$amend_ok"
+ then
+ die "$(gettext "\
+Unable to commit changes as HEAD has changed since git rebase stopped.")
+$(unstaged_advice)"
+ else
+ check_autostage
+ fi
+ elif test $unstaged = 1
+ then
+ if test -n "$amend_head" && test -z "$amend_ok"
+ then
+ die "$(gettext "\
+Unable to continue rebasing as there are unstaged changes and
+HEAD has changed since git rebase stopped.")
+$(unstaged_advice)"
+ else
+ die "$(autostage_advice)"
+ fi
+ fi
if test ! -d "$rewritten"
then
exec git rebase--helper ${force_rebase:+--no-ff} --continue
@@ -1083,31 +1160,11 @@ continue)
rm "$GIT_DIR"/CHERRY_PICK_HEAD ||
die "$(gettext "Could not remove CHERRY_PICK_HEAD")"
else
- if ! test -f "$author_script"
- then
- gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
- die "$(eval_gettext "\
-You have staged changes in your working tree.
-If these changes are meant to be
-squashed into the previous commit, run:
-
- git commit --amend \$gpg_sign_opt_quoted
-
-If they are meant to go into a new commit, run:
-
- git commit \$gpg_sign_opt_quoted
-
-In both cases, once you're done, continue with:
-
- git rebase --continue
-")"
- fi
. "$author_script" ||
die "$(gettext "Error trying to find the author identity to amend commit")"
- if test -f "$amend"
+ if test -n "$amend_head"
then
- current_head=$(git rev-parse --verify HEAD)
- test "$current_head" = $(cat "$amend") ||
+ test -n "$amend_ok" ||
die "$(gettext "\
You have uncommitted changes in your working tree. Please commit them
first and then run 'git rebase --continue' again.")"
@@ -1126,7 +1183,6 @@ first and then run 'git rebase --continue' again.")"
record_in_rewritten "$(cat "$state_dir"/stopped-sha)"
fi
- require_clean_work_tree "rebase"
do_rest
return 0
;;
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 37821d245433f757fa13f0a3e27da0312bebb7db..3eed8a3bc5a8e9c3bae32e181d367d9a9c0aff80 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -563,7 +563,7 @@ test_expect_success 'clean error after failed "exec"' '
echo "edited again" > file7 &&
git add file7 &&
test_must_fail git rebase --continue 2>error &&
- test_i18ngrep "you have staged changes in your working tree" error
+ test_i18ngrep "Not expecting staged changes" error
'
test_expect_success 'rebase a detached HEAD' '
--
2.13.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 3/5] Unify rebase amend message when HEAD has changed
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 1/5] rebase --continue: add --autostage to stage unstaged changes Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 2/5] rebase -i: improve --continue --autostage Phillip Wood
@ 2017-07-26 10:27 ` Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 4/5] Add tests for rebase --continue --autostage Phillip Wood
` (2 subsequent siblings)
5 siblings, 0 replies; 19+ messages in thread
From: Phillip Wood @ 2017-07-26 10:27 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Philip Oakley, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
If rebase --interactive is unable to commit staged changes because
HEAD has changed since rebase stopped the user gets different messages
depending on whether they specified --autostage or not. Update the
messages in the other code paths to match the --autostage one.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
The change from error() to fprintf() is to keep the messages consistent,
maybe the messages in the shell script should be prefixed with
'error:' instead.
git-rebase--interactive.sh | 10 ++++++----
sequencer.c | 22 +++++++++++++++++-----
2 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8140c88839b4f3a86f53faaaa2ba4433ecc7f58b..e1845e940b8de05b10b011d8167917a60a7c00b9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -1164,10 +1164,12 @@ $(unstaged_advice)"
die "$(gettext "Error trying to find the author identity to amend commit")"
if test -n "$amend_head"
then
- test -n "$amend_ok" ||
- die "$(gettext "\
-You have uncommitted changes in your working tree. Please commit them
-first and then run 'git rebase --continue' again.")"
+ test -n "$amend_ok" || {
+ gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")}
+ die "$(gettext "\
+Unable to commit changes as HEAD has changed since git rebase stopped.")
+$(staged_advice)"
+ }
do_with_author git commit --amend --no-verify -F "$msg" -e \
${gpg_sign_opt:+"$gpg_sign_opt"} ||
die "$(gettext "Could not commit staged changes.")"
diff --git a/sequencer.c b/sequencer.c
index 3010faf86398697469e903318a35421d911acb23..2722d36781e5c47ee81eb3359aa6178042430e68 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2214,12 +2214,24 @@ static int commit_staged_changes(struct replay_opts *opts)
if (get_sha1_hex(rev.buf, to_amend))
return error(_("invalid contents: '%s'"),
rebase_path_amend());
- if (hashcmp(head, to_amend))
- return error(_("\nYou have uncommitted changes in your "
- "working tree. Please, commit them\n"
- "first and then run 'git rebase "
- "--continue' again."));
+ if (hashcmp(head, to_amend)) {
+ const char *gpg_opt = gpg_sign_opt_quoted(opts);
+ fprintf(stderr, _(
+"Unable to commit changes as HEAD has changed since git rebase stopped.\n"
+"If you wish to squash the changes into the last commit, run:\n"
+"\n"
+" git commit --amend %s\n"
+"\n"
+"If they are meant to go into a new commit, run:\n"
+"\n"
+" git commit %s\n"
+"\n"
+"In both cases, once you're done, continue with:\n"
+"\n"
+" git rebase --continue\n"), gpg_opt, gpg_opt);
+ return -1;
+ }
strbuf_release(&rev);
flags |= AMEND_MSG;
}
--
2.13.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 4/5] Add tests for rebase --continue --autostage
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
` (2 preceding siblings ...)
2017-07-26 10:27 ` [RFC PATCH 3/5] Unify rebase amend message when HEAD has changed Phillip Wood
@ 2017-07-26 10:27 ` Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 5/5] Add rebase.continue.autostage config setting Phillip Wood
2017-07-26 18:21 ` [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Junio C Hamano
5 siblings, 0 replies; 19+ messages in thread
From: Phillip Wood @ 2017-07-26 10:27 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Philip Oakley, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
Make sure that --autostage stages any changes and fails if there are
merge markers in the file to be staged.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
t/t3418-rebase-continue.sh | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 4428b9086e8bcb383df801834d0de323f316f4fa..4e71e5d5c2f26866cd5d32bfea445f899398d6c9 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -14,7 +14,6 @@ test_expect_success 'setup' '
git checkout -b topic HEAD^ &&
test_commit "commit-new-file-F2-on-topic-branch" F2 22 &&
-
git checkout master
'
@@ -40,6 +39,40 @@ test_expect_success 'non-interactive rebase --continue works with touched file'
git rebase --continue
'
+reset () {
+ rm -fr .git/rebase-* &&
+ git reset --hard commit-new-file-F2-on-topic-branch &&
+ git checkout master
+}
+
+test_autostage () {
+ action=$1
+
+ test_expect_success "rebase $action --continue --autostage stages changes" '
+ reset &&
+ test_must_fail git rebase $action --onto master master topic &&
+ echo "Resolved" >F2 &&
+ git rebase --continue --autostage
+'
+
+ test_expect_success "rebase $action --continue --autostage fails when there are merge markers (1)" '
+ reset &&
+ test_must_fail git rebase $action --onto master master topic &&
+ test_must_fail git rebase --continue --autostage
+'
+
+ test_expect_success "rebase $action --continue --autostage fails when there are merge markers (2)" '
+ reset &&
+ test_must_fail git rebase $action --onto master master topic &&
+ git reset -- F2 &&
+ test_must_fail git rebase --continue --autostage
+ '
+}
+
+test_autostage
+test_autostage -i
+test_autostage -m
+
test_expect_success 'non-interactive rebase --continue with rerere enabled' '
test_config rerere.enabled true &&
test_when_finished "test_might_fail git rebase --abort" &&
--
2.13.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [RFC PATCH 5/5] Add rebase.continue.autostage config setting
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
` (3 preceding siblings ...)
2017-07-26 10:27 ` [RFC PATCH 4/5] Add tests for rebase --continue --autostage Phillip Wood
@ 2017-07-26 10:27 ` Phillip Wood
2017-07-26 18:21 ` [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Junio C Hamano
5 siblings, 0 replies; 19+ messages in thread
From: Phillip Wood @ 2017-07-26 10:27 UTC (permalink / raw)
To: Git Mailing List; +Cc: Junio C Hamano, Philip Oakley, Phillip Wood
From: Phillip Wood <phillip.wood@dunelm.org.uk>
This enables the user to always specify --autostage with --continue
The tests check that setting rebase.continue.autostage=true results in
'git rebase --continue' autostaging unstaged changes and that
'--no-autostage' overrides it.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
git-rebase.sh | 2 +-
t/t3418-rebase-continue.sh | 15 +++++++++++++++
2 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/git-rebase.sh b/git-rebase.sh
index 9ca387349b1cde440c4244de9125446fd35a7b67..632a19079ba1d88092f47121c7a0797af079aaf5 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -70,7 +70,7 @@ merge_dir="$GIT_DIR"/rebase-merge
apply_dir="$GIT_DIR"/rebase-apply
verbose=
diffstat=
-autostage=false
+autostage=$(git config --bool --get rebase.continue.autostage || echo false)
test "$(git config --bool rebase.stat)" = true && diffstat=t
autostash="$(git config --bool rebase.autostash || echo false)"
fork_point=auto
diff --git a/t/t3418-rebase-continue.sh b/t/t3418-rebase-continue.sh
index 4e71e5d5c2f26866cd5d32bfea445f899398d6c9..7c2a30a3cd8ba7e33519bd68a3f89e90409169da 100755
--- a/t/t3418-rebase-continue.sh
+++ b/t/t3418-rebase-continue.sh
@@ -67,6 +67,21 @@ test_autostage () {
git reset -- F2 &&
test_must_fail git rebase --continue --autostage
'
+
+ test_expect_success "rebase.continue.autostage rebase $action works" '
+ reset &&
+ test_must_fail git rebase $action --onto master master topic &&
+ echo "Resolved" >F2 &&
+ git -c rebase.continue.autostage=true rebase --continue
+'
+
+ test_expect_success "rebase $action --continue --no-autostage works" '
+ reset &&
+ test_must_fail git rebase $action --onto master master topic &&
+ echo "Resolved" >F2 &&
+ test_must_fail git -c rebase.continue.autostage=true rebase --continue --no-autostage
+'
+
}
test_autostage
--
2.13.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
` (4 preceding siblings ...)
2017-07-26 10:27 ` [RFC PATCH 5/5] Add rebase.continue.autostage config setting Phillip Wood
@ 2017-07-26 18:21 ` Junio C Hamano
2017-07-26 21:56 ` Junio C Hamano
5 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2017-07-26 18:21 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
Phillip Wood <phillip.wood@talktalk.net> writes:
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> These patches add an '--autostage' option (and corresponding config
> variable) to 'rebase --continue' that will stage any unstaged changes
> before continuing. This saves the user having to type 'git add' before
> running 'git rebase --continue'.
I wonder if this interacts with existing rerere.autoupdate
configuration variable and if so how (i.e. would they conflict and
fight with each other? would they work well together? would one of
them make the other unnecessary?). In any case, they look closely
related and perhaps should be named similarly.
I even have a suspicion that they may be essentially doing the same
thing.
For a previous discussion, you start from here:
https://public-inbox.org/git/7vej6xb4lr.fsf@gitster.siamese.dyndns.org/#t
and for the context, look at the original post by Ingo, to which the
above message is a response to.
Thanks.
>
> Phillip Wood (5):
> rebase --continue: add --autostage to stage unstaged changes
> rebase -i: improve --continue --autostage
> Unify rebase amend message when HEAD has changed
> Add tests for rebase --continue --autostage
> Add rebase.continue.autostage config setting
>
> git-rebase--am.sh | 1 +
> git-rebase--interactive.sh | 111 ++++++++++++++++++++++++++++++++----------
> git-rebase--merge.sh | 1 +
> git-rebase.sh | 76 ++++++++++++++++++++++++++---
> sequencer.c | 22 +++++++--
> t/t3404-rebase-interactive.sh | 2 +-
> t/t3418-rebase-continue.sh | 50 ++++++++++++++++++-
> 7 files changed, 222 insertions(+), 41 deletions(-)
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-07-26 18:21 ` [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Junio C Hamano
@ 2017-07-26 21:56 ` Junio C Hamano
2017-07-26 22:12 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2017-07-26 21:56 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
Junio C Hamano <gitster@pobox.com> writes:
> Phillip Wood <phillip.wood@talktalk.net> writes:
>
>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>
>> These patches add an '--autostage' option (and corresponding config
>> variable) to 'rebase --continue' that will stage any unstaged changes
>> before continuing. This saves the user having to type 'git add' before
>> running 'git rebase --continue'.
>
> I wonder if this interacts with existing rerere.autoupdate
> configuration variable and if so how (i.e. would they conflict and
> fight with each other? would they work well together? would one of
> them make the other unnecessary?). In any case, they look closely
> related and perhaps should be named similarly.
>
> I even have a suspicion that they may be essentially doing the same
> thing.
>
> For a previous discussion, you start from here:
>
> https://public-inbox.org/git/7vej6xb4lr.fsf@gitster.siamese.dyndns.org/#t
>
> and for the context, look at the original post by Ingo, to which the
> above message is a response to.
>
> Thanks.
Hmph, this is interesting.
"git rebase" does take "--rerere-autoupdate" option from the command
line, and propagates it to a later invocation of "rebase --continue"
by storing the value to $state_dir/allow_rerere_autoupdate file and
reading the value from it. $allow_rerere_autoupdate shell variable
is used to hold the setting.
I'd expect that this variable to be used in invocations of "git am"
in git-rebase--am.sh; but that does not seem to be the case. I
wonder if this was once working but over time we broke the feature
without anybody noticing it, or if the support was added but not
completed and the feature was a no-op from the beginning?
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-07-26 21:56 ` Junio C Hamano
@ 2017-07-26 22:12 ` Junio C Hamano
2017-07-27 10:36 ` Phillip Wood
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2017-07-26 22:12 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
Junio C Hamano <gitster@pobox.com> writes:
> Hmph, this is interesting.
>
> "git rebase" does take "--rerere-autoupdate" option from the command
> line, and propagates it to a later invocation of "rebase --continue"
> by storing the value to $state_dir/allow_rerere_autoupdate file and
> reading the value from it. $allow_rerere_autoupdate shell variable
> is used to hold the setting.
>
> I'd expect that this variable to be used in invocations of "git am"
> in git-rebase--am.sh; but that does not seem to be the case. I
> wonder if this was once working but over time we broke the feature
> without anybody noticing it, or if the support was added but not
> completed and the feature was a no-op from the beginning?
At least in v1.7.0 when doing "rebase -m", the rerere-autoupdate was
plumbed correctly through to the invocation of "git merge" that is
done inside git-rebase.sh. I do not see the same option passed down
to the invocation of "git am", so perhaps nobody cared back then
about rerere during "git rebase" that does not use "git am" backend,
even though "git am" itself were capable of talking the option.
In any case, if you corrected the existing "git rebase" and its
backend so that "--rerere-autoupdate" works as advertised, I think
you are already 80% there without adding a yet another option, as I
suspect that the most of the need for avoiding "git add" during a
"git rebase" session is during a conflict resolution, and allowing
"rerere" to automatically update the index with auto-resolution will
leave _only_ changes to the paths the end user actually needs to
take a look and manually fix still not in the index. And from the
workflow point of view, encouraging them to "git add" their manual
resolution after they are satisified with their changes by not doing
"git add" blindly for all changes, like your --autostage" does, is
probably a good thing.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-07-26 22:12 ` Junio C Hamano
@ 2017-07-27 10:36 ` Phillip Wood
2017-07-27 13:25 ` Phillip Wood
0 siblings, 1 reply; 19+ messages in thread
From: Phillip Wood @ 2017-07-27 10:36 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
On 26/07/17 23:12, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Hmph, this is interesting.
>>
>> "git rebase" does take "--rerere-autoupdate" option from the command
>> line, and propagates it to a later invocation of "rebase --continue"
>> by storing the value to $state_dir/allow_rerere_autoupdate file and
>> reading the value from it. $allow_rerere_autoupdate shell variable
>> is used to hold the setting.
>>
>> I'd expect that this variable to be used in invocations of "git am"
>> in git-rebase--am.sh; but that does not seem to be the case. I
>> wonder if this was once working but over time we broke the feature
>> without anybody noticing it, or if the support was added but not
>> completed and the feature was a no-op from the beginning?
>
> At least in v1.7.0 when doing "rebase -m", the rerere-autoupdate was
> plumbed correctly through to the invocation of "git merge" that is
> done inside git-rebase.sh. I do not see the same option passed down
> to the invocation of "git am", so perhaps nobody cared back then
> about rerere during "git rebase" that does not use "git am" backend,
> even though "git am" itself were capable of talking the option.
>
> In any case, if you corrected the existing "git rebase" and its
> backend so that "--rerere-autoupdate" works as advertised
I've had a quick look and I think it's just a case of adding
'$allow_rerere_autoupdate' to the invocation of 'git am'. 'git am' calls
rerere_clear() when skipping so that doesn't need to go into the shell
script. I'll come up with a test to check it works as I think it does.
> I think
> you are already 80% there without adding a yet another option, as I
> suspect that the most of the need for avoiding "git add" during a
> "git rebase" session is during a conflict resolution, and allowing
> "rerere" to automatically update the index with auto-resolution will
> leave _only_ changes to the paths the end user actually needs to
> take a look and manually fix still not in the index.
I'm interested in the 20% as it's about 100% of my rebase conflicts.
I've got rerere enabled but I cannot recall it helping me with a rebase
conflict in the five or six years I've been using git [1]. A lot of my
rebase conflicts come from fixup! commits or reordering things with
rebase -i and generally they're only going to happen once as I don't
apply the same fixup! more than once and if I've rearranged commits the
conflicts are likely to be different in the (possibly unlikely) event I
rearrange them again in the future.
Once I've resolved a conflict I will look at the diff and maybe compile
and run some tests. Then, if I'm happy I'll run 'git rebase --continue',
to me the 'git add' stage doesn't really add anything useful, it's just
an inconvenience.
The other use I have for --autostage is when editing commits with rebase
-i, once I'm happy with the edit I just want to continue and have rebase
amend the commit. I don't feel having to run 'git add' is helpful in
this case.
> And from the
> workflow point of view, encouraging them to "git add" their manual
> resolution after they are satisified with their changes by not doing
> "git add" blindly for all changes, like your --autostage" does, is
> probably a good thing.
In my mind running 'git rebase --continue' is the signal to git that the
conflicts are resolved.
Best Wishes
Phillip
[1] Maybe I've not noticed it helping when rebasing onto an updated
upstream, although if I'm actively changing the rebased commits between
upstream updates the conflicts may well change between updates. Also I
don't tend to get many upstream conflicts with the projects I'm working on.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-07-27 10:36 ` Phillip Wood
@ 2017-07-27 13:25 ` Phillip Wood
2017-07-27 15:24 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Phillip Wood @ 2017-07-27 13:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
On 27/07/17 11:36, Phillip Wood wrote:
> On 26/07/17 23:12, Junio C Hamano wrote:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> Hmph, this is interesting.
>>>
>>> "git rebase" does take "--rerere-autoupdate" option from the command
>>> line, and propagates it to a later invocation of "rebase --continue"
>>> by storing the value to $state_dir/allow_rerere_autoupdate file and
>>> reading the value from it. $allow_rerere_autoupdate shell variable
>>> is used to hold the setting.
>>>
>>> I'd expect that this variable to be used in invocations of "git am"
>>> in git-rebase--am.sh; but that does not seem to be the case. I
>>> wonder if this was once working but over time we broke the feature
>>> without anybody noticing it, or if the support was added but not
>>> completed and the feature was a no-op from the beginning?
>>
>> At least in v1.7.0 when doing "rebase -m", the rerere-autoupdate was
>> plumbed correctly through to the invocation of "git merge" that is
>> done inside git-rebase.sh. I do not see the same option passed down
>> to the invocation of "git am", so perhaps nobody cared back then
>> about rerere during "git rebase" that does not use "git am" backend,
>> even though "git am" itself were capable of talking the option.
>>
>> In any case, if you corrected the existing "git rebase" and its
>> backend so that "--rerere-autoupdate" works as advertised
>
> I've had a quick look and I think it's just a case of adding
> '$allow_rerere_autoupdate' to the invocation of 'git am'. 'git am' calls
> rerere_clear() when skipping so that doesn't need to go into the shell
> script. I'll come up with a test to check it works as I think it does.
>
>> I think
>> you are already 80% there without adding a yet another option, as I
>> suspect that the most of the need for avoiding "git add" during a
>> "git rebase" session is during a conflict resolution, and allowing
>> "rerere" to automatically update the index with auto-resolution will
>> leave _only_ changes to the paths the end user actually needs to
>> take a look and manually fix still not in the index.
>
> I'm interested in the 20% as it's about 100% of my rebase conflicts.
> I've got rerere enabled but I cannot recall it helping me with a rebase
> conflict in the five or six years I've been using git [1]. A lot of my
> rebase conflicts come from fixup! commits or reordering things with
> rebase -i and generally they're only going to happen once as I don't
> apply the same fixup! more than once and if I've rearranged commits the
> conflicts are likely to be different in the (possibly unlikely) event I
> rearrange them again in the future.
>
> Once I've resolved a conflict I will look at the diff and maybe compile
> and run some tests. Then, if I'm happy I'll run 'git rebase --continue',
> to me the 'git add' stage doesn't really add anything useful, it's just
> an inconvenience.
>
> The other use I have for --autostage is when editing commits with rebase
> -i, once I'm happy with the edit I just want to continue and have rebase
> amend the commit. I don't feel having to run 'git add' is helpful in
> this case.
>
>> And from the
>> workflow point of view, encouraging them to "git add" their manual
>> resolution after they are satisified with their changes by not doing
>> "git add" blindly for all changes, like your --autostage" does, is
>> probably a good thing.
Git allows 'git commit -a' to complete a conflicted merge which I think
is much the same thing as I'm proposing. Also there's nothing to stop a
user accidentally running 'git add' on a path that isn't resolved or
just blindly running 'add -u' before continuing the rebase because they
think they've resolved everything. I guess it is a kind of
convenience/safety trade-off.
> In my mind running 'git rebase --continue' is the signal to git that the
> conflicts are resolved.
> Best Wishes
>
> Phillip
>
> [1] Maybe I've not noticed it helping when rebasing onto an updated
> upstream, although if I'm actively changing the rebased commits between
> upstream updates the conflicts may well change between updates. Also I
> don't tend to get many upstream conflicts with the projects I'm working on.
Thinking about it some more, once a conflict is resolved, the next time
that rewritten commit is rebased onto an updated upstream the same
conflict will not occur as the rewritten commit that's being replayed
does not conflict with the previous upstream - the resolution is baked
into the commit that's being replayed.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-07-27 13:25 ` Phillip Wood
@ 2017-07-27 15:24 ` Junio C Hamano
2017-08-21 10:32 ` Phillip Wood
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2017-07-27 15:24 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
Phillip Wood <phillip.wood@talktalk.net> writes:
>> On 26/07/17 23:12, Junio C Hamano wrote:
>>> I think
>>> you are already 80% there without adding a yet another option,...
>> ...
>> I'm interested in the 20% as it's about 100% of my rebase conflicts.
OK, then at least a fixed --rerere-autoupdate would hopefully limit
the scope of the additional 20%; I'd suspect that a new option would
also internally turn on --rerere-autoupdate, so that the remaining
changes you would see upon --continue would be limited to what the
user had to manually resolve (and edit without having textual conflict,
aka evil merge to resolve semantic conflicts).
I am *not* opposed to an option to tell the command to blindly take
such remaining changes, as long as it stays optional---the use of
the option can then be taken as a strong signal that the user is OK
with the local changes in the working tree, even if the user may not
have marked them as resolved with "git add".
>>> And from the
>>> workflow point of view, encouraging them to "git add" their manual
>>> resolution after they are satisified with their changes by not doing
>>> "git add" blindly for all changes, like your --autostage" does, is
>>> probably a good thing.
>
> Git allows 'git commit -a' to complete a conflicted merge which I
> think is much the same thing as I'm proposing....
"-a" is a strong enough sign that the user is OK with all the paths;
"git commit" without an option does not. So it is OK for a new
option (perhaps "--all-autoupdate", which does more than the
existing "--rerere-autoupdate") to become the signal that the user
is OK with all the local changes.
This is a tangent, but concluding a merge with "commit -a" (or "add
-u && commit -a") has always been discouraged among Git expert
users, and it will stay to be so. If you search the list archive,
you would find a good explanation by Linus on this, but a short
version is that this is because it is normal to start a merge in a
dirty working tree where the user has local changes that the user
knows will not interfere with the merge.
Because "rebase" refuses to work in a dirty working tree, the
analogy with "merge" does not quite hold. Doing "add -u" before
telling it to "--continue" would be much safer.
Thanks.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-07-27 15:24 ` Junio C Hamano
@ 2017-08-21 10:32 ` Phillip Wood
2017-08-21 22:41 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Phillip Wood @ 2017-08-21 10:32 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
On 27/07/17 16:24, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
>
>>> On 26/07/17 23:12, Junio C Hamano wrote:
>>>> I think
>>>> you are already 80% there without adding a yet another option,...
>>> ...
>>> I'm interested in the 20% as it's about 100% of my rebase conflicts.
>
> OK, then at least a fixed --rerere-autoupdate would hopefully limit
> the scope of the additional 20%; I'd suspect that a new option would
> also internally turn on --rerere-autoupdate, so that the remaining
> changes you would see upon --continue would be limited to what the
> user had to manually resolve (and edit without having textual conflict,
> aka evil merge to resolve semantic conflicts).
I can see the logic in that but I was imagining (and the patches
implement) that the --autostage option would be passed to 'rebase
--continue' not when the rebase was started by which time it is too late
to turn on --rerere-autoupdate for the current conflicts. I prefer
having to pass --autostage with --continue so that it is a concious
decision by the user to stage unstaged changes when they continue rather
than rebase just doing it each time it continues.
> I am *not* opposed to an option to tell the command to blindly take
> such remaining changes, as long as it stays optional---the use of
> the option can then be taken as a strong signal that the user is OK
> with the local changes in the working tree, even if the user may not
> have marked them as resolved with "git add".
I agree it definitely needs to be optional (possibly with a config
option to have it on by default but I'm not wedded to that as it is easy
to set up a git or shell alias)
What is the best way forward on this? The patches I posted add a
'--autostage' option to be passed with '--continue' which means that
staging all the unstaged changes remains optional but does not allow
--rerere-autoupdate to be automatically enabled. I'm not sure about the
check for merge markers, as it is implemented it also checks for
whitespace errors which is really the domain of the pre-commit hook. If
we go for an explicit --autostage without the config key to make it on
by default then maybe it is OK to drop the check, but it keeping it
could be a useful safety measure. I don't think it would be too much
work the change diff to allow '--check=merge-markers' as internally the
whitespace and marker checks are implemented separately.
Best Wishes
Phillip
>>>> And from the
>>>> workflow point of view, encouraging them to "git add" their manual
>>>> resolution after they are satisified with their changes by not doing
>>>> "git add" blindly for all changes, like your --autostage" does, is
>>>> probably a good thing.
>>
>> Git allows 'git commit -a' to complete a conflicted merge which I
>> think is much the same thing as I'm proposing....
>
> "-a" is a strong enough sign that the user is OK with all the paths;
> "git commit" without an option does not. So it is OK for a new
> option (perhaps "--all-autoupdate", which does more than the
> existing "--rerere-autoupdate") to become the signal that the user
> is OK with all the local changes.
>
> This is a tangent, but concluding a merge with "commit -a" (or "add
> -u && commit -a") has always been discouraged among Git expert
> users, and it will stay to be so. If you search the list archive,
> you would find a good explanation by Linus on this, but a short
> version is that this is because it is normal to start a merge in a
> dirty working tree where the user has local changes that the user
> knows will not interfere with the merge.
>
> Because "rebase" refuses to work in a dirty working tree, the
> analogy with "merge" does not quite hold. Doing "add -u" before
> telling it to "--continue" would be much safer.
>
> Thanks.
>
>
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-08-21 10:32 ` Phillip Wood
@ 2017-08-21 22:41 ` Junio C Hamano
2017-08-22 10:54 ` Phillip Wood
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2017-08-21 22:41 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
Phillip Wood <phillip.wood@talktalk.net> writes:
> ... I prefer
> having to pass --autostage with --continue so that it is a concious
> decision by the user to stage unstaged changes when they continue rather
> than rebase just doing it each time it continues.
In other words, instead of
git add -u && git rebase --continue
you would want a quicker way to say
git rebase --continue $something_here
If that is the case, that is understandable to me. Is the "-u" (I
think "git add -u" is short for "--update" but I didn't check) taken
as a valid option to "git rebase"? If not, that $something_here could
be "-u".
Thanks for pinging the thread; otherwise I would have forgotten,
especially because not many other people were involved in the
discussion to begin with.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-08-21 22:41 ` Junio C Hamano
@ 2017-08-22 10:54 ` Phillip Wood
2017-08-22 15:54 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Phillip Wood @ 2017-08-22 10:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
On 21/08/17 23:41, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
>
>> ... I prefer
>> having to pass --autostage with --continue so that it is a concious
>> decision by the user to stage unstaged changes when they continue rather
>> than rebase just doing it each time it continues.
>
> In other words, instead of
>
> git add -u && git rebase --continue
>
> you would want a quicker way to say
>
> git rebase --continue $something_here
Exactly
> If that is the case, that is understandable to me. Is the "-u" (I
> think "git add -u" is short for "--update" but I didn't check) taken
> as a valid option to "git rebase"? If not, that $something_here could
> be "-u".
At the moment $something_else is -a/--autostage but -u/--update (I've
checked the add man page and you're right) could be good as well.
rebase --continue -a
behaves like commit -a in that it commits all updated tracked files and
does not take pathspecs, if we go for -u then there is a difference with
'git add -u' as that can take an optional pathspec.
Did you have any further thoughts on what checks if any this new option
should make to avoid staging obviously unresolved files?
> Thanks for pinging the thread; otherwise I would have forgotten,
> especially because not many other people were involved in the
> discussion to begin with.
Yes it would be interesting to hear if this would be useful for others too.
Best Wishes
Phillip
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-08-22 10:54 ` Phillip Wood
@ 2017-08-22 15:54 ` Junio C Hamano
2017-08-24 13:25 ` Phillip Wood
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2017-08-22 15:54 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
Phillip Wood <phillip.wood@talktalk.net> writes:
>> In other words, instead of
>>
>> git add -u && git rebase --continue
>>
>> you would want a quicker way to say
>>
>> git rebase --continue $something_here
>
> Exactly
> ...
> rebase --continue -a
>
> behaves like commit -a in that it commits all updated tracked files and
> does not take pathspecs.
Hmph, but what 'a' in "commit -a" wants to convey is "all", and in
the context of "rebase" we want to avoid saying "all". A user can
be confused into thinking "all" refers to "all subsequent commits"
not "all paths conflicted".
Besides, with the $something_here option, the user is explicitly
telling us that the user finisished the resolution of conflicts left
in the working tree. There is nothing "auto" about it.
> Did you have any further thoughts on what checks if any this new option
> should make to avoid staging obviously unresolved files?
The more I think about this, the more I am convinced that it is a
very bad idea to give such a $something_here option only to "rebase".
The standard flow for any operation that could stop in the middle
because the command needs help from the user with conflicts that
cannot be mechanically resolved is
(1) write out conflicted state in the working tree, and mark these
paths in the index by leaving them in higher stages
(i.e. "ls-files -u" would list them);
(2) the user edits them and marks the ones that are now resolved;
(3) the user tells us to "continue".
and this is not limited to "rebase". "cherry-pick", "am", and
"revert" all share the above, and even "merge" (which is a single
commit operation to which "continue" in its original meaning---to
tell us the user is done with this step and wants us to go on
processing further commits or patches---does not quite apply) does.
And "rebase" is an oddball in that it is the only command that we
could deduce which files in the working tree should be "add"ed, i.e.
"all the files that are different from HEAD". All others allow
users to begin in a dirty working tree (they only require the index
to be clean), so your
git [cherry-pick|am|revert] --continue $something_here
cannot be "git add -u && git [that command] --continue". Blindly
taking any and all files that have modifications from HEAD will
produce a wrong result.
You cannot even sanely solve that by first recording the paths that
are conflicted before giving control back to the user and limit the
"add" to them, i.e. doing "git add" only for paths that appear in
"git ls-files -u" output would not catch additional changes the was
needed in files that did not initially conflict (i.e. "evil merge"
will have to modify a file that was not involved in the mechanical
part of the conflict), because out of the files that are dirty in
the working tree, some are evil merge resolutions and others are
ones that were dirty from the beginning.
The only reason "git rebase" can get away without having to worry
about all of the above is because it insists that the working tree
is clean before it begins. In the ideal world, however, we would
want to lift that and allow it to start in a working tree that has
an unfinished local modification that does not interfere with the
rebase, just like all other operations that could stop upon a
conflict. And when that happens, your $something_here option to
"git rebase --continue" will have to worry about it, too.
So...
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-08-22 15:54 ` Junio C Hamano
@ 2017-08-24 13:25 ` Phillip Wood
2017-08-24 16:46 ` Junio C Hamano
0 siblings, 1 reply; 19+ messages in thread
From: Phillip Wood @ 2017-08-24 13:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
On 22/08/17 16:54, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
>
>>> In other words, instead of
>>>
>>> git add -u && git rebase --continue
>>>
>>> you would want a quicker way to say
>>>
>>> git rebase --continue $something_here
>>
>> Exactly
>> ...
>> rebase --continue -a
>>
>> behaves like commit -a in that it commits all updated tracked files and
>> does not take pathspecs.
>
> Hmph, but what 'a' in "commit -a" wants to convey is "all", and in
> the context of "rebase" we want to avoid saying "all". A user can
> be confused into thinking "all" refers to "all subsequent commits"
> not "all paths conflicted".
>
> Besides, with the $something_here option, the user is explicitly
> telling us that the user finisished the resolution of conflicts left
> in the working tree. There is nothing "auto" about it.
>
>> Did you have any further thoughts on what checks if any this new option
>> should make to avoid staging obviously unresolved files?
>
> The more I think about this, the more I am convinced that it is a
> very bad idea to give such a $something_here option only to "rebase".
>
> The standard flow for any operation that could stop in the middle
> because the command needs help from the user with conflicts that
> cannot be mechanically resolved is
>
> (1) write out conflicted state in the working tree, and mark these
> paths in the index by leaving them in higher stages
> (i.e. "ls-files -u" would list them);
>
> (2) the user edits them and marks the ones that are now resolved;
>
> (3) the user tells us to "continue".
>
> and this is not limited to "rebase". "cherry-pick", "am", and
> "revert" all share the above, and even "merge" (which is a single
> commit operation to which "continue" in its original meaning---to
> tell us the user is done with this step and wants us to go on
> processing further commits or patches---does not quite apply) does.
>
> And "rebase" is an oddball in that it is the only command that we
> could deduce which files in the working tree should be "add"ed, i.e.
> "all the files that are different from HEAD". All others allow
> users to begin in a dirty working tree (they only require the index
> to be clean),
Are you sure about that, the second sentence of the cherry-pick man page
is "This requires your working tree to be clean (no modifications from
the HEAD commit).". If you pass '--no-commit' then this is relaxed so
that the index can differ from HEAD but it is not clear from the man
page if the working tree can differ from the index (potentially that
could give different conflicts in the index and working tree). I'm not
sure what a rebase that does not commit changes would look like.
> so your
>
> git [cherry-pick|am|revert] --continue $something_here
>
> cannot be "git add -u && git [that command] --continue". Blindly
> taking any and all files that have modifications from HEAD will
> produce a wrong result.
>
> You cannot even sanely solve that by first recording the paths that
> are conflicted before giving control back to the user and limit the
> "add" to them, i.e. doing "git add" only for paths that appear in
> "git ls-files -u" output would not catch additional changes the was
> needed in files that did not initially conflict (i.e. "evil merge"
> will have to modify a file that was not involved in the mechanical
> part of the conflict), because out of the files that are dirty in
> the working tree, some are evil merge resolutions and others are
> ones that were dirty from the beginning.
I wonder if git could record the state of the working tree in a
temporary index just before it exits with conflicts. Then when the user
has resolved the conflicts
git [cherry-pick|am|revert] --continue $something_here
could do something like
{ GIT_INDEX_FILE=$TMP_INDEX git diff-files --name-only &&
git ls-files -u } | git update-index --remove --stdin
to stage the files that the user has edited. This would not catch the
case where a new untracked file has been created that should be added to
the conflict resolution, but I think it is reasonable to expect the user
to manually add new files in this case. It would also not add an
unchanged file whose changes should be part of the conflict resolution
but I can't think of a sensible case where the user might want that at
the moment.
>
> The only reason "git rebase" can get away without having to worry
> about all of the above is because it insists that the working tree
> is clean before it begins. In the ideal world, however, we would
> want to lift that and allow it to start in a working tree that has
> an unfinished local modification that does not interfere with the
> rebase, just like all other operations that could stop upon a
> conflict.
It could be expensive to check that the local modifications will not
interfere with the rebase - wouldn't it have to look at all the files
touched by each commit before starting? What do cherry-pick and am do
here when picking several commits?
And when that happens, your $something_here option to
> "git rebase --continue" will have to worry about it, too.
> So...
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-08-24 13:25 ` Phillip Wood
@ 2017-08-24 16:46 ` Junio C Hamano
2017-08-25 11:54 ` Phillip Wood
0 siblings, 1 reply; 19+ messages in thread
From: Junio C Hamano @ 2017-08-24 16:46 UTC (permalink / raw)
To: Phillip Wood; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
Phillip Wood <phillip.wood@talktalk.net> writes:
> It could be expensive to check that the local modifications will not
> interfere with the rebase - wouldn't it have to look at all the files
> touched by each commit before starting? What do cherry-pick and am do
> here when picking several commits?
Yes, so?
"I do not think of a way to implement it cheaply, so we forbid
anybody from finding a clever optimization to implement it cheaply
by adding a feature that will not work well with it when it
happens?"
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase
2017-08-24 16:46 ` Junio C Hamano
@ 2017-08-25 11:54 ` Phillip Wood
0 siblings, 0 replies; 19+ messages in thread
From: Phillip Wood @ 2017-08-25 11:54 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git Mailing List, Philip Oakley, Phillip Wood
On 24/08/17 17:46, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
>
>> It could be expensive to check that the local modifications will not
>> interfere with the rebase - wouldn't it have to look at all the files
>> touched by each commit before starting? What do cherry-pick and am do
>> here when picking several commits?
>
> Yes, so?
>
> "I do not think of a way to implement it cheaply, so we forbid
> anybody from finding a clever optimization to implement it cheaply
> by adding a feature that will not work well with it when it
> happens?"
>
Ouch, I think that is a rather unfair characterization of what I said
and certainly does not reflect what I was trying to say. I was asking
since cherry-pick and am can pick/apply multiple commits/patches and
apparently allow the user to start with a dirty tree what do they do
about checking that the applied commits/patches do not interfere with
any local changes. As for "adding a feature that will not work well with
it when it happens?" I suggested a possible solution in the message.
Anyway you're not convinced this feature is a good idea and no one else
has commented so I'll drop it.
Phil
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2017-08-25 11:54 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-26 10:27 [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 1/5] rebase --continue: add --autostage to stage unstaged changes Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 2/5] rebase -i: improve --continue --autostage Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 3/5] Unify rebase amend message when HEAD has changed Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 4/5] Add tests for rebase --continue --autostage Phillip Wood
2017-07-26 10:27 ` [RFC PATCH 5/5] Add rebase.continue.autostage config setting Phillip Wood
2017-07-26 18:21 ` [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase Junio C Hamano
2017-07-26 21:56 ` Junio C Hamano
2017-07-26 22:12 ` Junio C Hamano
2017-07-27 10:36 ` Phillip Wood
2017-07-27 13:25 ` Phillip Wood
2017-07-27 15:24 ` Junio C Hamano
2017-08-21 10:32 ` Phillip Wood
2017-08-21 22:41 ` Junio C Hamano
2017-08-22 10:54 ` Phillip Wood
2017-08-22 15:54 ` Junio C Hamano
2017-08-24 13:25 ` Phillip Wood
2017-08-24 16:46 ` Junio C Hamano
2017-08-25 11:54 ` Phillip Wood
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).