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

3 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:56:30

Ramkumar Ramachandra [off-list ref] writes:
quoted
quoted
+     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 ;-).
quoted
quoted
+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".

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

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:30

Junio C Hamano wrote:
Ramkumar Ramachandra [off-list ref] writes:
quoted
quoted
quoted
+     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.
quoted
quoted
quoted
+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?

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

From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:30

Ramkumar Ramachandra wrote:
Junio C Hamano wrote:
quoted
Ramkumar Ramachandra [off-list ref] writes:
quoted
quoted
quoted
+     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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help