Re: Incorrect git-blame result if I use full path to file
- From
Linus Torvalds <torvalds@linux-foundation.org>
- Date
- Dec 3, 2007, 17:26 UTC
- Message-ID
- <alpine.LFD.0.9999.0712030922250.8458@woody.linux-foundation.org>
- In-Reply-To
- <7vhcj0seok.fsf@gitster.siamese.dyndns.org>
On Sun, 2 Dec 2007, Junio C Hamano wrote:
Show 9 quoted lines
> "Anatol Pomozov" <anatol.pomozov@gmail.com> writes: > > > > I just start learning git and I found a bug (but sorry if the > > functionality I am trying to blame as a bug not actually bug and it > > was made by intention) > > I think it is rather a sloppy error checking than a bug. It should be > throwing a stone back at you when you feed it a full path, or converting > it back to work tree relative path before using.
How about this patch?
It makes "get_pathspec()" make all the paths it returns relative, if it can. HOWEVER! I think it should actually die() if it sees an absolute path that it cannot convert (because it really cannot do anything sane about it), but I commented that out for now because that requires some test case change: right now we actually have a few test cases for insane filename arguments, and they expect the old behaviour.
Comments? This changes behaviour subtly (and if we enable the "die(..)" logic, not-so-subtly), but I think that in any case where it changes behaviour, the new behaviour would be an improvement, and the old one would be nonsensical (ie you get *some* results with an absolute pathname, just not the ones you'd expect!)
Note the die() comment in the bad case in "make_relative()".
Linus --- setup.c | 34 +++++++++++++++++++++++++++++++++- 1 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/setup.c b/setup.c index 2c7b5cb..fadf4ee 100644 --- a/setup.c +++ b/setup.c @@ -111,11 +111,26 @@ void verify_non_filename(const char *prefix, const char *arg) die("'%s': %s", arg, strerror(errno)); } +static const char *make_relative(const char *file, const char *pwd, int pwdlen) +{ + if (strncmp(file, pwd, pwdlen)) + goto bad; + if (file[pwdlen] != '/') + goto bad; + return file + pwdlen + 1; + +bad: + /* Should we die() here or just do a "return file"? */ + /* die("pathname '%s' is not in the repository", file); */ + return file; +} + const char **get_pathspec(const char *prefix, const char **pathspec) { + const char *pwd; const char *entry = *pathspec; const char **p; - int prefixlen; + int prefixlen, pwdlen; if (!prefix && !entry) return NULL; @@ -127,9 +142,26 @@ const char **get_pathspec(const char *prefix, const char **pathspec) return spec; } + pwd = NULL; + pwdlen = 0; + p = pathspec; + do { + if (*entry == '/') { + if (!pwd) { + char buffer[PATH_MAX + 1]; + if (!getcwd(buffer, sizeof(buffer))) + break; + pwd = buffer; + pwdlen = strlen(buffer); + } + *p = make_relative(entry, pwd, pwdlen); + } + } while ((entry = *++p) != NULL); + /* Otherwise we have to re-write the entries.. */ p = pathspec; prefixlen = prefix ? strlen(prefix) : 0; + entry = *p; do { *p = prefix_path(prefix, prefixlen, entry); } while ((entry = *++p) != NULL);