git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 0/2] rebase: don't override --no-reschedule-failed-exec with config
Date: Fri,  9 Apr 2021 10:01:36 +0200	[thread overview]
Message-ID: <cover-0.3-0000000000-20210409T075713Z-avarab@gmail.com> (raw)
In-Reply-To: <cover.1616411973.git.avarab@gmail.com>

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.

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
-- 
2.31.1.584.gf4baedee75


  parent reply	other threads:[~2021-04-09  8:02 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-10 19:04 [PATCH 0/3] rebase: offer to reschedule failed exec commands automatically Johannes Schindelin via GitGitGadget
2018-12-10 19:04 ` [PATCH 1/3] rebase: introduce --reschedule-failed-exec Johannes Schindelin via GitGitGadget
2018-12-10 23:18   ` Elijah Newren
2018-12-11 10:14     ` Johannes Schindelin
2018-12-11 16:16       ` Elijah Newren
2018-12-10 19:04 ` [PATCH 2/3] rebase: add a config option to default to --reschedule-failed-exec Johannes Schindelin via GitGitGadget
2021-03-22 11:48   ` [PATCH 0/3] rebase: don't override --no-reschedule-failed-exec with config Ævar Arnfjörð Bjarmason
2021-03-22 11:48     ` [PATCH 1/3] rebase tests: camel-case rebase.rescheduleFailedExec consistently Ævar Arnfjörð Bjarmason
2021-03-22 11:48     ` [PATCH 2/3] rebase tests: use test_unconfig after test_config Ævar Arnfjörð Bjarmason
2021-03-30 13:53       ` Phillip Wood
2021-03-22 11:48     ` [PATCH 3/3] rebase: don't override --no-reschedule-failed-exec with config Ævar Arnfjörð Bjarmason
2021-03-29 14:49       ` Phillip Wood
2021-03-29 16:12         ` Ævar Arnfjörð Bjarmason
2021-03-29 17:15           ` Phillip Wood
2021-03-24 11:50     ` [PATCH 0/3] " Johannes Schindelin
2021-03-30 13:40     ` Phillip Wood
2021-04-09  8:01     ` Ævar Arnfjörð Bjarmason [this message]
2021-04-09  8:01       ` [PATCH v2 1/2] rebase tests: camel-case rebase.rescheduleFailedExec consistently Ævar Arnfjörð Bjarmason
2021-04-09  8:01       ` [PATCH v2 2/2] rebase: don't override --no-reschedule-failed-exec with config Ævar Arnfjörð Bjarmason
2021-04-15 15:24       ` [PATCH v2 0/2] " Phillip Wood
2018-12-10 19:05 ` [PATCH 3/3] rebase: introduce a shortcut for --reschedule-failed-exec Johannes Schindelin via GitGitGadget
2018-12-10 22:08 ` [PATCH 0/3] rebase: offer to reschedule failed exec commands automatically Johannes Sixt
2018-12-10 22:56   ` Stefan Beller
2018-12-11  3:28     ` Junio C Hamano
2018-12-11 10:31     ` Johannes Schindelin
2018-12-11 17:36       ` Stefan Beller
2018-12-10 23:20 ` Elijah Newren
2018-12-11 10:19   ` email lags, was " Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover-0.3-0000000000-20210409T075713Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood123@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).