rebase: offer to reschedule failed exec commands automatically

From: Johannes Schindelin <[email protected]>

A common use case for the --exec option is to verify that each commit in a topic branch compiles cleanly, via git rebase -x make <base>.

However, when an exec in such a rebase fails, it is not re-scheduled, which in this instance is not particularly helpful.

Let's offer a flag to reschedule failed exec commands.

Based on an idea by Paul Morelle.

Signed-off-by Johannes Schindelin <[email protected]>

---
 Documentation/git-rebase.txt  |  5 +++++
 builtin/rebase--interactive.c |  2 ++
 builtin/rebase.c              | 16 +++++++++++++++-
 git-legacy-rebase.sh          | 16 +++++++++++++++-
 git-rebase--common.sh         |  1 +
 sequencer.c                   | 13 ++++++++++---
 sequencer.h                   |  1 +
 t/t3418-rebase-continue.sh    |  6 ++++++
 8 files changed, 55 insertions(+), 5 deletions(-)

Documentation/git-rebase.txt +5 -0

index 80793bad8d..9dd68f77f6 100644
@@ -501,6 +501,11 @@ See also INCOMPATIBLE OPTIONS below.
        with care: the final stash application after a successful
        rebase might result in non-trivial conflicts.
 
+--reschedule-failed-exec::
+--no-reschedule-failed-exec::
+       Automatically reschedule `exec` commands that failed. This only makes
+       sense in interactive mode (or when an `--exec` option was provided).
+
 INCOMPATIBLE OPTIONS
 --------------------
 

builtin/rebase--interactive.c +2 -0

index a2ab68ed06..a9ab009fdb 100644
@@ -192,6 +192,8 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix)
                OPT_STRING(0, "onto-name", &onto_name, N_("onto-name"), N_("onto name")),
                OPT_STRING(0, "cmd", &cmd, N_("cmd"), N_("the command to run")),
                OPT_RERERE_AUTOUPDATE(&opts.allow_rerere_auto),
+               OPT_BOOL(0, "reschedule-failed-exec", &opts.reschedule_failed_exec,
+                        N_("automatically re-schedule any `exec` that fails")),
                OPT_END()
        };
 

builtin/rebase.c +6 -0

index 5b3e5baec8..6d556fc6c8 100644
@@ -104,6 +104,7 @@ struct rebase_options {
        int rebase_merges, rebase_cousins;
        char *strategy, *strategy_opts;
        struct strbuf git_format_patch_opt;
+       int reschedule_failed_exec;
 };
 
 static int is_interactive(struct rebase_options *opts)

@@ -415,6 +416,8 @@ static int run_specific_rebase(struct rebase_options *opts)
                        argv_array_push(&child.args, opts->gpg_sign_opt);
                if (opts->signoff)
                        argv_array_push(&child.args, "--signoff");
+               if (opts->reschedule_failed_exec)
+                       argv_array_push(&child.args, "--reschedule-failed-exec");
 
                status = run_command(&child);
                goto finished_rebase;

@@ -903,6 +906,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                                   "strategy")),
                OPT_BOOL(0, "root", &options.root,
                         N_("rebase all reachable commits up to the root(s)")),
+               OPT_BOOL(0, "reschedule-failed-exec",
+                        &options.reschedule_failed_exec,
+                        N_("automatically re-schedule any `exec` that fails")),
                OPT_END(),
        };
        int i;

builtin/rebase.c +6 -1

        if (options.git_am_opts.argc) {
                /* all am options except -q are compatible only with --am */
                for (i = options.git_am_opts.argc - 1; i >= 0; i--)

@@ -1220,7 +1229,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                options.flags |= REBASE_FORCE;
        }
 
-       if (options.type == REBASE_PRESERVE_MERGES)
+       if (options.type == REBASE_PRESERVE_MERGES) {
                /*
                 * Note: incompatibility with --signoff handled in signoff block above
                 * Note: incompatibility with --interactive is just a strong warning;

@@ -1230,6 +1239,11 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                        die(_("error: cannot combine '--preserve-merges' with "
                              "'--rebase-merges'"));
 
+               if (options.reschedule_failed_exec)
+                       die(_("error: cannot combine '--preserve-merges' with "
+                             "'--reschedule-failed-exec'"));
+       }
+
        if (options.rebase_merges) {
                if (strategy_options.nr)
                        die(_("error: cannot combine '--rebase-merges' with "

git-legacy-rebase.sh +12 -1

index b97ffdc9dd..5f0f0c5ab8 100755
@@ -48,6 +48,7 @@ skip!              skip current patch and continue
 edit-todo!         edit the todo list during an interactive rebase
 quit!              abort but keep HEAD where it is
 show-current-patch! show the patch file being applied or merged
+reschedule-failed-exec automatically reschedule failed exec commands
 "
 . git-sh-setup
 set_reflog_action rebase

@@ -92,6 +93,7 @@ autosquash=
 keep_empty=
 allow_empty_message=--allow-empty-message
 signoff=
+reschedule_failed_exec=
 test "$(git config --bool rebase.autosquash)" = "true" && autosquash=t
 case "$(git config --bool commit.gpgsign)" in
 true)  gpg_sign_opt=-S ;;

@@ -126,6 +128,8 @@ read_basic_state () {
                signoff="$(cat "$state_dir"/signoff)"
                force_rebase=t
        }
+       test -f "$state_dir"/reschedule-failed-exec &&
+               reschedule_failed_exec=t
 }
 
 finish_rebase () {

@@ -163,7 +167,8 @@ run_interactive () {
                "$allow_empty_message" "$autosquash" "$verbose" \
                "$force_rebase" "$onto_name" "$head_name" "$strategy" \
                "$strategy_opts" "$cmd" "$switch_to" \
-               "$allow_rerere_autoupdate" "$gpg_sign_opt" "$signoff"
+               "$allow_rerere_autoupdate" "$gpg_sign_opt" "$signoff" \
+               "$reschedule_failed_exec"
 }
 
 run_specific_rebase () {

@@ -378,6 +383,12 @@ do
        --gpg-sign=*)
                gpg_sign_opt="-S${1#--gpg-sign=}"
                ;;
+       --reschedule-failed-exec)
+               reschedule_failed_exec=--reschedule-failed-exec
+               ;;
+       --no-reschedule-failed-exec)
+               reschedule_failed_exec=
+               ;;
        --)
                shift
                break

git-rebase--common.sh +1 -0

index 7e39d22871..a8a44608e0 100644
@@ -19,6 +19,7 @@ write_basic_state () {
                "$state_dir"/allow_rerere_autoupdate
        test -n "$gpg_sign_opt" && echo "$gpg_sign_opt" > "$state_dir"/gpg_sign_opt
        test -n "$signoff" && echo "$signoff" >"$state_dir"/signoff
+       test -n "$reschedule_failed_exec" && : > "$state_dir"/reschedule-failed-exec
 }
 
 apply_autostash () {

sequencer.c +10 -3

index e1a4dd15f1..69bee63e11 100644
@@ -158,6 +158,7 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
 static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
 static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
 static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet")
+static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedule-failed-exec")
 
 static int git_sequencer_config(const char *k, const char *v, void *cb)
 {

@@ -2362,6 +2363,9 @@ static int read_populate_opts(struct replay_opts *opts)
                        opts->signoff = 1;
                }
 
+               if (file_exists(rebase_path_reschedule_failed_exec()))
+                       opts->reschedule_failed_exec = 1;
+
                read_strategy_opts(opts, &buf);
                strbuf_release(&buf);
 

@@ -2443,6 +2447,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
                write_file(rebase_path_gpg_sign_opt(), "-S%s\n", opts->gpg_sign);
        if (opts->signoff)
                write_file(rebase_path_signoff(), "--signoff\n");
+       if (opts->reschedule_failed_exec)
+               write_file(rebase_path_reschedule_failed_exec(), "%s", "");
 
        return 0;
 }

@@ -3586,9 +3592,10 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
                        *end_of_arg = saved;
 
                        /* Reread the todo file if it has changed. */
-                       if (res)
-                               ; /* fall through */
-                       else if (stat(get_todo_path(opts), &st))
+                       if (res) {
+                               if (opts->reschedule_failed_exec)
+                                       reschedule = 1;
+                       } else if (stat(get_todo_path(opts), &st))
                                res = error_errno(_("could not stat '%s'"),
                                                  get_todo_path(opts));
                        else if (match_stat_data(&todo_list->stat, &st)) {

sequencer.h +1 -0

index 5071a73563..1f865dae26 100644
@@ -39,6 +39,7 @@ struct replay_opts {
        int allow_empty_message;
        int keep_redundant_commits;
        int verbose;
+       int reschedule_failed_exec;
 
        int mainline;
 

t/t3418-rebase-continue.sh +0 -0

index 0210b2ac6f..54b26a9284 100755

Quoted 5 lines · Johannes Schindelin via GitGitGadget

(quoted diff) +0 -0

@@ -1195,6 +1201,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                break;
        }
 

builtin/rebase.c int cmd_rebase(int argc, const char **argv, const char *prefix) · line 1206

Quoted 3 lines

builtin/rebase.c +3 -0

+       if (options.reschedule_failed_exec && !is_interactive(&options))
+               die(_("--reschedule-failed-exec requires an interactive rebase"));
+

I was surprised at first that you checked is_interactive() rather than checking for --exec being specified. But I guess this is because users can manually specify 'exec' lines.

What if the user specifies an implicitly interactive rebase (i.e. no editing of the todo list, such as with --rebase-merges or --keep-empty, or soon --strategy or --strategy-option) and also doesn't specify --exec?

git-legacy-rebase.sh then · line 553

Quoted 10 lines

git-legacy-rebase.sh +3 -0

@@ -534,6 +545,9 @@ then
        #       git-rebase.txt caveats with "unless you know what you are doing"
        test -n "$rebase_merges" &&
                die "$(gettext "error: cannot combine '--preserve-merges' with '--rebase-merges'")"
+
+       test -n "$reschedule_failed_exec" &&
+               die "$(gettext "error: cannot combine '--preserve-merges' with '--reschedule-failed-exec'")"
 fi
 
 if test -n "$rebase_merges"

In the builtin rebase, you checked that --reschedule-failed-exec had to be used with an interactive rebase. Here in the legacy rebase you have no such check at all.

Not sure if that's an oversight, or if we're at the point where we just start intentionally allowing legacy rebase to lag and soon throw it out. (When do we get to that point?)

The rest of the patch looks good to me.

Hi Elijah,

Quoted 7 lines · Elijah Newren

Quoted 5 lines · Johannes Schindelin via GitGitGadget

(quoted diff) +0 -0

@@ -1195,6 +1201,9 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
                break;
        }
 

Quoted 7 lines

builtin/rebase.c int cmd_rebase(int argc, const char **argv, const char *prefix) · line 1206

Quoted 3 lines

builtin/rebase.c +3 -0

+       if (options.reschedule_failed_exec && !is_interactive(&options))
+               die(_("--reschedule-failed-exec requires an interactive rebase"));
+

I was surprised at first that you checked is_interactive() rather than checking for --exec being specified. But I guess this is because users can manually specify 'exec' lines.

Indeed, that is exactly the reason.

Quoted 4 lines

What if the user specifies an implicitly interactive rebase (i.e. no editing of the todo list, such as with --rebase-merges or --keep-empty, or soon --strategy or --strategy-option) and also doesn't specify --exec?

Then the todo list won't have any exec lines, and the flag is irrelevant (but does not do any harm).

... except in the case that the rebase fails at some stage, the user edits the todo list with git rebase --edit-todo and inserts an exec line.

Fix a test added in 906b63942ac (rebase --am: ignore rebase.rescheduleFailedExec, 2019-07-01) to reset its config after it runs. This doesn't matter now since it's the last test in the file, but will in a subsequent commit where I'll add new tests after this one.

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---
  t/t3418-rebase-continue.sh | 1 +
  1 file changed, 1 insertion(+)

t/t3418-rebase-continue.sh +0 -0

index fe407e63cf1..ea14ef496cb 100755

t/t3418-rebase-continue.sh +0 -0

  	test_config rebase.rescheduleFailedExec true &&
  	test_must_fail git rebase -x false HEAD^ &&
  	grep "^exec false" .git/rebase-merge/git-rebase-todo &&

Fix a bug in how --no-reschedule-failed-exec interacts with rebase.rescheduleFailedExec=true being set in the config. Before this change the --no-reschedule-failed-exec config option would be overridden by the config.

This bug happened because of the particulars of how "rebase" works v.s. most other git commands when it comes to parsing options and config:

When we read the config and parse the CLI options we correctly prefer the --no-reschedule-failed-exec option over rebase.rescheduleFailedExec=true in the config. So far so good.

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".

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.

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.

Even more confusingly: Since rebase accepts different options based on its state there wasn't even a way to get around this with "rebase --continue --no-reschedule-failed-exec" (but you could of course set the config with "rebase -c ...").

I think the least bad way out of this is to declare that for such options and config whatever we decide at the beginning of the rebase goes. So we'll now always create either a "reschedule-failed-exec" or a "no-reschedule-failed-exec file at the start, not just the former if we decided we wanted the feature.

With this new worldview you can no longer change the setting once a rebase has started except by manually removing the state files discussed above. I think making it work like that is the the least confusing thing we can do.

In the future we might want to learn to change the setting in the middle by combining "--edit-todo" with "--[no-]reschedule-failed-exec", we currently don't support combining those options, or any other way to change the state in the middle of the rebase short of manually editing the files in ".git/rebase-merge/*".

The bug being fixed here originally came about because of a combination of the behavior of the code added in d421afa0c66 (rebase:
introduce --reschedule-failed-exec, 2018-12-10) and the addition of the config variable in 969de3ff0e0 (rebase: add a config option to default to --reschedule-failed-exec, 2018-12-10).

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---
  Documentation/git-rebase.txt |  8 ++++++++
  sequencer.c                  |  5 +++++
  t/t3418-rebase-continue.sh   | 25 +++++++++++++++++++++++++
  3 files changed, 38 insertions(+)

Documentation/git-rebase.txt +8 -0

index a0487b5cc58..b48e6225769 100644
@@ -622,6 +622,14 @@ See also INCOMPATIBLE OPTIONS below.
  --no-reschedule-failed-exec::
  	Automatically reschedule `exec` commands that failed. This only makes
  	sense in interactive mode (or when an `--exec` option was provided).
++
+Even though this option applies once a rebase is started, it's set for
+the whole rebase at the start based on either the
+`rebase.rescheduleFailedExec` configuration (see linkgit:git-config[1]
+or "CONFIGURATION" below) or whether this option is
+provided. Otherwise an explicit `--no-reschedule-failed-exec` at the
+start would be overridden by the presence of
+`rebase.rescheduleFailedExec=true` configuration.
 
  INCOMPATIBLE OPTIONS
  --------------------

sequencer.c +5 -0

index 848204d3dc3..59735fdff62 100644
@@ -164,6 +164,7 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
  static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
  static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
  static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedule-failed-exec")
+static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec")
  static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
  static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
 

@@ -2672,6 +2673,8 @@ static int read_populate_opts(struct replay_opts *opts)
 
  		if (file_exists(rebase_path_reschedule_failed_exec()))
  			opts->reschedule_failed_exec = 1;
+		else if (file_exists(rebase_path_no_reschedule_failed_exec()))
+			opts->reschedule_failed_exec = 0;
 
  		if (file_exists(rebase_path_drop_redundant_commits()))
  			opts->drop_redundant_commits = 1;

@@ -2772,6 +2775,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
  		write_file(rebase_path_ignore_date(), "%s", "");
  	if (opts->reschedule_failed_exec)
  		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
+	else
+		write_file(rebase_path_no_reschedule_failed_exec(), "%s", "");
 
  	return 0;
  }

t/t3418-rebase-continue.sh +25 -0

index ea14ef496cb..9553d969646 100755
@@ -291,4 +291,29 @@ test_expect_success 'rebase.rescheduleFailedExec only affects `rebase -i`' '
  	git rebase HEAD^
  '
 
+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 &&
+	! grep "has been rescheduled" err
+'
+
+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
+'
+
+test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebase' '
+	test_when_finished "git rebase --abort" &&
+	test_must_fail git rebase -x false HEAD~2 &&
+	test_expect_code 129 git rebase --continue --no-reschedule-failed-exec &&
+	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
+'
+
  test_done

Hi Ævar

Quoted 8 lines · Ævar Arnfjörð Bjarmason

Fix a bug in how --no-reschedule-failed-exec interacts with rebase.rescheduleFailedExec=true being set in the config. Before this change the --no-reschedule-failed-exec config option would be overridden by the config.
This bug happened because of the particulars of how "rebase" works v.s. most other git commands when it comes to parsing options and config:
When we read the config and parse the CLI options we correctly

Quoted 8 lines

prefer
the --no-reschedule-failed-exec option over rebase.rescheduleFailedExec=true in the config. So far so good. 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".

The exec command is rescheduled in the todo file as soon as it fails, we do not wait for the user to run 'rebase --continue' to reschedule it. However if it still fails after restarting or a later exec fails we have the problem you describe.

Quoted 9 lines

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.
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.
Even more confusingly: Since rebase accepts different options based on

Quoted 8 lines

its state there wasn't even a way to get around this with "rebase --continue --no-reschedule-failed-exec" (but you could of course set the config with "rebase -c ...").
I think the least bad way out of this is to declare that for such options and config whatever we decide at the beginning of the rebase goes. So we'll now always create either a "reschedule-failed-exec" or a "no-reschedule-failed-exec file at the start, not just the former if we decided we wanted the feature.

Thanks for working on this and for the detailed commit message. I'm not entirely convinced we want yet another state file in .git/rebase-merge. We could we start writing the setting to the file rather than having different files for whether the option is on or off. If we use the contents of the file it could be -1 for 'use config', 0 'off', 1 'on'. The downside is that starting 'rebase --no-reschedule-failed-exec' with a new version of git and then
continuing with an old version would do the wrong thing.

Best Wishes

Phillip

Quoted 102 lines

With this new worldview you can no longer change the setting once a rebase has started except by manually removing the state files discussed above. I think making it work like that is the the least confusing thing we can do.
In the future we might want to learn to change the setting in the middle by combining "--edit-todo" with "--[no-]reschedule-failed-exec", we currently don't support combining those options, or any other way to change the state in the middle of the rebase short of manually editing the files in ".git/rebase-merge/*".
The bug being fixed here originally came about because of a combination of the behavior of the code added in d421afa0c66 (rebase:
introduce --reschedule-failed-exec, 2018-12-10) and the addition of the config variable in 969de3ff0e0 (rebase: add a config option to default to --reschedule-failed-exec, 2018-12-10).

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---
  Documentation/git-rebase.txt |  8 ++++++++
  sequencer.c                  |  5 +++++
  t/t3418-rebase-continue.sh   | 25 +++++++++++++++++++++++++
  3 files changed, 38 insertions(+)

Documentation/git-rebase.txt +8 -0

index a0487b5cc58..b48e6225769 100644
@@ -622,6 +622,14 @@ See also INCOMPATIBLE OPTIONS below.
  --no-reschedule-failed-exec::
  	Automatically reschedule `exec` commands that failed. This only makes
  	sense in interactive mode (or when an `--exec` option was provided).
++
+Even though this option applies once a rebase is started, it's set for
+the whole rebase at the start based on either the
+`rebase.rescheduleFailedExec` configuration (see linkgit:git-config[1]
+or "CONFIGURATION" below) or whether this option is
+provided. Otherwise an explicit `--no-reschedule-failed-exec` at the
+start would be overridden by the presence of
+`rebase.rescheduleFailedExec=true` configuration.
    INCOMPATIBLE OPTIONS
  --------------------

sequencer.c +1 -0

index 848204d3dc3..59735fdff62 100644
@@ -164,6 +164,7 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
  static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
  static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
  static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedule-failed-exec")
+static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec")
  static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
  static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
  @@ -2672,6 +2673,8 @@ static int read_populate_opts(struct

replay_opts *opts)
if (file_exists(rebase_path_reschedule_failed_exec()))
opts->reschedule_failed_exec = 1;

sequencer.c +4 -0

+		else if (file_exists(rebase_path_no_reschedule_failed_exec()))
+			opts->reschedule_failed_exec = 0;
    		if (file_exists(rebase_path_drop_redundant_commits()))
  			opts->drop_redundant_commits = 1;

@@ -2772,6 +2775,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
  		write_file(rebase_path_ignore_date(), "%s", "");
  	if (opts->reschedule_failed_exec)
  		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
+	else
+		write_file(rebase_path_no_reschedule_failed_exec(), "%s", "");
    	return 0;
  }

t/t3418-rebase-continue.sh +24 -1

index ea14ef496cb..9553d969646 100755
@@ -291,4 +291,29 @@ test_expect_success 'rebase.rescheduleFailedExec only affects `rebase -i`' '
  	git rebase HEAD^
  '
  +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 &&
+	! grep "has been rescheduled" err
+'
+
+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
+'
+
+test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebase' '
+	test_when_finished "git rebase --abort" &&
+	test_must_fail git rebase -x false HEAD~2 &&
+	test_expect_code 129 git rebase --continue --no-reschedule-failed-exec &&
+	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
+'
+
  test_done

Quoted 20 lines · Phillip Wood

Hi Ævar

Quoted 16 lines · Ævar Arnfjörð Bjarmason

Fix a bug in how --no-reschedule-failed-exec interacts with rebase.rescheduleFailedExec=true being set in the config. Before this change the --no-reschedule-failed-exec config option would be overridden by the config.
This bug happened because of the particulars of how "rebase" works v.s. most other git commands when it comes to parsing options and config:
When we read the config and parse the CLI options we correctly prefer
the --no-reschedule-failed-exec option over rebase.rescheduleFailedExec=true in the config. So far so good. 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".

Quoted 4 lines

The exec command is rescheduled in the todo file as soon as it fails, we do not wait for the user to run 'rebase --continue' to reschedule it. However if it still fails after restarting or a later exec fails we have the problem you describe.

Right, as noted in [1] I grokked those internals eventually, but the commit message is written from the viewpoint of a hypothetical user...

B.t.w. if you've had a chance to read [1] over it would be interesting to get some thoughts on that rambling. So far I only managed to upset Johannes :)

1. http://lore.kernel.org/git/[email protected]

Quoted 18 lines

Quoted 9 lines

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.
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.
Even more confusingly: Since rebase accepts different options based on

Quoted 8 lines

its state there wasn't even a way to get around this with "rebase --continue --no-reschedule-failed-exec" (but you could of course set the config with "rebase -c ...").
I think the least bad way out of this is to declare that for such options and config whatever we decide at the beginning of the rebase goes. So we'll now always create either a "reschedule-failed-exec" or a "no-reschedule-failed-exec file at the start, not just the former if we decided we wanted the feature.

Quoted 8 lines

Thanks for working on this and for the detailed commit message. I'm not entirely convinced we want yet another state file in .git/rebase-merge. We could we start writing the setting to the file rather than having different files for whether the option is on or off. If we use the contents of the file it could be -1 for 'use config', 0 'off', 1 'on'. The downside is that starting 'rebase --no-reschedule-failed-exec' with a new version of git and then
continuing with an old version would do the wrong thing.

Yes I suppose, but it seems much simpler indeed to just represent this sort of tri-state as a ENOENT/no-FOO/FOO neither exists + file pair with how the current code is set up,

especially because (as you note) we'd
need to phase-in any writing of the content across multiple versions or something, least in-progress rebases across versions subtly behave weirdly.

Well, in this case it's not such a big deal, but I'd rather not establish the pattern for something that *does* matter.

Quoted 106 lines

Best Wishes

Phillip

Quoted 102 lines

With this new worldview you can no longer change the setting once a rebase has started except by manually removing the state files discussed above. I think making it work like that is the the least confusing thing we can do.
In the future we might want to learn to change the setting in the middle by combining "--edit-todo" with "--[no-]reschedule-failed-exec", we currently don't support combining those options, or any other way to change the state in the middle of the rebase short of manually editing the files in ".git/rebase-merge/*".
The bug being fixed here originally came about because of a combination of the behavior of the code added in d421afa0c66 (rebase:
introduce --reschedule-failed-exec, 2018-12-10) and the addition of the config variable in 969de3ff0e0 (rebase: add a config option to default to --reschedule-failed-exec, 2018-12-10).

Signed-off-by Ævar Arnfjörð Bjarmason <[email protected]>

---
   Documentation/git-rebase.txt |  8 ++++++++
   sequencer.c                  |  5 +++++
   t/t3418-rebase-continue.sh   | 25 +++++++++++++++++++++++++
   3 files changed, 38 insertions(+)

Documentation/git-rebase.txt +8 -0

index a0487b5cc58..b48e6225769 100644
@@ -622,6 +622,14 @@ See also INCOMPATIBLE OPTIONS below.
   --no-reschedule-failed-exec::
   	Automatically reschedule `exec` commands that failed. This only makes
   	sense in interactive mode (or when an `--exec` option was provided).
++
+Even though this option applies once a rebase is started, it's set for
+the whole rebase at the start based on either the
+`rebase.rescheduleFailedExec` configuration (see linkgit:git-config[1]
+or "CONFIGURATION" below) or whether this option is
+provided. Otherwise an explicit `--no-reschedule-failed-exec` at the
+start would be overridden by the presence of
+`rebase.rescheduleFailedExec=true` configuration.
     INCOMPATIBLE OPTIONS
   --------------------

sequencer.c +1 -0

index 848204d3dc3..59735fdff62 100644
@@ -164,6 +164,7 @@ static GIT_PATH_FUNC(rebase_path_strategy, "rebase-merge/strategy")
   static GIT_PATH_FUNC(rebase_path_strategy_opts, "rebase-merge/strategy_opts")
   static GIT_PATH_FUNC(rebase_path_allow_rerere_autoupdate, "rebase-merge/allow_rerere_autoupdate")
   static GIT_PATH_FUNC(rebase_path_reschedule_failed_exec, "rebase-merge/reschedule-failed-exec")
+static GIT_PATH_FUNC(rebase_path_no_reschedule_failed_exec, "rebase-merge/no-reschedule-failed-exec")
   static GIT_PATH_FUNC(rebase_path_drop_redundant_commits, "rebase-merge/drop_redundant_commits")
   static GIT_PATH_FUNC(rebase_path_keep_redundant_commits, "rebase-merge/keep_redundant_commits")
   @@ -2672,6 +2673,8 @@ static int read_populate_opts(struct

replay_opts *opts)
if (file_exists(rebase_path_reschedule_failed_exec()))
opts->reschedule_failed_exec = 1;

sequencer.c +4 -0

+		else if (file_exists(rebase_path_no_reschedule_failed_exec()))
+			opts->reschedule_failed_exec = 0;
     		if (file_exists(rebase_path_drop_redundant_commits()))
   			opts->drop_redundant_commits = 1;

@@ -2772,6 +2775,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
   		write_file(rebase_path_ignore_date(), "%s", "");
   	if (opts->reschedule_failed_exec)
   		write_file(rebase_path_reschedule_failed_exec(), "%s", "");
+	else
+		write_file(rebase_path_no_reschedule_failed_exec(), "%s", "");
     	return 0;
   }

t/t3418-rebase-continue.sh +24 -1

index ea14ef496cb..9553d969646 100755
@@ -291,4 +291,29 @@ test_expect_success 'rebase.rescheduleFailedExec only affects `rebase -i`' '
   	git rebase HEAD^
   '
   +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 &&
+	! grep "has been rescheduled" err
+'
+
+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
+'
+
+test_expect_success 'there is no --no-reschedule-failed-exec in an ongoing rebase' '
+	test_when_finished "git rebase --abort" &&
+	test_must_fail git rebase -x false HEAD~2 &&
+	test_expect_code 129 git rebase --continue --no-reschedule-failed-exec &&
+	test_expect_code 129 git rebase --edit-todo --no-reschedule-failed-exec
+'
+
   test_done

I recently started using rebase.rescheduleFailedExec=true and noticed this bug in its implementation.

It's conceptually a relatively simple fix,

but as noted in 3/3 rebase being a "start this operation, run other command verbs later" has an unintuitive interaction with our usual "command-line options override config".

!README FIRST!
Everthing after this line has no relevance to this series, it's just a side musing on another (mis-)feature of --reschedule-failed-exec. !/README FIRST!

[... snip rant about the odd progress counting when rescheduling todo commands ...]

I recently started using rebase.rescheduleFailedExec=true and noticed this bug in its implementation. It's conceptually a relatively simple fix, but as noted in 3/3 rebase being a "start this operation, run other command verbs later" has an unintuitive interaction with our usual "command-line options override config".

!README FIRST!
Everthing after this line has no relevance to this series, it's just a side musing on another (mis-)feature of --reschedule-failed-exec. !/README FIRST!

There's another bug/misfeature I've noticed with rebase.rescheduleFailedExec=true (although maybe it'll be argued by someone that it's a feature). Let's say you run:

     git rebase -x false --reschedule-failed-exec HEAD~2

You'll now land in a state of (according to our helpful PS1 code, but also the rebase state) of 2/4. Aside: Because you're rebasing 2 commits, we have 4 because of the 2x exec. So far so good and all of that's expected.

But now when you do "git rebase --continue" we'll fail as expected, but not as expected (at least I find it funny) bump the count to 3/5, then 4/6, 5/7 etc.

With my "I just read the code" hat on this makes perfect sense. Every time we process a TODO item we bump the count of processed items, and under --reschedule-failed-exec we simply push the command that just failed onto the list, hence the increasing done/TODO count when a command fails.

But I don't see how it makes any sense to expose this as UI via "git status" and __git_ps1. I asked git to process this item of 4 sequencer TODO items. If I fail an item it makes more sense that I just don't get past it, not that under the hood a new identical item is rescheduled for me. Just don't advance past the current item!

Now if I've tried X times to make the "make test" pass for each commit in my 3-commit series I'm going to be on item 10/12 or whatever. There's no way to just look at that and see where I'm at in the sequence.

As an aside it would arguably make more sense to report 1/3 instead of 2/6 for the first commit of 3 with a failing -x "make test", but you can have X number of "exec" items and a manually edited list etc.

So that's probably a no-go but at least once I'm used to it I know if i'm on 4/6 I'm on commit 2/3, with --reschedule-failed-exec you'll have no idea what 12/14 or whatever means for where you are in your 3-patch sequence, it has no relation to the TODO list you edited, just rebase-merge's own internal state.

Getting back on topic: This just seems like needlessly exposing an implementation detail, I also asked to "pick" a commit, but if that item "fails" e.g. due to:

     $ git rebase --continue
     You must edit all merge conflicts and then
     mark them as resolved using git add

We don't push a new "pick" item on the list and inflate the count, so why would we do that for "exec"?

Just say the command failed, return
its non-zero status from "git rebase --continue", and don't advance.

Maybe it is useful to keep track of the N number of failures, and e.g. report in __git_ps1:

     master|REBASE 2/6
     master|REBASE 2(tries: 1)/6
     master|REBASE 2(tries: 2)/6

Instead of the current:

     master|REBASE 2/6
     master|REBASE 3/7
     master|REBASE 4/8

To say I'm on 2/6, but that I've tried 3 times already and failed to advance past it.

Anyway, I don't have patches for this side-report/rant. Looking at the implementation it seemed more non-trivial than I was willing to quickly fix.

We bump the count fairly early before we even get to there being an "exec" item, to implement this proposed view of the world we'd need to defer that (or go back and edit it once we see "failed exec" and that we're using --reschedule-failed-exec).

I'm not familiar enough with the sequencer internals to know if trying that would lead us down a path of e.g. having inconsistent or bad state if we'd die in the middle of all of that.

Ævar Arnfjörð Bjarmason (3):
rebase tests: camel-case rebase.rescheduleFailedExec consistently rebase tests: use test_unconfig after test_config
rebase: don't override --no-reschedule-failed-exec with config

  Documentation/git-rebase.txt |  8 ++++++++
  sequencer.c                  |  5 +++++
  t/t3418-rebase-continue.sh   | 30 ++++++++++++++++++++++++++++--
  3 files changed, 41 insertions(+), 2 deletions(-)

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

The idea was brought up by Paul Morelle.

To be honest, this idea of rescheduling a failed exec makes so much sense that I wish we had done this from the get-go.

Am 10.12.18 um 20:04 schrieb Johannes Schindelin via GitGitGadget:

Quoted 4 lines

The idea was brought up by Paul Morelle.

To be honest, this idea of rescheduling a failed exec makes so much sense that I wish we had done this from the get-go.

The status quo was actually not that bad a decision, because it made 'x false' as a substitute for 'break' very convenient.

But now that we have a real 'break', I'm very much in favor of flipping the behavior over to rescheduling. (I'm actually not a user of the feature, but the proposed behavior is so compellingly logical.)

I wonder if it might be better to push this mechanism one layer down: Instead of having a flag that changes the behavior of the "exec" instructions and having a handy '-y' short cut for the new mode, we'd rather have a new type of command that executes&retries a command
...
By having two classes, I would anticipate fewer compatibility issues ('"Exec" behaves differently, and I forgot I had turned on the rescheduling').

Quoted 10 lines · Johannes Sixt

Am 10.12.18 um 20:04 schrieb Johannes Schindelin via GitGitGadget:

Quoted 4 lines

The idea was brought up by Paul Morelle.

To be honest, this idea of rescheduling a failed exec makes so much sense that I wish we had done this from the get-go.

The status quo was actually not that bad a decision, because it made 'x false' as a substitute for 'break' very convenient.

Quoted 3 lines

But now that we have a real 'break', I'm very much in favor of flipping the behavior over to rescheduling. (I'm actually not a user of the feature, but the proposed behavior is so compellingly logical.)

In rare cases I had commands that may be dangerous if rerun, but I'd just not run them with -y but with -x.

That brings me to some confusion I had in the last patch:
To catch a flaky test I surely would be tempted to
git rebase -x make -y "make test"
but that might reschedule a compile failure as well, as a single -y has implications on all other -x's.

I wonder if it might be better to push this mechanism one layer down: Instead of having a flag that changes the behavior of the "exec" instructions and having a handy '-y' short cut for the new mode, we'd rather have a new type of command that executes&retries a command ("exnrt", 'n'), which can still get the '-y' command line flag, but more importantly by having 2 separate sets of commands we'd have one set that is a one-shot, and the other that is retried. Then we can teach the user which is safe and which isn't for rescheduling.

By having two classes, I would anticipate fewer compatibility issues ('"Exec" behaves differently, and I forgot I had turned on the rescheduling').

Talking about rescheduling: In the above example the flaky test can flake more than once, so I'd be stuck with keeping 'git rebase --continue'ing after I see the test flaked once again.

My workflow with interactive rebase and fixing up things as I go always involves a manual final "make test" to check "for real", which I could lose now, which is nice.

It is amazing to me how much my perspective changed when I actually had to teach Git to new users. Things that I live with easily all of a sudden become these unnecessarily confusing road blocks that make it *so hard* to actually use Git.

My workflow with git rebase -r --exec "make test" is pretty similar to yours. With the difference that I would want those commands to be rescheduled *even if* the command is flakey. Actually, *in particular* when it is flakey.

I really see myself calling

        git config --global rebase.rescheduleFailedExec true

The idea was brought up by Paul Morelle.

To be honest, this idea of rescheduling a failed exec makes so much sense that I wish we had done this from the get-go.

So let's do the next best thing: implement a command-line option and a config setting to make it so.

The obvious intent behind that config setting is to not only give users a way to opt into that behavior already, but also to make it possible to flip the default at a later date, after the feature has been battle-tested and after deprecating the non-rescheduling behavior for a couple of Git versions.

If the team agrees with my reasoning, then the 3rd patch (introducing -y <cmd> as a shortcut for --reschedule-failed-exec -x <cmd>) might not make much sense, as it would introduce a short option that would become obsolete soon.

Johannes Schindelin (3):
rebase: introduce --reschedule-failed-exec
rebase: add a config option to default to --reschedule-failed-exec
rebase: introduce a shortcut for --reschedule-failed-exec

 Documentation/config/rebase.txt |  5 ++++
 Documentation/git-rebase.txt    | 11 +++++++++
 builtin/rebase--interactive.c   |  2 ++
 builtin/rebase.c                | 42 ++++++++++++++++++++++++++++++++-
 git-legacy-rebase.sh            | 24 ++++++++++++++++++-
 git-rebase--common.sh           |  1 +
 sequencer.c                     | 13 +++++++---
 sequencer.h                     |  1 +
 t/t3418-rebase-continue.sh      | 14 +++++++++++
 9 files changed, 108 insertions(+), 5 deletions(-)

base-commit: a1598010f775d82b5adf12c29d0f5bc9b41434c6
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-90%2Fdscho%2Freschedule-failed-exec-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-90/dscho/reschedule-failed-exec-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/90

Signature · 1 line

gitgitgadget

The idea was brought up by Paul Morelle.

To be honest, this idea of rescheduling a failed exec makes so much sense that I wish we had done this from the get-go.

So let's do the next best thing: implement a command-line option and a config setting to make it so.

The obvious intent behind that config setting is to not only give users a way to opt into that behavior already, but also to make it possible to flip the default at a later date, after the feature has been battle-tested and after deprecating the non-rescheduling behavior for a couple of Git versions.

If the team agrees with my reasoning, then the 3rd patch (introducing -y <cmd> as a shortcut for --reschedule-failed-exec -x <cmd>) might not make much sense, as it would introduce a short option that would become obsolete soon.

Quoted 15 lines · Johannes Schindelin via GitGitGadget

The idea was brought up by Paul Morelle.

To be honest, this idea of rescheduling a failed exec makes so much sense that I wish we had done this from the get-go.

So let's do the next best thing: implement a command-line option and a config setting to make it so.

The obvious intent behind that config setting is to not only give users a way to opt into that behavior already, but also to make it possible to flip the default at a later date, after the feature has been battle-tested and after deprecating the non-rescheduling behavior for a couple of Git versions.

Quoted 5 lines

If the team agrees with my reasoning, then the 3rd patch (introducing -y <cmd> as a shortcut for --reschedule-failed-exec -x <cmd>) might not make much sense, as it would introduce a short option that would become obsolete soon.

Complete side-track: This email showed up for me just five minutes ago, whereas the rest of the series showed up four hours ago, making me think this email had disappeared and trying to figure out how to respond when I didn't have the original. Any ideas why there might be that level of lag?