Hi Ævar
On 09/04/2021 09:01, Ævar Arnfjörð Bjarmason wrote:
This fixes a bug where we read config & options when "git rebase -i -x
make" starts, and will understand the "--no-reschedule-failed-exec"
option, but once a command fails we'll lose track of having had a
"--no-reschedule-failed-exec" and will happily re-read the
"rebase.rescheduleFailedExec=true" config the user might have.
Thus we'll have config winning over explicit command-line
options. This series fixes that bug.
Changes since v1:
* I forgot how test_config works, and was doing a test_when_finished
"test_unconfig", which isn't needed, duh! Thanks to Phillip Wood
for that & other reviews on this series.
* There was a discussion about how to add yet another rebase
machinery state file. I think the consensus is "let's just do it
like this", i.e. we could write some tri-state content to the file,
but we'd get into back-compat issues with other git versions.
These patches look fine to me now
Thanks
Phillip
There was a discussiob about how to manage this whole state (a DB,
JSON?) in another thread, let's kick the can of how exactly we store
state down the line, and just fix the bug using the existing state
saving convention for now.
Ævar Arnfjörð Bjarmason (2):
rebase tests: camel-case rebase.rescheduleFailedExec consistently
rebase: don't override --no-reschedule-failed-exec with config
Documentation/git-rebase.txt | 8 ++++++++
sequencer.c | 5 +++++
t/t3418-rebase-continue.sh | 27 +++++++++++++++++++++++++--
3 files changed, 38 insertions(+), 2 deletions(-)
Range-diff against v1:
1: 002dc72ee7 = 1: e0dd2cb82a rebase tests: camel-case rebase.rescheduleFailedExec consistently
2: 330b33e7a8 < -: ---------- rebase tests: use test_unconfig after test_config
3: e00300d58d ! 2: 7991160de3 rebase: don't override --no-reschedule-failed-exec with config
@@ Commit message
However the --reschedule-failed-exec option doesn't take effect when
the rebase starts (we'd just create a
".git/rebase-merge/reschedule-failed-exec" file if it was true). It
- only takes effect when the exec command fails, and the user wants to
- run "rebase --continue".
+ only takes effect when the exec command fails, at which point we'll
+ reschedule the failed "exec" command.
- At that point we'll have forgotten that we asked for
- --no-reschedule-failed-exec at the start, and will happily re-read the
- config.
+ Since we only wrote out the positive
+ ".git/rebase-merge/reschedule-failed-exec" under
+ --reschedule-failed-exec, but nothing with --no-reschedule-failed-exec
+ we'll forget that we asked not to reschedule failed "exec", and would
+ happily re-read the config and see that
+ rebase.rescheduleFailedExec=true is set.
- We'll then see that rebase.rescheduleFailedExec=true is set. At that
- point we have no record of having set --no-reschedule-failed-exec
- earlier. So the config will effectively override the user having
- explicitly disabled the option on the command-line.
+ So the config will effectively override the user having explicitly
+ disabled the option on the command-line.
Even more confusingly: Since rebase accepts different options based on
its state there wasn't even a way to get around this with "rebase
@@ t/t3418-rebase-continue.sh: test_expect_success 'rebase.rescheduleFailedExec onl
+test_expect_success 'rebase.rescheduleFailedExec=true & --no-reschedule-failed-exec' '
+ test_when_finished "git rebase --abort" &&
-+ test_when_finished "test_unconfig rebase.rescheduleFailedExec" &&
+ test_config rebase.rescheduleFailedExec true &&
+ test_must_fail git rebase -x false --no-reschedule-failed-exec HEAD~2 &&
+ test_must_fail git rebase --continue 2>err &&
@@ t/t3418-rebase-continue.sh: test_expect_success 'rebase.rescheduleFailedExec onl
+test_expect_success 'new rebase.rescheduleFailedExec=true setting in an ongoing rebase is ignored' '
+ test_when_finished "git rebase --abort" &&
+ test_must_fail git rebase -x false HEAD~2 &&
-+ test_when_finished "test_unconfig rebase.rescheduleFailedExec" &&
+ test_config rebase.rescheduleFailedExec true &&
+ test_must_fail git rebase --continue 2>err &&
+ ! grep "has been rescheduled" err
