threads / discuss / 41360

git diff HEAD^(255) fails

Subject: git diff HEAD^(255) fails

## tl;dr

6 messages between Feb 6, 2016 and Feb 23, 2016.

replies: 5people: 5as markdown or json

Ole Tange· Feb 6, 2016, 21:56 UTC · lore

git diff first looks for a file, then looks if it is a reference to a revision. If the file fails due to being too long, the diff fails:

$ git init $ git diff 'HEAD^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^' HEAD fatal: failed to stat 'HEAD^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^': File name too long

If file name too long it should just try to see if it is a reference to a revision.

/Ole
Nguyễn Thái Ngọc Duy· Feb 6, 2016, 23:23 UTC · re: Ole Tange · lore

[PATCH] setup.c: make check_filename() return 0 on ENAMETOOLONG

Noticed-by: Ole Tange <ole@tange.dk>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 On Sun, Feb 7, 2016 at 4:56 AM, Ole Tange <ole@tange.dk> wrote:
 > If file name too long it should just try to see if it is a reference
 > to a revision.
 Looks easy enough to fix.
 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/setup.c b/setup.c
index 2c4b22c..ab8f85d 100644
--- a/setup.c
+++ b/setup.c
@@ -147,7 +147,7 @@ int check_filename(const char *prefix, const char *arg)
 		name = arg;
 	if (!lstat(name, &st))
 		return 1; /* file exists */
-	if (errno == ENOENT || errno == ENOTDIR)
+	if (errno == ENOENT || errno == ENOTDIR || errno == ENAMETOOLONG)
 		return 0; /* file does not exist */
 	die_errno("failed to stat '%s'", arg);
 }

-- 
2.7.0.377.g4cd97dd
Johannes Schindelin· Feb 7, 2016, 12:23 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] setup.c: make check_filename() return 0 on ENAMETOOLONG

Hi Duy,
On Sun, 7 Feb 2016, Nguyễn Thái Ngọc Duy wrote:
Show 8 quoted lines
> Noticed-by: Ole Tange <ole@tange.dk>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  On Sun, Feb 7, 2016 at 4:56 AM, Ole Tange <ole@tange.dk> wrote:
>  > If file name too long it should just try to see if it is a reference
>  > to a revision.
> 
>  Looks easy enough to fix.
Maybe with a little bit more informative commit message? ;-)
Something like
	Avoid interpreting too-long parameter as file name
	Even if it is easier to write HEAD~2000, it is legal to write
	HEAD^^^... (repeats "^" 2000 times in total). However, such a
	string is too long to be a legal filename (and on Windows, by
	default even much, much shorter strings are still illegal
	because they exceed MAX_PATH).
	Therefore, if the check_filename() function encounters too long
	a command-line parameter, it should interpet the error code
	ENAMETOOLONG as a strong hint that this is not a file name
	instead of dying with an error message.
	Noticed-by: ...

What do you think? Dscho

Nguyễn Thái Ngọc Duy· Feb 8, 2016, 05:16 UTC · re: Johannes Schindelin · lore

[PATCH] Avoid interpreting too-long parameter as file name

Even if it is easier to write HEAD~2000, it is legal to write HEAD^^^... (repeats "^" 2000 times in total). However, such a string is too long to be a legal filename (and on Windows, by default even much, much shorter strings are still illegal because they exceed MAX_PATH).

Therefore, if the check_filename() function encounters too long a command-line parameter, it should interpet the error code ENAMETOOLONG as a strong hint that this is not a file name instead of dying with an error message.

Noticed-by: Ole Tange <ole@tange.dk>
Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Note, git grep ENOENT.*ENOTDIR reveals a couple more matches, but I
 didn't check if they should receive the same treatment.
 Another option is just use file_exists() here instead, but I guess
 that's too relaxing.
 setup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/setup.c b/setup.c
index 2c4b22c..ab8f85d 100644
--- a/setup.c
+++ b/setup.c
@@ -147,7 +147,7 @@ int check_filename(const char *prefix, const char *arg)
 		name = arg;
 	if (!lstat(name, &st))
 		return 1; /* file exists */
-	if (errno == ENOENT || errno == ENOTDIR)
+	if (errno == ENOENT || errno == ENOTDIR || errno == ENAMETOOLONG)
 		return 0; /* file does not exist */
 	die_errno("failed to stat '%s'", arg);
 }

-- 
2.7.0.377.g4cd97dd
Junio C Hamano· Feb 8, 2016, 20:46 UTC · re: Nguyễn Thái Ngọc Duy · lore

Re: [PATCH] Avoid interpreting too-long parameter as file name

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
Show 14 quoted lines
> Even if it is easier to write HEAD~2000, it is legal to write
> HEAD^^^... (repeats "^" 2000 times in total). However, such a string is
> too long to be a legal filename (and on Windows, by default even much,
> much shorter strings are still illegal because they exceed MAX_PATH).
>
> Therefore, if the check_filename() function encounters too long a
> command-line parameter, it should interpet the error code ENAMETOOLONG
> as a strong hint that this is not a file name instead of dying with an
> error message.
>
> Noticed-by: Ole Tange <ole@tange.dk>
> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

While I think it is a good thing to try solving, i.e. it would be nicer to the user if "git foo HEAD^^^^..." can be spelled without needing a "--" disambiguation, I am not sure this patch solves it at the right level. The log message is unclear if the patch author even thought about ramifications of the callers not involved in the case written in it.

For example, verify_filename() calls this function, saying "This string must name a file and otherwise I want you to die".

There is a direct call to check_filename() in builtin/checkout.c; it is unclear how it would interact with this change, either.

Kevin Daudt· Feb 23, 2016, 20:58 UTC · re: Ole Tange · lore

Re: git diff HEAD^(255) fails

On Sat, Feb 06, 2016 at 10:56:46PM +0100, Ole Tange wrote:
Show 13 quoted lines
> git diff first looks for a file, then looks if it is a reference to a
> revision. If the file fails due to being too long, the diff fails:
> 
> $ git init
> $ git diff 'HEAD^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^'
> HEAD
> fatal: failed to stat
> 'HEAD^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^':
> File name too long
> 
> If file name too long it should just try to see if it is a reference
> to a revision.
> 
Is there a reason you are repeating 255 "^" instead of using HEAD~255?

← back to recent threads