fatal: git grep: cannot generate relative filenames containing '..'

fatal: git grep: cannot generate relative filenames containing '..'

[George Spelvin]

Could someone fix this some day?  "git grep <pattern> ../include"
is something I find myself wanting quite frequently, and it's a fresh
annoyance every time I type it to discover that it still doesn't work.

While you're at it, an option to search the entire git tree rather than
the current subdirectory would also be useful.  I was thinking about
a flag like -r (for "root"), but a second idea dawned on me: interpret
absolute pathnames as relative to the root of the repository.  So I could
"git grep <pattern> /" or "git grep <pattern> /include" from any subdirectory.

As it is currently, absolute pathnames don't work very well either...
[1]$ git grep xyzzy $PWD
fatal: '/home/linux/project/src' is outside repository
[2]$ cd /usr/src/linux
[3]$ git grep xyzzy $PWD
CREDITS:E: rjd@xyzzy.clara.co.uk
drivers/hwmon/hwmon-vid.c: * With assistance from Trent Piepho <xyzzy@speakeasy.
drivers/infiniband/hw/ipath/ipath_debug.h: * if(infinipath_debug & _IPATH_xyzzy)
drivers/media/dvb/frontends/or51132.c: *    Copyright (C) 2007 Trent Piepho <xyz
drivers/media/video/cx88/cx88-alsa.c: *    (c) 2007 Trent Piepho <xyzzy@speakeas
drivers/video/intelfb/intelfbhw.c:      /* do some funky magic - xyzzy */
include/linux/byteorder/swab.h: * Trent Piepho <xyzzy@speakeasy.org> 2007114
include/linux/hwmon-vid.h:    With assistance from Trent Piepho <xyzzy@speakeasy
[4]$ cd include
[5]$ git grep xyzzy $PWD
fatal: git grep: cannot generate relative filenames containing '..'

I don't quite understand that last error message.

Thank you.

Re: fatal: git grep: cannot generate relative filenames containing '..'

[Markus Heidelberg]

George Spelvin, 15.01.2009:
> While you're at it, an option to search the entire git tree rather than
> the current subdirectory would also be useful.  I was thinking about
> a flag like -r (for "root"), but a second idea dawned on me: interpret
> absolute pathnames as relative to the root of the repository.  So I could
> "git grep <pattern> /" or "git grep <pattern> /include" from any subdirectory.

I have never used submodules execpt for trying out, but I think it would
ambigous when calling this command from inside a submodule. It's not
clear, whether the main repo or the submodule should be used.

Markus

Re: fatal: git grep: cannot generate relative filenames containing '..'

[Junio C Hamano]

"George Spelvin" <linux@horizon.com> writes:

> Could someone fix this some day?

As far as I know you are the first to ask for this, so perhaps people never
considered this was something to "fix"....

> "git grep <pattern> ../include"
> is something I find myself wanting quite frequently, and it's a fresh
> annoyance every time I type it to discover that it still doesn't work.
>
> While you're at it, an option to search the entire git tree rather than
> the current subdirectory would also be useful.  I was thinking about
> a flag like -r (for "root"),...

... but

    $ grep -r foo ..

will look for foo anywhere below one level up from your current directory,
and I think it may be a reasonable enhancement to teach:

    $ git grep foo ..

to do the same.  Patches welcome ;-).

By the way, congratulations for getting a name.  Can we fix one entry in
our .mailmap file?

Re: fatal: git grep: cannot generate relative filenames containing '..'

[Johannes Schindelin]

Hi,

On Thu, 15 Jan 2009, George Spelvin wrote:

> Could someone fix this some day?

Yes, someone could.

Ciao,
Dscho

Re: fatal: git grep: cannot generate relative filenames containing '..'

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Thu, 15 Jan 2009, George Spelvin wrote:
>
>> Could someone fix this some day?
>
> Yes, someone could.

Or perhaps someone did more than two years ago with --full-name?

Re: fatal: git grep: cannot generate relative filenames containing '..'

[George Spelvin]

Junio C Hamano <gitster@pobox.com> wrote:
> Or perhaps someone did more than two years ago with --full-name?

Thank you for pointing that out!  It's a bit of a handful to type,
but at least it makes what I want to do possible without having to
write a wrapper script.  And I can reduce the typing with an alias.

I'm still trying to figure out why the basic form doesn't work,
though.  Is there something more subtle than that fact that the
filename simplification in grep_sha1 and grep_file might do the
wrong thing?  If I comment out the check in cmd_grep(), it seems
to work, although it prints out some funky filenames.

That seems like a straightforward thing to fix.  Basically, strip
off the common part of the prefix and name, and for every remaining
component in the prefix, prepend a ../ to the name.

I don't mind doing the coding, but can someone who groks the
code more fully tell me if I'm missing something major?

Re: fatal: git grep: cannot generate relative filenames containing '..'

[George Spelvin]

> I don't mind doing the coding, but can someone who groks the
> code more fully tell me if I'm missing something major?

Here's a first draft, that Works For Me(tm).  Does anyone see anything
broken about it?

(This code released into the public domain, copyright abandoned.  You may
do anything you like with it, including evil things, as long as you
don't bother me asking for additional permissions.)

diff --git a/builtin-grep.c b/builtin-grep.c
index bebf15c..5727a8b 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -90,12 +90,74 @@ static int pathspec_matches(const char **paths, const char *name)
 	return 0;
 }
 
+/*
+ * Write a filename like "/usr/src/linux/include/linux/zlib.h" as
+ * a path relative to some prefix like "/usr/src/linux/kernel/"
+ * (The prefix always includes a trailing slash.)
+ * label_len, if non-zero, describes a leading portion on the name
+ * (typically of the form "HEAD^^:") which should not be stripped off.
+ * 
+ * The result is in one of three places:
+ * - It may be a pointer into the supplied name in the simplest case,
+ * - It may be returned in a static buffer if it is of reaonable size, or
+ * - It may be in a malloced buffer if it is large.
+ * A pointer to be passed to free() is returned in *to_free, which
+ * is set to NULL if the return value is not to be freed, or is equal
+ * to the return value if it is.  (It could be simply a boolean, but doing
+ * it this way eliminates a test in the caller.)
+ */
+static const char *make_relative(unsigned label_len, const char *prefix, unsigned prefix_len, const char *name, void **to_free)
+{
+	static char name_buf[PATH_MAX];
+	char *cp;
+	unsigned n, match_len = 0, slashes = 0;
+	unsigned name_len;
+
+	for (n = 0; n < prefix_len; n++) {
+		if (prefix[n] != name[label_len+n]) {
+			for (; n < prefix_len; n++)
+				slashes += (prefix[n] == '/');
+			break;
+		}
+		if (prefix[n] == '/')
+			match_len = n+1;
+	}
+
+	/* Can we return a substring of the input string? */
+	if (!slashes && (!label_len || !match_len)) {
+		*to_free = NULL;
+		return name + match_len;
+	}
+
+	/* Nope, assemble the full response */
+
+	/* Output buffer will be tag + "../"*slashes + name + '\0' */
+	name_len = strlen(name + label_len + match_len) + 1;
+	n = label_len + 3*slashes + name_len + 1;
+	if (n <= ARRAY_SIZE(name_buf)) {
+		cp = name_buf;
+		*to_free = NULL;
+	} else {
+		*to_free = cp = xmalloc(n);
+	}
+
+	/* Now fill in the buffer */
+	memcpy(cp, name, label_len);
+	n = label_len;
+	while (slashes--) {
+		memcpy(cp+n, "../", 3);
+		n += 3;
+	}
+	memcpy(cp+n, name+label_len+match_len, name_len);
+	return cp;
+}
+
 static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1, const char *name, int tree_name_len)
 {
 	unsigned long size;
 	char *data;
 	enum object_type type;
-	char *to_free = NULL;
+	void *to_free = NULL;
 	int hit;
 
 	data = read_sha1_file(sha1, &type, &size);
@@ -103,24 +165,9 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1, const char
 		error("'%s': unable to read %s", name, sha1_to_hex(sha1));
 		return 0;
 	}
-	if (opt->relative && opt->prefix_length) {
-		static char name_buf[PATH_MAX];
-		char *cp;
-		int name_len = strlen(name) - opt->prefix_length + 1;
-
-		if (!tree_name_len)
-			name += opt->prefix_length;
-		else {
-			if (ARRAY_SIZE(name_buf) <= name_len)
-				cp = to_free = xmalloc(name_len);
-			else
-				cp = name_buf;
-			memcpy(cp, name, tree_name_len);
-			strcpy(cp + tree_name_len,
-			       name + tree_name_len + opt->prefix_length);
-			name = cp;
-		}
-	}
+	if (opt->relative)
+		name = make_relative(tree_name_len, opt->prefix,
+		                     opt->prefix_length, name, &to_free);
 	hit = grep_buffer(opt, name, data, size);
 	free(data);
 	free(to_free);
@@ -132,6 +179,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 	struct stat st;
 	int i;
 	char *data;
+	void *to_free = NULL;
 	size_t sz;
 
 	if (lstat(filename, &st) < 0) {
@@ -156,10 +204,12 @@ static int grep_file(struct grep_opt *opt, const char *filename)
 		return 0;
 	}
 	close(i);
-	if (opt->relative && opt->prefix_length)
-		filename += opt->prefix_length;
+	if (opt->relative)
+		filename = make_relative(0, opt->prefix, opt->prefix_length,
+		                         filename, &to_free);
 	i = grep_buffer(opt, filename, data, sz);
 	free(data);
+	free(to_free);
 	return i;
 }
 
@@ -528,7 +578,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	int i;
 
 	memset(&opt, 0, sizeof(opt));
-	opt.prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
+	opt.prefix = prefix;
+	opt.prefix_length = prefix ? strlen(prefix) : 0;
 	opt.relative = 1;
 	opt.pathname = 1;
 	opt.pattern_tail = &opt.pattern_list;
@@ -787,17 +838,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			verify_filename(prefix, argv[j]);
 	}
 
-	if (i < argc) {
+	if (i < argc)
 		paths = get_pathspec(prefix, argv + i);
-		if (opt.prefix_length && opt.relative) {
-			/* Make sure we do not get outside of paths */
-			for (i = 0; paths[i]; i++)
-				if (strncmp(prefix, paths[i], opt.prefix_length))
-					die("git grep: cannot generate relative filenames containing '..'");
-		}
-	}
 	else if (prefix) {
-		paths = xcalloc(2, sizeof(const char *));
+		paths = xmalloc(2 * sizeof *paths);
 		paths[0] = prefix;
 		paths[1] = NULL;
 	}
diff --git a/grep.h b/grep.h
index 45a222d..a02dccf 100644
--- a/grep.h
+++ b/grep.h
@@ -56,6 +56,7 @@ struct grep_opt {
 	struct grep_pat *pattern_list;
 	struct grep_pat **pattern_tail;
 	struct grep_expr *pattern_expression;
+	const char *prefix;
 	int prefix_length;
 	regex_t regexp;
 	unsigned linenum:1;

Re: fatal: git grep: cannot generate relative filenames containing '..'

[SZEDER Gábor]

On Thu, Jan 15, 2009 at 10:27:08PM -0500, George Spelvin wrote:
> Junio C Hamano <gitster@pobox.com> wrote:
> > Or perhaps someone did more than two years ago with --full-name?
> 
> Thank you for pointing that out!  It's a bit of a handful to type,

Well, if you happen to use bash, you will find our excellent ;) bash
completion script under contrib/completion.

hth,
Gábor

Re: fatal: git grep: cannot generate relative filenames containing '..'

[Jon Loeliger]

On Thu, 2009-01-15 at 17:52 -0800, Junio C Hamano wrote:
> "George Spelvin" <linux@horizon.com> writes:
> 

> By the way, congratulations for getting a name.  Can we fix one entry in
> our .mailmap file?


Hmmm....

http://en.wikipedia.org/wiki/George_Spelvin

    George Spelvin, Georgette Spelvin, and Georgina Spelvin are the
    traditional pseudonyms used in programs in American theater by
    actors who don't want to be credited or whose names would
    otherwise appear twice because they are playing more than one
    role in a production.

jdl

Re: fatal: git grep: cannot generate relative filenames containing '..'

[Hannu Koivisto]

Junio C Hamano <gitster@pobox.com> writes:

> "George Spelvin" <linux@horizon.com> writes:
>
>> Could someone fix this some day?
>
> As far as I know you are the first to ask for this, so perhaps people never
> considered this was something to "fix"....

I for one have faced that problem and considered it something that
would be nice to have fixed.  Then again, I consider the default
behaviour (when you pass no path) a bigger usability problem.

>> While you're at it, an option to search the entire git tree rather than
>> the current subdirectory would also be useful.  I was thinking about
>> a flag like -r (for "root"),...

It turns out that this "entire git tree" is practically my only use
case when using normal (find and) grep and when I first tried git
grep, I actually expected it to do just that with no path specified.

Why?  Because of the way at least git diff and git log work.  I
thought that just like with them, "git grep foo" would search the
entire git tree and I would have to say "git grep foo ." to limit to
the current directory and its subdirectories.  git-grep(1)'s "Look
for specified patterns in the working tree files..." at least
didn't seem to disagree with my expectation so I was a bit puzzled.

So I'd rather see git grep behave in a way consistent with git log
and git diff (I realize that would change current behaviour instead
of extending it).

-- 
Hannu

Re: fatal: git grep: cannot generate relative filenames containing '..'

[George Spelvin]

Hannu Koivisto <azure@iki.fi> wrote:
> It turns out that this "entire git tree" is practically my only use
> case when using normal (find and) grep and when I first tried git
> grep, I actually expected it to do just that with no path specified.
>
> Why?  Because of the way at least git diff and git log work.  I
> thought that just like with them, "git grep foo" would search the
> entire git tree and I would have to say "git grep foo ." to limit to
> the current directory and its subdirectories.  git-grep(1)'s "Look
> for specified patterns in the working tree files..." at least
> didn't seem to disagree with my expectation so I was a bit puzzled.
>
> So I'd rather see git grep behave in a way consistent with git log
> and git diff (I realize that would change current behaviour instead
> of extending it).

D'oh.  You're completely right.  Space-dot is trivial to type if you
want the current directory only, and that is more consistent.

Could that be considered for 1.7?