Mehul Jain <mehul.jain2...@gmail.com> writes:
> git pull --rebase understands --[no-]autostash flag.
>
> This flag overrides config variable "rebase.autoStash"
> if set (default is false).
Is that a statement of a fact? If so, is it true before this patch
is applied, or after?
Each project has local convention for log messages, and we do too.
A long log message typically start by explaining how the world is,
why that is not desirable, present a description of a possible world
that is better, and then give commands to somebody who is updating
the code telling him what to do to make that better world a reality
(and optionally how).
So perhaps (I am totally making this up; you need to fact check and
adjust):
If you enable rebase.autoStash option in your repository, there
is no way to override it for "git pull --rebase" from the
command line.
Teach "git pull" a new "--[no-]autostash" option so that a
rebase.autoStash configuration can be overridden. As "git
rebase" already knows "--[no-]autostash" option, it is just the
matter of passing one when we spawn the command as necessary.
or something. The first one gives the readers how the current world
works, and why it is not ideal. The proposed better world in this
case is too simple--the first paragraph complained that "we cannot
do X" and X is something reader would likely to agree is a good
thing to do, so it can be left unsaid that a better world is one in
which X can be done.
> When calling "git pull --rebase" with "--autostash",
> pull passes the "--autostash" option to rebase,
> which then runs rebase on a dirty worktree.
>
> With "--no-autostash" option, the command will die
> if the worktree is dirty, before calling rebase.
These two paragraphs are too obvious and probably are better left
unsaid. Especially the latter--you are changing "pull" and do not
control what "rebase" would do in the future. It could be that a
better rebase in the future may be able to do its job in a dirty
worktree without doing an autostash.
> Signed-off-by: Mehul Jain <mehul.jain2...@gmail.com>
> ---
>
> Thanks to Paul and Matthieu for comments on previous round.
> Changes:
> - --autostash flag added
> - OPT_COLOR_FLAG replaced by OPT_BOOL
> - Default value of opt_rebase changed
> - Few changes in code
> - Commit message improvements
> - Documentation added
> - Few tests removed as suggested by Paul
> - Added test for --autostash flag
> All tests passed: https://travis-ci.org/mehul2029/git
>
> builtin/pull.c | 13 ++++++++-----
> t/t5520-pull.sh | 19 +++++++++++++++++++
> t/t5521-pull-options.sh | 16 ++++++++++++++++
> 3 files changed, 43 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 10eff03..60b320e 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -85,6 +85,7 @@ static char *opt_squash;
> static char *opt_commit;
> static char *opt_edit;
> static char *opt_ff;
> +static int opt_autostash = 0;
Do not explicitly initialize a static to 0 (or NULL).
> static char *opt_verify_signatures;
> static struct argv_array opt_strategies = ARGV_ARRAY_INIT;
> static struct argv_array opt_strategy_opts = ARGV_ARRAY_INIT;
> @@ -146,6 +147,8 @@ static struct option pull_options[] = {
> OPT_PASSTHRU(0, "ff-only", &opt_ff, NULL,
> N_("abort if fast-forward is not possible"),
> PARSE_OPT_NOARG | PARSE_OPT_NONEG),
> + OPT_BOOL(0,"autostash",&opt_autostash,
> + N_("automatically stash/stash pop before and after rebase")),
> OPT_PASSTHRU(0, "verify-signatures", &opt_verify_signatures, NULL,
> N_("verify that the named commit has a valid GPG signature"),
> PARSE_OPT_NOARG),
> @@ -789,7 +792,8 @@ static int run_rebase(const unsigned char *curr_head,
> argv_array_pushv(&args, opt_strategy_opts.argv);
> if (opt_gpg_sign)
> argv_array_push(&args, opt_gpg_sign);
> -
> + if(opt_autostash)
Style: control keywords are followed by a single SP before the next '('.
> + argv_array_push(&args,"--autostash");
Style: a single SP after a comma.
How would --no-autostash defeat a configured rebase.autostash with this?
By the way, how would this affect "git pull --autostash" that is run
without "--rebase"? If this is an option to "git pull", shouldn't
the stashing done even when you are not doing a rebase but making a
merge?
> argv_array_push(&args, "--onto");
> argv_array_push(&args, sha1_to_hex(merge_head));
>
> @@ -813,6 +817,8 @@ int cmd_pull(int argc, const char **argv, const char
> *prefix)
> if (!getenv("GIT_REFLOG_ACTION"))
> set_reflog_message(argc, argv);
>
> + git_config_get_bool("rebase.autostash",&opt_autostash);
> +
Why is this change even necessary?
> argc = parse_options(argc, argv, prefix, pull_options, pull_usage, 0);
>
> parse_repo_refspecs(argc, argv, &repo, &refspecs);
> @@ -835,13 +841,10 @@ int cmd_pull(int argc, const char **argv, const char
> *prefix)
> hashclr(orig_head);
>
> if (opt_rebase) {
> - int autostash = 0;
> -
> if (is_null_sha1(orig_head) && !is_cache_unborn())
> die(_("Updating an unborn branch with changes added to
> the index."));
>
> - git_config_get_bool("rebase.autostash", &autostash);
> - if (!autostash)
> + if (!opt_autostash)
> die_on_unclean_work_tree(prefix);
I would have expected that
* a global opt_autostash is initialized to -1 (unspecified);
* opt_bool() would flip it to either 0 or 1 with --[no-]autostash;
* existing "rebase.autostash" configuration check inside "git pull"
code gets removed;
* and the code that builds "git rebase" invocation command line
will do
if (opt_autostash < 0)
; /* do nothing */
else if (opt_autostash == 0)
argv_array_push(&args, "--no-autostash");
else
argv_array_push(&args, "--autostash");
Then when "git pull --rebase" is run without "--[no-]autostash", the
underlying "git rebase" would be run without that option, and does its
usual thing, including reading rebase.autostash and deciding to do
"git stash". And when "git pull" is run with "--[no-]autostash",
the underlying "git rebase" would be given the same option, and
would do what it was told to do, ignoring rebase.autostash setting.
So why does "git pull" still need to look at rebase.autostash
configuration after this change?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
