git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] rebase: add `--update-refs=interactive`

From
D. Ben Knoble <ben.knoble@gmail.com>
Date
Feb 10, 2025, 20:22 UTC
Message-ID
<CALnO6CAM7WCOJV8s8ZARi3BAFwkh0TNTCod_YH9s+EpO7t-Qtg@mail.gmail.com>
In-Reply-To
<20250210191650.316329-1-intelfx@intelfx.name>
On Mon, Feb 10, 2025 at 2:17 PM Ivan Shapovalov <intelfx@intelfx.name> wrote:
Show 53 quoted lines
>
> In rebase-heavy workflows involving multiple interdependent feature
> branches, typing out `--update-refs` quickly becomes tiring, which
> can be mitigated with setting the `rebase.updateRefs` git-config option
> to perform update-refs by default.
>
> However, the utility of `rebase.updateRefs` is somewhat limited because
> you rarely want it in a non-interactive rebase (as it does not give you
> the chance to review the update-refs candidates, likely leading to
> updating refs that you didn't want updated -- I made quite an amount
> of mess by setting this option and subsequently forgetting about it).
>
> Try to find a middle ground by introducing a third value,
> `--update-refs=interactive` (and `rebase.updateRefs=interactive`)
> which means `--update-refs` when starting an interactive rebase and
> `--no-update-refs` otherwise. This option is primarily intended to be
> used in the gitconfig, but is also accepted on the command line
> for completeness.
>
> Signed-off-by: Ivan Shapovalov <intelfx@intelfx.name>
> ---
>  Documentation/config/rebase.txt |  7 +++-
>  Documentation/git-rebase.txt    |  8 +++-
>  builtin/rebase.c                | 72 +++++++++++++++++++++++++++++----
>  3 files changed, 77 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/config/rebase.txt b/Documentation/config/rebase.txt
> index c6187ab28b..d8bbaba69a 100644
> --- a/Documentation/config/rebase.txt
> +++ b/Documentation/config/rebase.txt
> @@ -24,7 +24,12 @@ rebase.autoStash::
>         Defaults to false.
>
>  rebase.updateRefs::
> -       If set to true enable `--update-refs` option by default.
> +       If set to true, enable the `--update-refs` option of
> +       linkgit:git-rebase[1] by default. When set to 'interactive',
> +       only enable `--update-refs` by default for interactive mode
> +       (equivalent to `--update-refs=interactive`).
> +       This option can be overridden by specifying any form of
> +       `--update-refs` on the command line.
>
>  rebase.missingCommitsCheck::
>         If set to "warn", git rebase -i will print a warning if some
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index b18cdbc023..ae6939588d 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -647,12 +647,18 @@ rebase --continue` is invoked. Currently, you cannot pass
>
>  --update-refs::
>  --no-update-refs::
> +--update-refs=interactive::

Based on `git grep -e '--.*\[=' Documentation/git-*.txt`, I think this should be more like

    --update-refs[=interactive]::
    --no-update-refs::
But maybe that unintentionally suggests that `=interactive` is the default?
Show 153 quoted lines
>         Automatically force-update any branches that point to commits that
>         are being rebased. Any branches that are checked out in a worktree
>         are not updated in this way.
>  +
> +If `--update-refs=interactive` is specified, the behavior is equivalent to
> +`--update-refs` if the rebase is interactive and `--no-update-refs` otherwise.
> +(This is mainly useful as a configuration setting, although it might also be
> +of use in aliases.)
> ++
>  If the configuration variable `rebase.updateRefs` is set, then this option
> -can be used to override and disable this setting.
> +can be used to override or disable the configuration.
>  +
>  See also INCOMPATIBLE OPTIONS below.
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 6c9eaf3788..57b456599b 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -129,10 +129,17 @@ struct rebase_options {
>         int reschedule_failed_exec;
>         int reapply_cherry_picks;
>         int fork_point;
> -       int update_refs;
> +       // UPDATE_REFS_{UNKNOWN,NO,ALWAYS} numeric values must never
> +       // change as post-option-parsing code works with {,config_}update_refs
> +       // as if they were ints
> +       enum {
> +               UPDATE_REFS_UNKNOWN = -1,
> +               UPDATE_REFS_NO = 0,
> +               UPDATE_REFS_ALWAYS = 1,
> +               UPDATE_REFS_INTERACTIVE,
> +       } update_refs, config_update_refs;
>         int config_autosquash;
>         int config_rebase_merges;
> -       int config_update_refs;
>  };
>
>  #define REBASE_OPTIONS_INIT {                          \
> @@ -150,8 +157,8 @@ struct rebase_options {
>                 .autosquash = -1,                       \
>                 .rebase_merges = -1,                    \
>                 .config_rebase_merges = -1,             \
> -               .update_refs = -1,                      \
> -               .config_update_refs = -1,               \
> +               .update_refs = UPDATE_REFS_UNKNOWN,     \
> +               .config_update_refs = UPDATE_REFS_UNKNOWN, \
>                 .strategy_opts = STRING_LIST_INIT_NODUP,\
>         }
>
> @@ -412,6 +419,18 @@ static void imply_merge(struct rebase_options *opts, const char *option)
>         }
>  }
>
> +static int coerce_update_refs(const struct rebase_options *opts, int update_refs)
> +{
> +       /* coerce "=interactive" into "no" rather than "not set" when not interactive
> +        * this way, `git -c rebase.updateRefs=yes rebase --update-refs=interactive [without -i]`
> +        * will not inherit the "yes" from the config */
> +       if (update_refs == UPDATE_REFS_INTERACTIVE)
> +               return (opts->flags & REBASE_INTERACTIVE_EXPLICIT)
> +                      ? UPDATE_REFS_ALWAYS
> +                      : UPDATE_REFS_NO;
> +       return update_refs;
> +}
> +
>  /* Returns the filename prefixed by the state_dir */
>  static const char *state_dir_path(const char *filename, struct rebase_options *opts)
>  {
> @@ -779,6 +798,17 @@ static void parse_rebase_merges_value(struct rebase_options *options, const char
>                 die(_("Unknown rebase-merges mode: %s"), value);
>  }
>
> +static int parse_update_refs_value(const char *value, const char *desc)
> +{
> +       int v = git_parse_maybe_bool(value);
> +       if (v >= 0)
> +               return v ? UPDATE_REFS_ALWAYS : UPDATE_REFS_NO;
> +       else if (!strcmp("interactive", value))
> +               return UPDATE_REFS_INTERACTIVE;
> +
> +       die(_("bad %s value '%s'; valid values are boolean or \"interactive\""), desc, value);
> +}
> +
>  static int rebase_config(const char *var, const char *value,
>                          const struct config_context *ctx, void *data)
>  {
> @@ -821,7 +851,8 @@ static int rebase_config(const char *var, const char *value,
>         }
>
>         if (!strcmp(var, "rebase.updaterefs")) {
> -               opts->config_update_refs = git_config_bool(var, value);
> +               opts->config_update_refs = parse_update_refs_value(value,
> +                       "rebase.updateRefs");
>                 return 0;
>         }
>
> @@ -1042,6 +1073,19 @@ static int parse_opt_rebase_merges(const struct option *opt, const char *arg, in
>         return 0;
>  }
>
> +static int parse_opt_update_refs(const struct option *opt, const char *arg, int unset)
> +{
> +       struct rebase_options *options = opt->value;
> +
> +       if (arg)
> +               options->update_refs = parse_update_refs_value(arg,
> +                       "--update-refs");
> +       else
> +               options->update_refs = unset ? UPDATE_REFS_NO : UPDATE_REFS_ALWAYS;
> +
> +       return 0;
> +}
> +
>  static void NORETURN error_on_missing_default_upstream(void)
>  {
>         struct branch *current_branch = branch_get(NULL);
> @@ -1187,9 +1231,11 @@ int cmd_rebase(int argc,
>                 OPT_BOOL(0, "autosquash", &options.autosquash,
>                          N_("move commits that begin with "
>                             "squash!/fixup! under -i")),
> -               OPT_BOOL(0, "update-refs", &options.update_refs,
> -                        N_("update branches that point to commits "
> -                           "that are being rebased")),
> +               OPT_CALLBACK_F(0, "update-refs", &options,
> +                       N_("(bool|interactive)"),
> +                       N_("update branches that point to commits "
> +                          "that are being rebased"),
> +                       PARSE_OPT_OPTARG, parse_opt_update_refs),
>                 { OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
>                         N_("GPG-sign commits"),
>                         PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
> @@ -1528,6 +1574,16 @@ int cmd_rebase(int argc,
>         if (isatty(2) && options.flags & REBASE_NO_QUIET)
>                 strbuf_addstr(&options.git_format_patch_opt, " --progress");
>
> +       /* coerce --update-refs=interactive into yes or no.
> +        * we do it here because there's just too much code below that handles
> +        * {,config_}update_refs in one way or another and modifying it to
> +        * account for the new state would be too invasive.
> +        * all further code uses {,config_}update_refs as a tristate. */
> +       options.update_refs =
> +               coerce_update_refs(&options, options.update_refs);
> +       options.config_update_refs =
> +               coerce_update_refs(&options, options.config_update_refs);
> +
>         if (options.git_am_opts.nr || options.type == REBASE_APPLY) {
>                 /* all am options except -q are compatible only with --apply */
>                 for (i = options.git_am_opts.nr - 1; i >= 0; i--)
> --
> 2.48.1.5.g9188e14f140
>
>
Should we add a test for this?
-- 
D. Ben Knoble
Previous: Ivan ShapovalovNext: Ivan Shapovalov
Message 2 of 16 in “rebase: add `--update-refs=interactive`”
  1. rebase: add `--update-refs=interactive`Ivan Shapovalov, Feb 10, 2025
  2. D. Ben KnobleFeb 10, 2025
  3. Ivan ShapovalovFeb 11, 2025
  4. Junio C HamanoFeb 11, 2025
  5. Ivan ShapovalovFeb 11, 2025
  6. D. Ben KnobleFeb 11, 2025
  7. D. Ben KnobleFeb 11, 2025
  8. Phillip WoodFeb 11, 2025
  9. Ivan ShapovalovFeb 11, 2025
  10. Phillip WoodFeb 12, 2025
  11. Junio C HamanoFeb 12, 2025
  12. Phillip WoodFeb 13, 2025
  13. Ivan ShapovalovFeb 12, 2025
  14. phillip.wood123@gmail.comFeb 13, 2025
  15. Ivan ShapovalovFeb 13, 2025
  16. phillip.wood123@gmail.comFeb 19, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.