[PATCH v5 0/2] checkout -m: refine autostash fallback

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

Avoiding checkout -m autostash retries when no tracked local changes exist
and visually separating autostash conflict advice from the subsequent
branch-switch message. Addresses #leftoverbits from here:
https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/

Changes in v5:

 * Improve commit messages.
 * Create helper do_stash_apply.

Changes in v4:

 * Conflicts now exit with status 1 like merge-tree, other failures exit 128
   so exit 1 unambiguously means conflicts. Stash changes split into their
   own commit.
 * The autostash apply helpers use the return value (enum
   stash_apply_result) instead of an out-parameter, and only claim conflicts
   when git stash apply actually reported them.

Changes in v3:

 * Use enum for git stash return values, to separate conflict from generic
   error.

Changes in v2:

 * Simplify logic and combine to one commit.
 * Test full output with test_cmp.

Harald Nordgren (2):
  stash: reserve exit status 1 for conflicts
  checkout: separate autostash conflict advice from branch-switch
    message

 Documentation/git-stash.adoc |   9 +++
 builtin/checkout.c           |  15 +++--
 builtin/stash.c              |  33 +++++++---
 sequencer.c                  | 113 ++++++++++++++++++++++-------------
 sequencer.h                  |  19 +++---
 stash.h                      |  21 +++++++
 t/t3903-stash.sh             |  25 +++++++-
 t/t7201-co.sh                |  16 +++--
 8 files changed, 181 insertions(+), 70 deletions(-)
 create mode 100644 stash.h


base-commit: 3cb9185f65410273787f74333cc027d2ea5daada
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v5
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v4:

 1:  ff43221802 ! 1:  fe22b1bfa6 stash: reserve exit status 1 for conflicts
     @@ Commit message
          status die() uses, when they fail for other reasons.  Document the
          exit statuses.
      
     -    cmd_stash() used to collapse the return values of the subcommand
     -    implementations to a boolean.  It now maps negative values, which
     -    signal a failure, to 128 and passes everything else through as-is.
     -    The only implementations that return a positive value are "apply",
     -    "pop" and "branch", which return the value of do_apply_stash():
     -    "apply" returns it directly, and "pop" and "branch" drop the stash
     -    entry, via do_drop_stash(), which always returns 0, only when the
     -    application succeeded.  The positive value is always 1, as
     -    do_apply_stash() only returns a positive value when the three-way
     -    merge was unclean.
     -
     -    Make the convention explicit by introducing enum stash_apply_result
     -    with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
     -    STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
     -    too.  They spawn "git stash apply" and can now tell conflicts apart
     -    from other failures, e.g. a crash or death by signal of the child,
     -    which map to exit statuses above 1.  Since we know the stash entry
     -    was saved, tell users so in the error message instead of leaving them
     -    wondering what happened to their stashed changes.
     +    The only subcommand implementations that can return a positive value
     +    are "apply", "pop" and "branch", which return the value of
     +    do_apply_stash(): "apply" returns it directly, and "pop" and "branch"
     +    drop the stash entry, via do_drop_stash(), which always returns 0,
     +    only when the application succeeded.  do_apply_stash() only returns a
     +    positive value when the three-way merge was unclean.  cmd_stash() now
     +    maps negative values to 128 and passes positive values through as the
     +    exit status, so exit status 1 unambiguously indicates conflicts.
     +    enum stash_apply_result makes the convention explicit, and the
     +    autostash helpers use it to tell users that their stashed changes
     +    were saved when applying them fails.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ builtin/stash.c: int cmd_stash(int argc,
      +		if (ret < 0)
      +			return 128;
      +		return ret;
     -+	} else if (!argc)
     ++	} else if (!argc) {
       		return !!push_stash_unassumed(0, NULL, prefix, repo);
     ++	}
       
       	/* Assume 'stash push' */
     + 	strvec_push(&args, "push");
      
       ## sequencer.c ##
      @@
     @@ sequencer.c: void create_autostash_ref(struct repository *r, const char *refname
      -				    const char *label_ours, const char *label_theirs,
      -				    const char *label_base,
      -				    const char *stash_msg)
     ++static enum stash_apply_result do_stash_apply(const char *stash_oid,
     ++					      const char *label_ours,
     ++					      const char *label_theirs,
     ++					      const char *label_base)
     + {
     + 	struct child_process child = CHILD_PROCESS_INIT;
     +-	int ret = 0;
     + 
     +-	if (attempt_apply) {
     +-		child.git_cmd = 1;
     +-		child.no_stdout = 1;
     +-		child.no_stderr = 1;
     +-		strvec_push(&child.args, "stash");
     +-		strvec_push(&child.args, "apply");
     +-		if (label_ours)
     +-			strvec_pushf(&child.args, "--label-ours=%s", label_ours);
     +-		if (label_theirs)
     +-			strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
     +-		if (label_base)
     +-			strvec_pushf(&child.args, "--label-base=%s", label_base);
     +-		strvec_push(&child.args, stash_oid);
     +-		ret = run_command(&child);
     +-	}
     +-
     +-	if (attempt_apply && !ret)
     ++	child.git_cmd = 1;
     ++	child.no_stdout = 1;
     ++	child.no_stderr = 1;
     ++	strvec_push(&child.args, "stash");
     ++	strvec_push(&child.args, "apply");
     ++	if (label_ours)
     ++		strvec_pushf(&child.args, "--label-ours=%s", label_ours);
     ++	if (label_theirs)
     ++		strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
     ++	if (label_base)
     ++		strvec_pushf(&child.args, "--label-base=%s", label_base);
     ++	strvec_push(&child.args, stash_oid);
     ++
     ++	switch (run_command(&child)) {
     ++	case 0:
     ++		return STASH_APPLY_CLEAN;
     ++	case STASH_APPLY_CONFLICT:
     ++		return STASH_APPLY_CONFLICT;
     ++	default:
     ++		return STASH_APPLY_ERROR;
     ++	}
     ++}
     ++
      +static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
      +							int attempt_apply,
      +							const char *label_ours,
      +							const char *label_theirs,
      +							const char *label_base,
      +							const char *stash_msg)
     - {
     - 	struct child_process child = CHILD_PROCESS_INIT;
     --	int ret = 0;
     ++{
      +	enum stash_apply_result ret = STASH_APPLY_CLEAN;
     - 
     - 	if (attempt_apply) {
     - 		child.git_cmd = 1;
     -@@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     - 			strvec_pushf(&child.args, "--label-base=%s", label_base);
     - 		strvec_push(&child.args, stash_oid);
     - 		ret = run_command(&child);
     -+		if (ret > 1)
     -+			ret = STASH_APPLY_ERROR;
     - 	}
     - 
     --	if (attempt_apply && !ret)
     ++
     ++	if (attempt_apply)
     ++		ret = do_stash_apply(stash_oid, label_ours, label_theirs,
     ++				     label_base);
     ++
      +	if (attempt_apply && ret == STASH_APPLY_CLEAN)
       		fprintf(stderr, _("Applied autostash.\n"));
       	else {
 2:  935fa0a9ae = 2:  d18ff3ea9a checkout: separate autostash conflict advice from branch-switch message

-- 
gitgitgadget