{"thread":{"id":"33255","subject":"[PATCH 0/3] Introduce pull.autostash","startedAt":"2013-03-22T12:29:56Z","lastAt":"2013-03-24T21:12:24Z","messageCount":11,"participants":["Ramkumar Ramachandra","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"211931","messageId":"1363955399-13153-1-git-send-email-artagnon@gmail.com","threadId":"33255","inReplyTo":null,"subject":"[PATCH 0/3] Introduce pull.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-22T12:29:56Z","receivedAt":"2013-03-22T12:29:56Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"This series adds a pull.autostash, making 'git pull' a tad bit more\nusable.  It is part of my bigger plan to make 'git pull' just DTRT\n(via configuration variables) in 90% of the cases.\n\n[1/3] and [2/3] are tiny \"while we're here\" patches.\n[3/3] is the main patch with tests.\n\nThanks for reading.\n\nRamkumar Ramachandra (3):\n  git-pull.sh: prefer invoking \"git <command>\" over \"git-<command>\"\n  t5521 (pull-options): use test_commit() where appropriate\n  git-pull.sh: introduce --[no-]autostash and pull.autostash\n\n Documentation/config.txt   |  5 ++++\n Documentation/git-pull.txt |  7 ++++++\n git-pull.sh                | 30 +++++++++++++++++++---\n t/t5521-pull-options.sh    | 63 ++++++++++++++++++++++++++++++++++++++++++++--\n 4 files changed, 99 insertions(+), 6 deletions(-)\n\n-- \n1.8.2.141.gad203c2.dirty\n"},{"id":"211933","messageId":"1363955399-13153-2-git-send-email-artagnon@gmail.com","threadId":"33255","inReplyTo":"1363955399-13153-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/3] git-pull.sh: prefer invoking \"git <command>\" over \"git-<command>\"","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-22T12:29:57Z","receivedAt":"2013-03-22T12:29:57Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"14e5d40c (pull: Fix parsing of -X<option>, 2010-01-17) added the lines\ncontaining \"git-push\" and \"git-merge\", even though the prevelant style\nat the time was to use the unhyphenated \"git <command>\" form.  Fix\nthis.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n git-pull.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-pull.sh b/git-pull.sh\nindex 266e682..37e1cd4 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -279,11 +279,11 @@ fi\n merge_name=$(git fmt-merge-msg $log_arg <\"$GIT_DIR/FETCH_HEAD\") || exit\n case \"$rebase\" in\n true)\n-\teval=\"git-rebase $diffstat $strategy_args $merge_args\"\n+\teval=\"git rebase $diffstat $strategy_args $merge_args\"\n \teval=\"$eval --onto $merge_head ${oldremoteref:-$merge_head}\"\n \t;;\n *)\n-\teval=\"git-merge $diffstat $no_commit $edit $squash $no_ff $ff_only\"\n+\teval=\"git merge $diffstat $no_commit $edit $squash $no_ff $ff_only\"\n \teval=\"$eval  $log_arg $strategy_args $merge_args $verbosity $progress\"\n \teval=\"$eval \\\"\\$merge_name\\\" HEAD $merge_head\"\n \t;;\n-- \n1.8.2.141.gad203c2.dirty\n"},{"id":"211934","messageId":"1363955399-13153-3-git-send-email-artagnon@gmail.com","threadId":"33255","inReplyTo":"1363955399-13153-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/3] t5521 (pull-options): use test_commit() where appropriate","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-22T12:29:58Z","receivedAt":"2013-03-22T12:29:58Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"test_commit() is a well-defined function in test-lib-functions.sh that\nallows you to create commits with a terse syntax.  Prefer using it\nover creating commits by hand.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t5521-pull-options.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex 1b06691..4a804f0 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -7,8 +7,8 @@ test_description='pull options'\n test_expect_success 'setup' '\n \tmkdir parent &&\n \t(cd parent && git init &&\n-\t echo one >file && git add file &&\n-\t git commit -m one)\n+\t test_commit \"one\" file \"one\"\n+\t)\n '\n \n test_expect_success 'git pull -q' '\n-- \n1.8.2.141.gad203c2.dirty\n"},{"id":"211932","messageId":"1363955399-13153-4-git-send-email-artagnon@gmail.com","threadId":"33255","inReplyTo":"1363955399-13153-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-22T12:29:59Z","receivedAt":"2013-03-22T12:29:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"This new configuration variable executes 'git stash' before attempting\nto merge/rebase, and 'git stash pop' after a successful merge/rebase.\nIt makes it convenient for people to pull with dirty worktrees.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/config.txt   |  5 ++++\n Documentation/git-pull.txt |  7 ++++++\n git-pull.sh                | 26 ++++++++++++++++++--\n t/t5521-pull-options.sh    | 59 ++++++++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 95 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex c1f435f..0becafe 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1786,6 +1786,11 @@ pull.rebase::\n \tof merging the default branch from the default remote when \"git\n \tpull\" is run. See \"branch.<name>.rebase\" for setting this on a\n \tper-branch basis.\n+\n+pull.autostash::\n+\tWhen true, automatically stash all changes before attempting to\n+\tmerge/rebase, and pop the stash after a successful\n+\tmerge/rebase.\n +\n *NOTE*: this is a possibly dangerous operation; do *not* use\n it unless you understand the implications (see linkgit:git-rebase[1]\ndiff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt\nindex c975743..bb57c86 100644\n--- a/Documentation/git-pull.txt\n+++ b/Documentation/git-pull.txt\n@@ -94,6 +94,13 @@ must be given before the options meant for 'git fetch'.\n \thas to be called afterwards to bring the work tree up to date with the\n \tmerge result.\n \n+--[no-]autostash::\n+\tWhen turned on, automatically stash all changes before\n+\tattempting to merge/rebase, and pop the stash after a\n+\tsuccessful merge/rebase.  Useful for people who want to pull\n+\twith a dirty worktree.  This option can also be set via the\n+\t`pull.autostash` configuration variable.\n+\n Options related to merging\n ~~~~~~~~~~~~~~~~~~~~~~~~~~\n \ndiff --git a/git-pull.sh b/git-pull.sh\nindex 37e1cd4..ad8e494 100755\n--- a/git-pull.sh\n+++ b/git-pull.sh\n@@ -48,6 +48,7 @@ if test -z \"$rebase\"\n then\n \trebase=$(git config --bool pull.rebase)\n fi\n+autostash=$(git config --bool pull.autostash)\n dry_run=\n while :\n do\n@@ -116,6 +117,12 @@ do\n \t--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)\n \t\trebase=false\n \t\t;;\n+\t--autostash)\n+\t\tautostash=true\n+\t\t;;\n+\t--no-autostash)\n+\t\tautostash=false\n+\t\t;;\n \t--recurse-submodules)\n \t\trecurse_submodules=--recurse-submodules\n \t\t;;\n@@ -196,7 +203,8 @@ test true = \"$rebase\" && {\n \t\tthen\n \t\t\tdie \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n \t\tfi\n-\telse\n+\telif test \"$autostash\" = false\n+\tthen\n \t\trequire_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n \tfi\n \toldremoteref= &&\n@@ -213,6 +221,12 @@ test true = \"$rebase\" && {\n \t\tfi\n \tdone\n }\n+\n+stash_required () {\n+\t! (git diff-files --quiet --ignore-submodules &&\n+\tgit diff-index --quiet --cached HEAD --ignore-submodules)\n+}\n+\n orig_head=$(git rev-parse -q --verify HEAD)\n git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok \"$@\" || exit 1\n test -z \"$dry_run\" || exit 0\n@@ -288,4 +302,12 @@ true)\n \teval=\"$eval \\\"\\$merge_name\\\" HEAD $merge_head\"\n \t;;\n esac\n-eval \"exec $eval\"\n+\n+if test \"$autostash\" = true && stash_required\n+then\n+\tgit stash\n+\teval \"$eval\"\n+\ttest $? = 0 && git stash pop\n+else\n+\teval \"exec $eval\"\n+fi\ndiff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\nindex 4a804f0..cecacbc 100755\n--- a/t/t5521-pull-options.sh\n+++ b/t/t5521-pull-options.sh\n@@ -90,4 +90,63 @@ test_expect_success 'git pull --all' '\n \t)\n '\n \n+test_expect_success 'pull --autostash with clean worktree' '\n+\tmkdir clonedautostash &&\n+\t(cd clonedautostash &&\n+\tgit init &&\n+\tgit pull --autostash ../parent &&\n+\ttest_must_fail test_path_is_file .git/refs/stash\n+\ttest_commit one\n+\t) &&\n+\trm -rf clonedautostash\n+'\n+\n+test_expect_success 'pull.autostash with clean worktree' '\n+\tmkdir clonedautostash &&\n+\t(cd clonedautostash &&\n+\tgit init &&\n+\ttest_config pull.autostash true &&\n+\tgit pull ../parent &&\n+\ttest_must_fail test_path_is_file .git/refs/stash\n+\ttest_commit one\n+\t) &&\n+\trm -rf clonedautostash\n+'\n+\n+test_expect_success 'pull.autostash without conflict' '\n+\tmkdir clonedautostash &&\n+\t(cd clonedautostash &&\n+\tgit init &&\n+\ttest_commit \"root_commit\" &&\n+\tcat >quux <<-\\EOF &&\n+\tthis is a non-conflicting file\n+\tEOF\n+\tgit add quux &&\n+\ttest_config pull.autostash true &&\n+\tgit pull ../parent &&\n+\ttest_must_fail test_path_is_file .git/refs/stash &&\n+\ttest_path_is_file quux &&\n+\ttest_commit one\n+\t) &&\n+\trm -rf clonedautostash\n+'\n+\n+test_expect_success 'pull.autostash with conflict' '\n+\tmkdir clonedautostash &&\n+\t(cd clonedautostash &&\n+\tgit init &&\n+\ttest_commit \"will_conflict\" file \"this is a conflicting file\" &&\n+\tcat >quux <<-\\EOF &&\n+\tthis is a non-conflicting file\n+\tEOF\n+\tgit add quux &&\n+\ttest_config pull.autostash true &&\n+\ttest_must_fail git pull ../parent &&\n+\ttest_must_fail test_commit one &&\n+\ttest_path_is_file .git/refs/stash &&\n+\ttest_must_fail test_path_is_file quux\n+\t) &&\n+\trm -rf clonedautostash\n+'\n+\n test_done\n-- \n1.8.2.141.gad203c2.dirty\n"},{"id":"211958","messageId":"7vy5df74yt.fsf@alter.siamese.dyndns.org","threadId":"33255","inReplyTo":"1363955399-13153-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/3] t5521 (pull-options): use test_commit() where appropriate","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-22T16:51:38Z","receivedAt":"2013-03-22T16:51:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> test_commit() is a well-defined function in test-lib-functions.sh that\n> allows you to create commits with a terse syntax.  Prefer using it\n> over creating commits by hand.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  t/t5521-pull-options.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\n> index 1b06691..4a804f0 100755\n> --- a/t/t5521-pull-options.sh\n> +++ b/t/t5521-pull-options.sh\n> @@ -7,8 +7,8 @@ test_description='pull options'\n>  test_expect_success 'setup' '\n>  \tmkdir parent &&\n>  \t(cd parent && git init &&\n> -\t echo one >file && git add file &&\n> -\t git commit -m one)\n> +\t test_commit \"one\" file \"one\"\n> +\t)\n>  '\n\nIn this test script perhaps it is OK, but I'd prefer people being\ncareful *not* to use test_commit in tests that involve refs (i.e.\npushing, pulling, ls-remote, for-each-ref, describe...) and paths\n(i.e.  ls-files, diff, ...).\n\nThere is one good point in the helper: it creates a commit with a\npredictable timestamp.\n\nBut it does a lot other *bad* things than that single good thing.\nIt adds a new path, and adds a new tag; neither of which is not\ndesirable in many circumstances.\n\nA better future direction would be to first make these \"frill\"\nfeatures into options to test_commit helper, fix the users that\ndepend on these additional tags and stuff to explicitly ask for\nthem, and then start advocating it for wider use, I think.\n"},{"id":"211960","messageId":"7vtxo374h6.fsf@alter.siamese.dyndns.org","threadId":"33255","inReplyTo":"1363955399-13153-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-22T17:02:13Z","receivedAt":"2013-03-22T17:02:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> @@ -1786,6 +1786,11 @@ pull.rebase::\n>  \tof merging the default branch from the default remote when \"git\n>  \tpull\" is run. See \"branch.<name>.rebase\" for setting this on a\n>  \tper-branch basis.\n> +\n> +pull.autostash::\n> +\tWhen true, automatically stash all changes before attempting to\n> +\tmerge/rebase, and pop the stash after a successful\n> +\tmerge/rebase.\n>  +\n>  *NOTE*: this is a possibly dangerous operation; do *not* use\n>  it unless you understand the implications (see linkgit:git-rebase[1]\n\nIs autosquash a possibly dangerous operation?\n\n> @@ -196,7 +203,8 @@ test true = \"$rebase\" && {\n>  \t\tthen\n>  \t\t\tdie \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n>  \t\tfi\n> -\telse\n> +\telif test \"$autostash\" = false\n> +\tthen\n>  \t\trequire_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n>  \tfi\n\nA safety net, after you run \"git stash\", to validate that the\nadded \"git stash\" indeed made the working tree clean, is necessary\nbelow, but there does not seem to be any.\n\n>  \toldremoteref= &&\n> @@ -213,6 +221,12 @@ test true = \"$rebase\" && {\n>  \t\tfi\n>  \tdone\n>  }\n> +\n> +stash_required () {\n> +\t! (git diff-files --quiet --ignore-submodules &&\n> +\tgit diff-index --quiet --cached HEAD --ignore-submodules)\n> +}\n> +\n>  orig_head=$(git rev-parse -q --verify HEAD)\n>  git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok \"$@\" || exit 1\n>  test -z \"$dry_run\" || exit 0\n> @@ -288,4 +302,12 @@ true)\n>  \teval=\"$eval \\\"\\$merge_name\\\" HEAD $merge_head\"\n>  \t;;\n>  esac\n> -eval \"exec $eval\"\n> +\n> +if test \"$autostash\" = true && stash_required\n> +then\n> +\tgit stash\n> +\teval \"$eval\"\n> +\ttest $? = 0 && git stash pop\n> +else\n> +\teval \"exec $eval\"\n> +fi\n\nDelaying to run \"git stash\" as much as possible is fine, but with\nthe above, can the user tell if something was stashed and she has\nto \"stash pop\" once she is done helping the command to resolve the\nconflicts, or she shouldn't run \"stash pop\" after she is done\n(because if nothing is stashed at this point, that \"pop\" will pop an\nunrelated stash)?\n\nWhat protects the \"git stash\" from failing and how is its error\nhandled?\n"},{"id":"212046","messageId":"CALkWK0n4CT-Pfm22vTToVp2kZqT7h9kBtF-1NoPOg3vOc+MSog@mail.gmail.com","threadId":"33255","inReplyTo":"7vy5df74yt.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/3] t5521 (pull-options): use test_commit() where appropriate","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-23T12:38:22Z","receivedAt":"2013-03-23T12:38:22Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>\n>> test_commit() is a well-defined function in test-lib-functions.sh that\n>> allows you to create commits with a terse syntax.  Prefer using it\n>> over creating commits by hand.\n>>\n>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n>> ---\n>>  t/t5521-pull-options.sh | 4 ++--\n>>  1 file changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh\n>> index 1b06691..4a804f0 100755\n>> --- a/t/t5521-pull-options.sh\n>> +++ b/t/t5521-pull-options.sh\n>> @@ -7,8 +7,8 @@ test_description='pull options'\n>>  test_expect_success 'setup' '\n>>       mkdir parent &&\n>>       (cd parent && git init &&\n>> -      echo one >file && git add file &&\n>> -      git commit -m one)\n>> +      test_commit \"one\" file \"one\"\n>> +     )\n>>  '\n>\n> In this test script perhaps it is OK, but I'd prefer people being\n> careful *not* to use test_commit in tests that involve refs (i.e.\n> pushing, pulling, ls-remote, for-each-ref, describe...) and paths\n> (i.e.  ls-files, diff, ...).\n\nOkay.\n\n> There is one good point in the helper: it creates a commit with a\n> predictable timestamp.\n\nYes, test_tick.  I've noticed that several tests call test_tick by\nhand before invoking \"git commit\".\n\n> But it does a lot other *bad* things than that single good thing.\n> It adds a new path, and adds a new tag; neither of which is not\n> desirable in many circumstances.\n>\n> A better future direction would be to first make these \"frill\"\n> features into options to test_commit helper, fix the users that\n> depend on these additional tags and stuff to explicitly ask for\n> them, and then start advocating it for wider use, I think.\n\nAgreed.  In fact, the commit message is constrained, because of this;\nyou can't create a commit with a commit message involving spaces,\nbecause that would result in an invalid tag name.\n"},{"id":"212049","messageId":"CALkWK0=oOt0teGqCjpDkerR3-t1cY=qKyK-AtoLLCz9L1-+vyw@mail.gmail.com","threadId":"33255","inReplyTo":"7vtxo374h6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-23T12:48:00Z","receivedAt":"2013-03-23T12:48:00Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>\n>> @@ -1786,6 +1786,11 @@ pull.rebase::\n>>       of merging the default branch from the default remote when \"git\n>>       pull\" is run. See \"branch.<name>.rebase\" for setting this on a\n>>       per-branch basis.\n>> +\n>> +pull.autostash::\n>> +     When true, automatically stash all changes before attempting to\n>> +     merge/rebase, and pop the stash after a successful\n>> +     merge/rebase.\n>>  +\n>>  *NOTE*: this is a possibly dangerous operation; do *not* use\n>>  it unless you understand the implications (see linkgit:git-rebase[1]\n>\n> Is autosquash a possibly dangerous operation?\n\nOops.  I must admit I was in a bit of a hurry with the documentation.\nI was eager to send out the series seeing that the tests were passing.\n\n>> @@ -196,7 +203,8 @@ test true = \"$rebase\" && {\n>>               then\n>>                       die \"$(gettext \"updating an unborn branch with changes added to the index\")\"\n>>               fi\n>> -     else\n>> +     elif test \"$autostash\" = false\n>> +     then\n>>               require_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n>>       fi\n>\n> A safety net, after you run \"git stash\", to validate that the\n> added \"git stash\" indeed made the working tree clean, is necessary\n> below, but there does not seem to be any.\n\nUm, isn't that part of the \"git stash\" testsuite?\n\n>>       oldremoteref= &&\n>> @@ -213,6 +221,12 @@ test true = \"$rebase\" && {\n>>               fi\n>>       done\n>>  }\n>> +\n>> +stash_required () {\n>> +     ! (git diff-files --quiet --ignore-submodules &&\n>> +     git diff-index --quiet --cached HEAD --ignore-submodules)\n>> +}\n>> +\n>>  orig_head=$(git rev-parse -q --verify HEAD)\n>>  git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok \"$@\" || exit 1\n>>  test -z \"$dry_run\" || exit 0\n>> @@ -288,4 +302,12 @@ true)\n>>       eval=\"$eval \\\"\\$merge_name\\\" HEAD $merge_head\"\n>>       ;;\n>>  esac\n>> -eval \"exec $eval\"\n>> +\n>> +if test \"$autostash\" = true && stash_required\n>> +then\n>> +     git stash\n>> +     eval \"$eval\"\n>> +     test $? = 0 && git stash pop\n>> +else\n>> +     eval \"exec $eval\"\n>> +fi\n>\n> Delaying to run \"git stash\" as much as possible is fine, but with\n> the above, can the user tell if something was stashed and she has\n> to \"stash pop\" once she is done helping the command to resolve the\n> conflicts, or she shouldn't run \"stash pop\" after she is done\n> (because if nothing is stashed at this point, that \"pop\" will pop an\n> unrelated stash)?\n\nI could easily tell, from the \"git pull\" output: if conflict, then\nstash hasn't been applied.  Otherwise, yes.  Should we print a message\nguarded by an advice variable?\n\n> What protects the \"git stash\" from failing and how is its error\n> handled?\n\nOh, this is my stupidity.  I should've just &&-chained the git stash,\neval $eval, and git stash pop.\n"},{"id":"212082","messageId":"7va9pt2r4e.fsf@alter.siamese.dyndns.org","threadId":"33255","inReplyTo":"CALkWK0=oOt0teGqCjpDkerR3-t1cY=qKyK-AtoLLCz9L1-+vyw@mail.gmail.com","subject":"Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-24T07:28:49Z","receivedAt":"2013-03-24T07:28:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n>>> +     elif test \"$autostash\" = false\n>>> +     then\n>>>               require_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n>>>       fi\n>>\n>> A safety net, after you run \"git stash\", to validate that the\n>> added \"git stash\" indeed made the working tree clean, is necessary\n>> below, but there does not seem to be any.\n>\n> Um, isn't that part of the \"git stash\" testsuite?\n\nYou should always \"trust but verify\" what other commands do at key\npoints of the operation; and I think this \"require-clean-work-tree\"\nis a key precondition for this mode of operation to work correctly.\n\nEspecially because you do not even bother to check the result of\n\"git stash\" before continuing ;-).\n\n>>> +if test \"$autostash\" = true && stash_required\n>>> +then\n>>> +     git stash\n>>> +     eval \"$eval\"\n>>> +     test $? = 0 && git stash pop\n>>> +else\n>>> +     eval \"exec $eval\"\n>>> +fi\n>>\n>> Delaying to run \"git stash\" as much as possible is fine, but with\n>> the above, can the user tell if something was stashed and she has\n>> to \"stash pop\" once she is done helping the command to resolve the\n>> conflicts, or she shouldn't run \"stash pop\" after she is done\n>> (because if nothing is stashed at this point, that \"pop\" will pop an\n>> unrelated stash)?\n>\n> I could easily tell, from the \"git pull\" output: if conflict, then\n> stash hasn't been applied.\n\nThe question was about \"git stash save\".  Depending on the cleanness\nof the tree when \"git pull\" was started, \"save\" may or may not have\nbeen done.  After resolving a conflicted \"git pull\" and committing\nthe result, the user does not have much clue if she needs to pop\nanything, does she?  Instead of the usual \"resolve the conflicts and\ncommit the result\", she may need to see \"resolve the conflicts,\ncommit the result, *AND* UNSTASH\".\n"},{"id":"212099","messageId":"CALkWK0k=jei8Z+n-4O92obQOR88FR6iFCSifVhDDS8jv37rOjA@mail.gmail.com","threadId":"33255","inReplyTo":"7va9pt2r4e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-24T17:56:59Z","receivedAt":"2013-03-24T17:56:59Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>\n>>>> +     elif test \"$autostash\" = false\n>>>> +     then\n>>>>               require_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n>>>>       fi\n>>>\n>>> A safety net, after you run \"git stash\", to validate that the\n>>> added \"git stash\" indeed made the working tree clean, is necessary\n>>> below, but there does not seem to be any.\n>>\n>> Um, isn't that part of the \"git stash\" testsuite?\n>\n> You should always \"trust but verify\" what other commands do at key\n> points of the operation; and I think this \"require-clean-work-tree\"\n> is a key precondition for this mode of operation to work correctly.\n>\n> Especially because you do not even bother to check the result of\n> \"git stash\" before continuing ;-).\n\nIf you think it's enough to replicate the codepath that precedes the\nactual saving in 'git stash' (which is essentially\nrequire-clean-work-tree), I'm in agreement with you.  I thought you\nwere implying a wider safety net, that wouldn't even assume the basic\nsanity of stash.\n\n>>>> +if test \"$autostash\" = true && stash_required\n>>>> +then\n>>>> +     git stash\n>>>> +     eval \"$eval\"\n>>>> +     test $? = 0 && git stash pop\n>>>> +else\n>>>> +     eval \"exec $eval\"\n>>>> +fi\n>>>\n>>> Delaying to run \"git stash\" as much as possible is fine, but with\n>>> the above, can the user tell if something was stashed and she has\n>>> to \"stash pop\" once she is done helping the command to resolve the\n>>> conflicts, or she shouldn't run \"stash pop\" after she is done\n>>> (because if nothing is stashed at this point, that \"pop\" will pop an\n>>> unrelated stash)?\n>>\n>> I could easily tell, from the \"git pull\" output: if conflict, then\n>> stash hasn't been applied.\n>\n> The question was about \"git stash save\".  Depending on the cleanness\n> of the tree when \"git pull\" was started, \"save\" may or may not have\n> been done.  After resolving a conflicted \"git pull\" and committing\n> the result, the user does not have much clue if she needs to pop\n> anything, does she?  Instead of the usual \"resolve the conflicts and\n> commit the result\", she may need to see \"resolve the conflicts,\n> commit the result, *AND* UNSTASH\".\n\nYes, good point.  Will it suffice to print a message?\n"},{"id":"212123","messageId":"CALkWK0k0Ek0Dqxrrv7dAzUgF2X2hCcz2SodcPEgiRdLFgc-gXA@mail.gmail.com","threadId":"33255","inReplyTo":"CALkWK0k=jei8Z+n-4O92obQOR88FR6iFCSifVhDDS8jv37rOjA@mail.gmail.com","subject":"Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-03-24T21:12:24Z","receivedAt":"2013-03-24T21:12:24Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Junio C Hamano wrote:\n>> Ramkumar Ramachandra <artagnon@gmail.com> writes:\n>>\n>>>>> +     elif test \"$autostash\" = false\n>>>>> +     then\n>>>>>               require_clean_work_tree \"pull with rebase\" \"Please commit or stash them.\"\n>>>>>       fi\n>>>>\n>>>> A safety net, after you run \"git stash\", to validate that the\n>>>> added \"git stash\" indeed made the working tree clean, is necessary\n>>>> below, but there does not seem to be any.\n>>>\n>>> Um, isn't that part of the \"git stash\" testsuite?\n>>\n>> You should always \"trust but verify\" what other commands do at key\n>> points of the operation; and I think this \"require-clean-work-tree\"\n>> is a key precondition for this mode of operation to work correctly.\n>>\n>> Especially because you do not even bother to check the result of\n>> \"git stash\" before continuing ;-).\n>\n> If you think it's enough to replicate the codepath that precedes the\n> actual saving in 'git stash' (which is essentially\n> require-clean-work-tree), I'm in agreement with you.  I thought you\n> were implying a wider safety net, that wouldn't even assume the basic\n> sanity of stash.\n\nEr, s/codepath that precedes the actual saving in 'git stash'/codepath\nthat precedes the actual pulling or merging in 'git pull'/\n\nI'm feeling a little muddled up today; weekends are usually bad.\n"}]}