threads / bug / 8842

[BUG] gitk fails with argument that is both existing directory and branch name

Subject: [BUG] gitk fails with argument that is both existing directory and branch name

## tl;dr

9 messages between Jul 3, 2007 and Jul 4, 2007.

replies: 8people: 6as markdown or json

Uwe Kleine-König· Jul 3, 2007, 20:23 UTC · lore
Hello,
I have a branch named fixes in my linux repo and until I did:
	git format-patch -o fixes linus/master 
the following command worked fine:
	
	gitk ^linus/master fixes

but now that there is a directory named fixes, nothing happens when running gitk. With rev-list I get 

	zeisberg@cassiopeia:~/gsrc/linux-2.6$ git rev-list ^linus/master fixes
	fatal: ambiguous argument 'fixes': both revision and filename
	Use '--' to separate filenames from revisions
but 
	
	gitk ^linus/master fixes --

doesn't work either. I'd suspect that the call to rev-list doesn't see the "--" and the error is lost somewhere inbetween ...?

I currently have no time to look into it, so for now you only get a report.

Best regards Uwe

-- 
Uwe Kleine-König

http://www.google.com/search?q=i+squared
Johannes Schindelin· Jul 3, 2007, 21:55 UTC · re: Uwe Kleine-König · lore

Re: [BUG] gitk fails with argument that is both existing directory and branch name

Hi,
On Tue, 3 Jul 2007, Uwe Kleine-K?nig wrote:
Show 7 quoted lines
> 	zeisberg@cassiopeia:~/gsrc/linux-2.6$ git rev-list ^linus/master fixes
> 	fatal: ambiguous argument 'fixes': both revision and filename
> 	Use '--' to separate filenames from revisions
> 
> but 
> 	
> 	gitk ^linus/master fixes --

Umm. Why don't you just use gitk linus/master..fixes? This is how we teach people to use rev-list anyway.

Ciao, Dscho

Linus Torvalds· Jul 3, 2007, 22:17 UTC · re: Johannes Schindelin · lore

Re: [BUG] gitk fails with argument that is both existing directory and branch name

On Tue, 3 Jul 2007, Johannes Schindelin wrote:
> 
> Umm. Why don't you just use gitk linus/master..fixes? This is how we teach 
> people to use rev-list anyway.
No, Uwe is right. There's a bug somewhere.
This _should_ work:
	# create a fake 'gitk' branch if you don't have one already
	git branch gitk master
	# This should look at the 'gitk' branch
	gitk gitk --
but it doesn't. It just hangs for me.

It seems to be some fairly recent gitk bug, too, because I'm pretty sure it _used_ to work.

I checked, and "git rev-list" works fine:
	git rev-list --default HEAD gitk
fails correctly with
	fatal: ambiguous argument 'gitk': both revision and filename
	Use '--' to separate filenames from revisions
and the way to get it to work as a branch-name is to do
	git rev-list --default HEAD gitk --
which works fine.
			Linus
Johannes Schindelin· Jul 3, 2007, 22:44 UTC · re: Linus Torvalds · lore

Re: [BUG] gitk fails with argument that is both existing directory and branch name

Hi,
On Tue, 3 Jul 2007, Linus Torvalds wrote:
Show 6 quoted lines
> On Tue, 3 Jul 2007, Johannes Schindelin wrote:
> 
> > Umm. Why don't you just use gitk linus/master..fixes? This is how we 
> > teach people to use rev-list anyway.
> 
> No, Uwe is right. There's a bug somewhere.

I never questioned that there is a bug. I was only wondering why he used that ambiguous way to specify a range, when a range is clearly what he wants.

Ah well. I'll look into it.

Ciao, Dscho

Johannes Schindelin· Jul 3, 2007, 22:55 UTC · re: Linus Torvalds · lore

[PATCH] gitk: fix for "gitk <ambiguous-name> --"

If you have an ambiguous ref, i.e. a file in the working directory bears the same name, you have to use "--" at the end to disambiguate ref from file. This works with "git rev-list". Make it work with gitk, too.

Noticed by Uwe Kleine-König.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
	Uwe, did your name come out right? I recently had to manually fix 
	pine, and realised only then what a tedious work it is to teach 
	pine about utf-8. Hopefully it worked out.
 gitk |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/gitk b/gitk
index 2d6a6ef..a248f5a 100755
--- a/gitk
+++ b/gitk
@@ -7440,7 +7440,7 @@ set cmdline_files {}
 set i [lsearch -exact $revtreeargs "--"]
 if {$i >= 0} {
     set cmdline_files [lrange $revtreeargs [expr {$i + 1}] end]
-    set revtreeargs [lrange $revtreeargs 0 [expr {$i - 1}]]
+    set revtreeargs [lrange $revtreeargs 0 $i]
 } elseif {$revtreeargs ne {}} {
     if {[catch {
 	set f [eval exec git rev-parse --no-revs --no-flags $revtreeargs]

-- 
1.5.3.rc0.2640.g59df9-dirty
Gerrit Pape· Jul 4, 2007, 07:37 UTC · re: Johannes Schindelin · lore

Re: [PATCH] gitk: fix for "gitk <ambiguous-name> --"

On Tue, Jul 03, 2007 at 11:55:24PM +0100, Johannes Schindelin wrote:
Show 5 quoted lines
> If you have an ambiguous ref, i.e. a file in the working directory bears
> the same name, you have to use "--" at the end to disambiguate ref from
> file. This works with "git rev-list". Make it work with gitk, too.
> 
> Noticed by Uwe Kleine-K??nig.
Not sure anymore why, but I patched differently
 http://article.gmane.org/gmane.comp.version-control.git/48102
Regards, Gerrit.
Uwe Kleine-König· Jul 4, 2007, 08:38 UTC · re: Johannes Schindelin · lore

Re: [PATCH] gitk: fix for "gitk <ambiguous-name> --"

Hello Johannes,
> If you have an ambiguous ref, i.e. a file in the working directory bears
> the same name, you have to use "--" at the end to disambiguate ref from
> file. This works with "git rev-list". Make it work with gitk, too.
This fixes only one half of the problem.  Now 
	gitk fixes ^linus/master --
works fine, but 
	gitk fixes ^linus/master
still hangs.  
I didn't try Gerrits patch yet.
BTW, I don't use a range, because originally I used something like:
	git branch | cut -b3- | grep -v ^attic/ | xargs gitk ^linus/master
Now I have to think how I can add the ending "--" ...

Best regards Uwe

-- 
Uwe Kleine-König

5 out of 4 people have trouble with fractions.
Paul Mackerras· Jul 4, 2007, 10:44 UTC · re: Johannes Schindelin · lore

Re: [PATCH] gitk: fix for "gitk <ambiguous-name> --"

Johannes Schindelin writes:
> If you have an ambiguous ref, i.e. a file in the working directory bears
> the same name, you have to use "--" at the end to disambiguate ref from
> file. This works with "git rev-list". Make it work with gitk, too.

This means that if you do "gitk rev -- file" we will end up with two "--" in the git rev-list command that gitk does. I think we actually want the patch below.

Junio: there seems to be an inconsistency between git rev-list and git
rev-parse here.  If a name is both a filename and a ref, git rev-list
will give a fatal error but git rev-parse will take it as a ref.
Paul.
diff --git a/gitk b/gitk
index d509145..502a01a 100755
--- a/gitk
+++ b/gitk
@@ -87,10 +87,7 @@ proc start_rev_list {view} {
 
     set startmsecs [clock clicks -milliseconds]
     set commitidx($view) 0
-    set args $viewargs($view)
-    if {$viewfiles($view) ne {}} {
-	set args [concat $args "--" $viewfiles($view)]
-    }
+    set args [concat $viewargs($view) "--" $viewfiles($view)]
     set order "--topo-order"
     if {$datemode} {
 	set order "--date-order"
Junio C Hamano· Jul 4, 2007, 16:58 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: fix for "gitk <ambiguous-name> --"

Paul Mackerras <paulus@samba.org> writes:
> Junio: there seems to be an inconsistency between git rev-list and git
> rev-parse here.  If a name is both a filename and a ref, git rev-list
> will give a fatal error but git rev-parse will take it as a ref.
	$ rev-parse --no-revs --no-flags gitk -- gitk
	$ rev-parse --no-revs --no-flags -- gitk
should give you the path "gitk", while
	$ rev-parse --revs-only --no-flags gitk -- gitk
	$ rev-parse --revs-only --no-flags gitk --
should give you the revision (i.e. branch name) "gitk", I think.
But it is not well defined what it should do upon
	$ rev-parse --no-flags gitk
	$ rev-parse gitk

Currently they treat "gitk" as a ref. On the other hand, as you said:

	$ rev-list gitk
would error out without leading or traiing -- to disambiguate.
So you are right that they behave differently.

It might be an improvement to make rev-parse require leading or trailing -- in such a case, but I haven't thought through the ramifications of such a change. Quite a lot of existing users expect "rev-parse foo" to favor a ref "foo" over a file "foo" and give its object name.

← back to recent threads