Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

[Junio C Hamano]

Ramkumar Ramachandra artag...@gmail.com writes:

 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit or 
 stash them.
   fi

 A safety net, after you run git stash, to validate that the
 added git stash indeed made the working tree clean, is necessary
 below, but there does not seem to be any.

 Um, isn't that part of the git stash testsuite?

You should always trust but verify what other commands do at key
points of the operation; and I think this require-clean-work-tree
is a key precondition for this mode of operation to work correctly.

Especially because you do not even bother to check the result of
git stash before continuing ;-).

 +if test $autostash = true  stash_required
 +then
 + git stash
 + eval $eval
 + test $? = 0  git stash pop
 +else
 + eval exec $eval
 +fi

 Delaying to run git stash as much as possible is fine, but with
 the above, can the user tell if something was stashed and she has
 to stash pop once she is done helping the command to resolve the
 conflicts, or she shouldn't run stash pop after she is done
 (because if nothing is stashed at this point, that pop will pop an
 unrelated stash)?

 I could easily tell, from the git pull output: if conflict, then
 stash hasn't been applied.

The question was about git stash save.  Depending on the cleanness
of the tree when git pull was started, save may or may not have
been done.  After resolving a conflicted git pull and committing
the result, the user does not have much clue if she needs to pop
anything, does she?  Instead of the usual resolve the conflicts and
commit the result, she may need to see resolve the conflicts,
commit the result, *AND* UNSTASH.


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

Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

[Ramkumar Ramachandra]

Junio C Hamano wrote:
 Ramkumar Ramachandra artag...@gmail.com writes:

 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit or 
 stash them.
   fi

 A safety net, after you run git stash, to validate that the
 added git stash indeed made the working tree clean, is necessary
 below, but there does not seem to be any.

 Um, isn't that part of the git stash testsuite?

 You should always trust but verify what other commands do at key
 points of the operation; and I think this require-clean-work-tree
 is a key precondition for this mode of operation to work correctly.

 Especially because you do not even bother to check the result of
 git stash before continuing ;-).

If you think it's enough to replicate the codepath that precedes the
actual saving in 'git stash' (which is essentially
require-clean-work-tree), I'm in agreement with you.  I thought you
were implying a wider safety net, that wouldn't even assume the basic
sanity of stash.

 +if test $autostash = true  stash_required
 +then
 + git stash
 + eval $eval
 + test $? = 0  git stash pop
 +else
 + eval exec $eval
 +fi

 Delaying to run git stash as much as possible is fine, but with
 the above, can the user tell if something was stashed and she has
 to stash pop once she is done helping the command to resolve the
 conflicts, or she shouldn't run stash pop after she is done
 (because if nothing is stashed at this point, that pop will pop an
 unrelated stash)?

 I could easily tell, from the git pull output: if conflict, then
 stash hasn't been applied.

 The question was about git stash save.  Depending on the cleanness
 of the tree when git pull was started, save may or may not have
 been done.  After resolving a conflicted git pull and committing
 the result, the user does not have much clue if she needs to pop
 anything, does she?  Instead of the usual resolve the conflicts and
 commit the result, she may need to see resolve the conflicts,
 commit the result, *AND* UNSTASH.

Yes, good point.  Will it suffice to print a message?
--
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

Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

[Ramkumar Ramachandra]

Ramkumar Ramachandra wrote:
 Junio C Hamano wrote:
 Ramkumar Ramachandra artag...@gmail.com writes:

 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit 
 or stash them.
   fi

 A safety net, after you run git stash, to validate that the
 added git stash indeed made the working tree clean, is necessary
 below, but there does not seem to be any.

 Um, isn't that part of the git stash testsuite?

 You should always trust but verify what other commands do at key
 points of the operation; and I think this require-clean-work-tree
 is a key precondition for this mode of operation to work correctly.

 Especially because you do not even bother to check the result of
 git stash before continuing ;-).

 If you think it's enough to replicate the codepath that precedes the
 actual saving in 'git stash' (which is essentially
 require-clean-work-tree), I'm in agreement with you.  I thought you
 were implying a wider safety net, that wouldn't even assume the basic
 sanity of stash.

Er, s/codepath that precedes the actual saving in 'git stash'/codepath
that precedes the actual pulling or merging in 'git pull'/

I'm feeling a little muddled up today; weekends are usually bad.
--
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

Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

[Ramkumar Ramachandra]

Junio C Hamano wrote:
 Ramkumar Ramachandra artag...@gmail.com writes:

 @@ -1786,6 +1786,11 @@ pull.rebase::
   of merging the default branch from the default remote when git
   pull is run. See branch.name.rebase for setting this on a
   per-branch basis.
 +
 +pull.autostash::
 + When true, automatically stash all changes before attempting to
 + merge/rebase, and pop the stash after a successful
 + merge/rebase.
  +
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]

 Is autosquash a possibly dangerous operation?

Oops.  I must admit I was in a bit of a hurry with the documentation.
I was eager to send out the series seeing that the tests were passing.

 @@ -196,7 +203,8 @@ test true = $rebase  {
   then
   die $(gettext updating an unborn branch with changes 
 added to the index)
   fi
 - else
 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit or 
 stash them.
   fi

 A safety net, after you run git stash, to validate that the
 added git stash indeed made the working tree clean, is necessary
 below, but there does not seem to be any.

Um, isn't that part of the git stash testsuite?

   oldremoteref= 
 @@ -213,6 +221,12 @@ test true = $rebase  {
   fi
   done
  }
 +
 +stash_required () {
 + ! (git diff-files --quiet --ignore-submodules 
 + git diff-index --quiet --cached HEAD --ignore-submodules)
 +}
 +
  orig_head=$(git rev-parse -q --verify HEAD)
  git fetch $verbosity $progress $dry_run $recurse_submodules 
 --update-head-ok $@ || exit 1
  test -z $dry_run || exit 0
 @@ -288,4 +302,12 @@ true)
   eval=$eval \\$merge_name\ HEAD $merge_head
   ;;
  esac
 -eval exec $eval
 +
 +if test $autostash = true  stash_required
 +then
 + git stash
 + eval $eval
 + test $? = 0  git stash pop
 +else
 + eval exec $eval
 +fi

 Delaying to run git stash as much as possible is fine, but with
 the above, can the user tell if something was stashed and she has
 to stash pop once she is done helping the command to resolve the
 conflicts, or she shouldn't run stash pop after she is done
 (because if nothing is stashed at this point, that pop will pop an
 unrelated stash)?

I could easily tell, from the git pull output: if conflict, then
stash hasn't been applied.  Otherwise, yes.  Should we print a message
guarded by an advice variable?

 What protects the git stash from failing and how is its error
 handled?

Oh, this is my stupidity.  I should've just -chained the git stash,
eval $eval, and git stash pop.
--
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

[PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

[Ramkumar Ramachandra]

This new configuration variable executes 'git stash' before attempting
to merge/rebase, and 'git stash pop' after a successful merge/rebase.
It makes it convenient for people to pull with dirty worktrees.

Signed-off-by: Ramkumar Ramachandra artag...@gmail.com
---
 Documentation/config.txt   |  5 
 Documentation/git-pull.txt |  7 ++
 git-pull.sh| 26 ++--
 t/t5521-pull-options.sh| 59 ++
 4 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1f435f..0becafe 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1786,6 +1786,11 @@ pull.rebase::
of merging the default branch from the default remote when git
pull is run. See branch.name.rebase for setting this on a
per-branch basis.
+
+pull.autostash::
+   When true, automatically stash all changes before attempting to
+   merge/rebase, and pop the stash after a successful
+   merge/rebase.
 +
 *NOTE*: this is a possibly dangerous operation; do *not* use
 it unless you understand the implications (see linkgit:git-rebase[1]
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index c975743..bb57c86 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -94,6 +94,13 @@ must be given before the options meant for 'git fetch'.
has to be called afterwards to bring the work tree up to date with the
merge result.
 
+--[no-]autostash::
+   When turned on, automatically stash all changes before
+   attempting to merge/rebase, and pop the stash after a
+   successful merge/rebase.  Useful for people who want to pull
+   with a dirty worktree.  This option can also be set via the
+   `pull.autostash` configuration variable.
+
 Options related to merging
 ~~
 
diff --git a/git-pull.sh b/git-pull.sh
index 37e1cd4..ad8e494 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -48,6 +48,7 @@ if test -z $rebase
 then
rebase=$(git config --bool pull.rebase)
 fi
+autostash=$(git config --bool pull.autostash)
 dry_run=
 while :
 do
@@ -116,6 +117,12 @@ do
--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
rebase=false
;;
+   --autostash)
+   autostash=true
+   ;;
+   --no-autostash)
+   autostash=false
+   ;;
--recurse-submodules)
recurse_submodules=--recurse-submodules
;;
@@ -196,7 +203,8 @@ test true = $rebase  {
then
die $(gettext updating an unborn branch with changes 
added to the index)
fi
-   else
+   elif test $autostash = false
+   then
require_clean_work_tree pull with rebase Please commit or 
stash them.
fi
oldremoteref= 
@@ -213,6 +221,12 @@ test true = $rebase  {
fi
done
 }
+
+stash_required () {
+   ! (git diff-files --quiet --ignore-submodules 
+   git diff-index --quiet --cached HEAD --ignore-submodules)
+}
+
 orig_head=$(git rev-parse -q --verify HEAD)
 git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok 
$@ || exit 1
 test -z $dry_run || exit 0
@@ -288,4 +302,12 @@ true)
eval=$eval \\$merge_name\ HEAD $merge_head
;;
 esac
-eval exec $eval
+
+if test $autostash = true  stash_required
+then
+   git stash
+   eval $eval
+   test $? = 0  git stash pop
+else
+   eval exec $eval
+fi
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 4a804f0..cecacbc 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -90,4 +90,63 @@ test_expect_success 'git pull --all' '
)
 '
 
+test_expect_success 'pull --autostash with clean worktree' '
+   mkdir clonedautostash 
+   (cd clonedautostash 
+   git init 
+   git pull --autostash ../parent 
+   test_must_fail test_path_is_file .git/refs/stash
+   test_commit one
+   ) 
+   rm -rf clonedautostash
+'
+
+test_expect_success 'pull.autostash with clean worktree' '
+   mkdir clonedautostash 
+   (cd clonedautostash 
+   git init 
+   test_config pull.autostash true 
+   git pull ../parent 
+   test_must_fail test_path_is_file .git/refs/stash
+   test_commit one
+   ) 
+   rm -rf clonedautostash
+'
+
+test_expect_success 'pull.autostash without conflict' '
+   mkdir clonedautostash 
+   (cd clonedautostash 
+   git init 
+   test_commit root_commit 
+   cat quux -\EOF 
+   this is a non-conflicting file
+   EOF
+   git add quux 
+   test_config pull.autostash true 
+   git pull ../parent 
+   test_must_fail test_path_is_file .git/refs/stash 
+   test_path_is_file quux 
+   test_commit one
+   ) 
+   rm -rf clonedautostash
+'
+
+test_expect_success 

Re: [PATCH 3/3] git-pull.sh: introduce --[no-]autostash and pull.autostash

[Junio C Hamano]

Ramkumar Ramachandra artag...@gmail.com writes:

 @@ -1786,6 +1786,11 @@ pull.rebase::
   of merging the default branch from the default remote when git
   pull is run. See branch.name.rebase for setting this on a
   per-branch basis.
 +
 +pull.autostash::
 + When true, automatically stash all changes before attempting to
 + merge/rebase, and pop the stash after a successful
 + merge/rebase.
  +
  *NOTE*: this is a possibly dangerous operation; do *not* use
  it unless you understand the implications (see linkgit:git-rebase[1]

Is autosquash a possibly dangerous operation?

 @@ -196,7 +203,8 @@ test true = $rebase  {
   then
   die $(gettext updating an unborn branch with changes 
 added to the index)
   fi
 - else
 + elif test $autostash = false
 + then
   require_clean_work_tree pull with rebase Please commit or 
 stash them.
   fi

A safety net, after you run git stash, to validate that the
added git stash indeed made the working tree clean, is necessary
below, but there does not seem to be any.

   oldremoteref= 
 @@ -213,6 +221,12 @@ test true = $rebase  {
   fi
   done
  }
 +
 +stash_required () {
 + ! (git diff-files --quiet --ignore-submodules 
 + git diff-index --quiet --cached HEAD --ignore-submodules)
 +}
 +
  orig_head=$(git rev-parse -q --verify HEAD)
  git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok 
 $@ || exit 1
  test -z $dry_run || exit 0
 @@ -288,4 +302,12 @@ true)
   eval=$eval \\$merge_name\ HEAD $merge_head
   ;;
  esac
 -eval exec $eval
 +
 +if test $autostash = true  stash_required
 +then
 + git stash
 + eval $eval
 + test $? = 0  git stash pop
 +else
 + eval exec $eval
 +fi

Delaying to run git stash as much as possible is fine, but with
the above, can the user tell if something was stashed and she has
to stash pop once she is done helping the command to resolve the
conflicts, or she shouldn't run stash pop after she is done
(because if nothing is stashed at this point, that pop will pop an
unrelated stash)?

What protects the git stash from failing and how is its error
handled?
--
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