git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Stash missing, but not.  Can apply, but not drop or list the stash.
@ 2008-09-23 21:23 mattjackets
  2008-09-23 23:08 ` Brandon Casey
  0 siblings, 1 reply; 5+ messages in thread
From: mattjackets @ 2008-09-23 21:23 UTC (permalink / raw)
  To: git

I have a strange stash problem.  There is a single stash in the repo.

git stash apply 0  --  works, but results in a conflict.  Lets just go
ahead and drop the stash...

git stash list  --  shows nothing.  huh?  Lets go ahead with the drop
anyway and hope it works...

$ git stash drop stash@{0}
fatal: Log .git/logs/refs/stash is empty.
stash@{0}: not a valid stashed state

sure enough, .git/logs/refs/stash is empty

git stash clear  --  does nothing

I'm at a loss.  I can apply the stash cleanly to older revisions, and
gitk still shows the stash branch.  How can I fix this?  is it safe to
simply delete the stash branch as if it was any other branch?

Thanks,
Matt
-- 
  matt
  mattlist@fastmail.fm

-- 
http://www.fastmail.fm - Access all of your messages and folders
                          wherever you are

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Stash missing, but not.  Can apply, but not drop or list the stash.
  2008-09-23 21:23 Stash missing, but not. Can apply, but not drop or list the stash mattjackets
@ 2008-09-23 23:08 ` Brandon Casey
  2008-09-23 23:57   ` [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied Brandon Casey
  0 siblings, 1 reply; 5+ messages in thread
From: Brandon Casey @ 2008-09-23 23:08 UTC (permalink / raw)
  To: mattjackets; +Cc: git

mattjackets wrote:
> I have a strange stash problem.  There is a single stash in the repo.
> 
> git stash apply 0  --  works, but results in a conflict.

The correct form is 'git stash apply stash@{0}'

'git stash apply 0' is doing the right thing for the wrong reason.
For example, 'git stash apply 1' would not do the right thing.

This is unrelated to your issue, but it seems you have uncovered
a flaw which should be fixed.

In git stash, when your command line above is used, eventually
the following command is executed:

    git rev-parse --revs-only --no-flags --default refs/stash 0

'git rev-parse' fails to resolve revision "0" and prints out nothing
and then falls back to printing the revision of "refs/stash". This
is not what is desired by stash.

Either rev-parse needs to error out in this case, or 'git stash' needs
to be changed so that '--default refs/stash' is not used here. Possibly,
something like what is done inside drop_stash().

>  Lets just go
> ahead and drop the stash...
> 
> git stash list  --  shows nothing.  huh?  Lets go ahead with the drop
> anyway and hope it works...
> 
> $ git stash drop stash@{0}
> fatal: Log .git/logs/refs/stash is empty.
> stash@{0}: not a valid stashed state
> 
> sure enough, .git/logs/refs/stash is empty

Right, it must exist since you actually got the error message
'fatal: Log .git/logs/refs/stash is empty', but it contains
nothing. Not sure how that happened.

> git stash clear  --  does nothing

It correctly removes .git/refs/stash for me.

> I'm at a loss.  I can apply the stash cleanly to older revisions, and
> gitk still shows the stash branch.  How can I fix this?  is it safe to
> simply delete the stash branch as if it was any other branch?

In this case yes it would be safe to just do 'rm .git/refs/stash', but
like I said, 'git stash clear' worked for me.

-brandon

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied
  2008-09-23 23:08 ` Brandon Casey
@ 2008-09-23 23:57   ` Brandon Casey
  2008-10-01 19:53     ` regression: stash show -p (was [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied) Brandon Casey
  0 siblings, 1 reply; 5+ messages in thread
From: Brandon Casey @ 2008-09-23 23:57 UTC (permalink / raw)
  To: mattjackets; +Cc: Git Mailing List

apply_stash() and show_stash() each call rev-parse with
'--default refs/stash' as an argument. This option causes rev-parse to
operate on refs/stash if it is not able to successfully operate on any
element of the command line. This includes failure to supply a "valid"
revision. This has the effect of causing 'stash apply' and 'stash show'
to operate as if stash@{0} had been supplied when an invalid revision is
supplied.

e.g. 'git stash apply stahs@{1}' would fall back to
     'git stash apply stash@{0}'

This patch modifies these two functions so that they avoid using the
--default option of rev-parse.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


This should fix the case I mention above, but it does not fix the
case where a non-existent reflog entry is specified. In this case
the last entry will be selected.

	$ git stash list
	stash@{0}: WIP on master: c050772... small java change
	stash@{1}: WIP on master: c050772... small java change
	stash@{2}: WIP on master: c050772... small java change
	stash@{3}: WIP on master: c050772... small java change
	$ git stash apply stash@{10}
	warning: Log for 'stash' only has 4 entries.
	# On branch master
	# Changed but not updated:
	... etc.

stash@{3} was applied.

Luckily, the dangerous case has no effect.

	$ git stash drop stash@{10}
	Dropped stash@{10} (b7a2467e58109c92d799d059f508f35853d0bff7)
	$ git stash list
	stash@{0}: WIP on master: c050772... small java change
	stash@{1}: WIP on master: c050772... small java change
	stash@{2}: WIP on master: c050772... small java change
	stash@{3}: WIP on master: c050772... small java change

-brandon


 git-stash.sh |   17 +++++++++++++++--
 1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index d799c76..6bd2572 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -144,7 +144,14 @@ show_stash () {
 	then
 		flags=--stat
 	fi
-	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
+
+	if test $# = 0
+	then
+		set x "$ref_stash@{0}"
+		shift
+	fi
+
+	s=$(git rev-parse --revs-only --no-flags "$@")
 
 	w_commit=$(git rev-parse --verify "$s") &&
 	b_commit=$(git rev-parse --verify "$s^") &&
@@ -163,13 +170,19 @@ apply_stash () {
 		shift
 	esac
 
+	if test $# = 0
+	then
+		set x "$ref_stash@{0}"
+		shift
+	fi
+
 	# current index state
 	c_tree=$(git write-tree) ||
 		die 'Cannot apply a stash in the middle of a merge'
 
 	# stash records the work tree, and is a merge between the
 	# base commit (first parent) and the index tree (second parent).
-	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
+	s=$(git rev-parse --revs-only --no-flags "$@") &&
 	w_tree=$(git rev-parse --verify "$s:") &&
 	b_tree=$(git rev-parse --verify "$s^1:") &&
 	i_tree=$(git rev-parse --verify "$s^2:") ||
-- 
1.6.0.1.244.gdc19

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* regression: stash show -p (was [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied)
  2008-09-23 23:57   ` [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied Brandon Casey
@ 2008-10-01 19:53     ` Brandon Casey
  2008-10-02 23:52       ` [PATCH] git-stash.sh: fix flawed fix of invalid ref handling (commit da65e7c1) Brandon Casey
  0 siblings, 1 reply; 5+ messages in thread
From: Brandon Casey @ 2008-10-01 19:53 UTC (permalink / raw)
  To: mattjackets; +Cc: Git Mailing List


Thanks to the patch referenced in the subject, 'git stash show -p'
no longer works if a revision is _not_ supplied.

This is because the argument list passed to show_stash() is not empty,
so the default $ref_stash@{0} is not appended.

This brings me back to an unanswered question, "Should rev-parse fall
back to its --default argument when an invalid ref is supplied?". The
documentation does not imply that it should.

>From Documentation/git-rev-parse.txt:

   --default <arg>::
        If there is no parameter given by the user, use `<arg>`
        instead.

Currently a rev-parse invocation like:

    $ git rev-parse --default HEAD a_non_existent_ref

will fall back to operating on HEAD. Is this correct? Could there be
some part of git that depends on this behavior? filter-branch is now
the only script which uses the --default option of rev-parse, not sure
about the c code.

-brandon



Brandon Casey wrote:
> apply_stash() and show_stash() each call rev-parse with
> '--default refs/stash' as an argument. This option causes rev-parse to
> operate on refs/stash if it is not able to successfully operate on any
> element of the command line. This includes failure to supply a "valid"
> revision. This has the effect of causing 'stash apply' and 'stash show'
> to operate as if stash@{0} had been supplied when an invalid revision is
> supplied.
> 
> e.g. 'git stash apply stahs@{1}' would fall back to
>      'git stash apply stash@{0}'
> 
> This patch modifies these two functions so that they avoid using the
> --default option of rev-parse.
> 
> Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
> ---
> 
> 
> This should fix the case I mention above, but it does not fix the
> case where a non-existent reflog entry is specified. In this case
> the last entry will be selected.
> 
> 	$ git stash list
> 	stash@{0}: WIP on master: c050772... small java change
> 	stash@{1}: WIP on master: c050772... small java change
> 	stash@{2}: WIP on master: c050772... small java change
> 	stash@{3}: WIP on master: c050772... small java change
> 	$ git stash apply stash@{10}
> 	warning: Log for 'stash' only has 4 entries.
> 	# On branch master
> 	# Changed but not updated:
> 	... etc.
> 
> stash@{3} was applied.
> 
> Luckily, the dangerous case has no effect.
> 
> 	$ git stash drop stash@{10}
> 	Dropped stash@{10} (b7a2467e58109c92d799d059f508f35853d0bff7)
> 	$ git stash list
> 	stash@{0}: WIP on master: c050772... small java change
> 	stash@{1}: WIP on master: c050772... small java change
> 	stash@{2}: WIP on master: c050772... small java change
> 	stash@{3}: WIP on master: c050772... small java change
> 
> -brandon
> 
> 
>  git-stash.sh |   17 +++++++++++++++--
>  1 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/git-stash.sh b/git-stash.sh
> index d799c76..6bd2572 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -144,7 +144,14 @@ show_stash () {
>  	then
>  		flags=--stat
>  	fi
> -	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
> +
> +	if test $# = 0
> +	then
> +		set x "$ref_stash@{0}"
> +		shift
> +	fi
> +
> +	s=$(git rev-parse --revs-only --no-flags "$@")
>  
>  	w_commit=$(git rev-parse --verify "$s") &&
>  	b_commit=$(git rev-parse --verify "$s^") &&
> @@ -163,13 +170,19 @@ apply_stash () {
>  		shift
>  	esac
>  
> +	if test $# = 0
> +	then
> +		set x "$ref_stash@{0}"
> +		shift
> +	fi
> +
>  	# current index state
>  	c_tree=$(git write-tree) ||
>  		die 'Cannot apply a stash in the middle of a merge'
>  
>  	# stash records the work tree, and is a merge between the
>  	# base commit (first parent) and the index tree (second parent).
> -	s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
> +	s=$(git rev-parse --revs-only --no-flags "$@") &&
>  	w_tree=$(git rev-parse --verify "$s:") &&
>  	b_tree=$(git rev-parse --verify "$s^1:") &&
>  	i_tree=$(git rev-parse --verify "$s^2:") ||

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] git-stash.sh: fix flawed fix of invalid ref handling (commit da65e7c1)
  2008-10-01 19:53     ` regression: stash show -p (was [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied) Brandon Casey
@ 2008-10-02 23:52       ` Brandon Casey
  0 siblings, 0 replies; 5+ messages in thread
From: Brandon Casey @ 2008-10-02 23:52 UTC (permalink / raw)
  To: Nanako Shiraishi; +Cc: Git Mailing List

The referenced commit tried to fix a flaw in stash's handling of a user
supplied invalid ref. i.e. 'git stash apply fake_ref@{0}' should fail
instead of applying stash@{0}. But, it did so in a naive way by avoiding the
use of the --default option of rev-parse, and instead manually supplied the
default revision if the user supplied an empty command line. This prevented
a common usage scenario of supplying flags on the stash command line (i.e.
non-empty command line) which would be parsed by lower level git commands,
without supplying a specific revision. This should fall back to the default
revision, but now it causes an error. e.g. 'git stash show -p'

The correct fix is to use the --verify option of rev-parse, which fails
properly if an invalid ref is supplied, and still allows falling back to a
default ref when one is not supplied.

Convert stash-drop to use --verify while we're at it, since specifying
multiple revisions for any of these commands is also an error and --verify
makes it so.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---


The patch is simpler than it looks, since it includes a revert of my
original patch.

Here is 'git diff da65e7c1^:git-stash.sh git-stash.sh':

 @@ -144,17 +144,16 @@ show_stash () {
        then
                flags=--stat
        fi
-       s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@")
 
-       w_commit=$(git rev-parse --verify "$s") &&
-       b_commit=$(git rev-parse --verify "$s^") &&
+       w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
+       b_commit=$(git rev-parse --verify "$w_commit^") &&
        git diff $flags $b_commit $w_commit
 }
 
 @@ -169,7 +168,7 @@ apply_stash () {
 
        # stash records the work tree, and is a merge between the
        # base commit (first parent) and the index tree (second parent).
-       s=$(git rev-parse --revs-only --no-flags --default $ref_stash "$@") &&
+       s=$(git rev-parse --verify --default $ref_stash "$@") &&
        w_tree=$(git rev-parse --verify "$s:") &&
        b_tree=$(git rev-parse --verify "$s^1:") &&
        i_tree=$(git rev-parse --verify "$s^2:") ||
 @@ -229,7 +228,7 @@ drop_stash () {
                shift
        fi
        # Verify supplied argument looks like a stash entry
-       s=$(git rev-parse --revs-only --no-flags "$@") &&
+       s=$(git rev-parse --verify "$@") &&
        git rev-parse --verify "$s:"   > /dev/null 2>&1 &&
        git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
        git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||

I already messed this up once, so a review is appreciated. Any reason
--verify wasn't used in the first place?

I know why it wasn't used in drop_stash(), it's because I just copied it
from apply_stash().

-brandon


 git-stash.sh |   22 ++++------------------
 1 files changed, 4 insertions(+), 18 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 42f626f..b9ace99 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -145,16 +145,8 @@ show_stash () {
 		flags=--stat
 	fi
 
-	if test $# = 0
-	then
-		set x "$ref_stash@{0}"
-		shift
-	fi
-
-	s=$(git rev-parse --revs-only --no-flags "$@")
-
-	w_commit=$(git rev-parse --verify "$s") &&
-	b_commit=$(git rev-parse --verify "$s^") &&
+	w_commit=$(git rev-parse --verify --default $ref_stash "$@") &&
+	b_commit=$(git rev-parse --verify "$w_commit^") &&
 	git diff $flags $b_commit $w_commit
 }
 
@@ -170,19 +162,13 @@ apply_stash () {
 		shift
 	esac
 
-	if test $# = 0
-	then
-		set x "$ref_stash@{0}"
-		shift
-	fi
-
 	# current index state
 	c_tree=$(git write-tree) ||
 		die 'Cannot apply a stash in the middle of a merge'
 
 	# stash records the work tree, and is a merge between the
 	# base commit (first parent) and the index tree (second parent).
-	s=$(git rev-parse --revs-only --no-flags "$@") &&
+	s=$(git rev-parse --verify --default $ref_stash "$@") &&
 	w_tree=$(git rev-parse --verify "$s:") &&
 	b_tree=$(git rev-parse --verify "$s^1:") &&
 	i_tree=$(git rev-parse --verify "$s^2:") ||
@@ -242,7 +228,7 @@ drop_stash () {
 		shift
 	fi
 	# Verify supplied argument looks like a stash entry
-	s=$(git rev-parse --revs-only --no-flags "$@") &&
+	s=$(git rev-parse --verify "$@") &&
 	git rev-parse --verify "$s:"   > /dev/null 2>&1 &&
 	git rev-parse --verify "$s^1:" > /dev/null 2>&1 &&
 	git rev-parse --verify "$s^2:" > /dev/null 2>&1 ||
-- 
1.6.0.2.323.g7c850

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2008-10-02 23:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-23 21:23 Stash missing, but not. Can apply, but not drop or list the stash mattjackets
2008-09-23 23:08 ` Brandon Casey
2008-09-23 23:57   ` [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied Brandon Casey
2008-10-01 19:53     ` regression: stash show -p (was [PATCH maint] git-stash.sh: don't default to refs/stash if invalid ref supplied) Brandon Casey
2008-10-02 23:52       ` [PATCH] git-stash.sh: fix flawed fix of invalid ref handling (commit da65e7c1) Brandon Casey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).