threads / discuss / 14860

something fishy with Git commit and log from file

Subject: something fishy with Git commit and log from file

## tl;dr

15 messages between Aug 6, 2008 and Aug 7, 2008.

replies: 14people: 6as markdown or json

Pascal Obry· Aug 6, 2008, 10:30 UTC · lore
The following command fails on my repository:
    $ git commit --file=clog
    fatal: could not read log file 'clog': No such file or directory
    $ cat clog
    toto
Using the following command the commit pass without problem:
    $ cat clog | git commit --file=-
With GDB I get:

$ gdb --args git ci --file=clog This GDB was configured as "i686-pc-cygwin"... (gdb) break strbuf_read_file Breakpoint 1 at 0x44e64a: file strbuf.c, line 301. (gdb) run Starting program: /usr/local/bin/git.exe ci --file=clog

Breakpoint 1, strbuf_read_file (sb=0x22cac0, path=0x22ccfa "clog", hint=0) at strbuf.c:301 301 fd = open(path, O_RDONLY); (gdb) print path $1 = 0x0

???? outch, this is strange, or a gdb artifact?

(gdb) n 302 if (fd < 0) (gdb) print fd $2 = -1 (gdb) print *0x22ccfa $3 = 1735355491 (gdb) print (char)*0x22ccfa $4 = 99 'c' (gdb) print (char)*0x22ccfb $5 = 108 'l' (gdb) print (char)*0x22ccfc $6 = 111 'o' (gdb) print (char)*0x22ccfd $7 = 103 'g' (gdb) print (char)*0x22ccfe $8 = 0 '\0'

But looks like path really contains 'clog'!!!

No luck for now to find the problem. Note that the same command pass fine on other repositories. At this point this looks really like some kind of memory corruption...

I'm on Windows, using Git for Cygwin and the compiler is:

$ gcc --version gcc (GCC) 3.4.4 (cygming special, gdc 0.12, using dmd 0.125)

Pascal.
-- 
--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
Luciano Rocha· Aug 6, 2008, 10:44 UTC · re: Pascal Obry · lore

Re: something fishy with Git commit and log from file

On Wed, Aug 06, 2008 at 12:30:06PM +0200, Pascal Obry wrote:
Show 29 quoted lines
> 
>  The following command fails on my repository:
> 
>     $ git commit --file=clog
>     fatal: could not read log file 'clog': No such file or directory
> 
>     $ cat clog
>     toto
> 
>  Using the following command the commit pass without problem:
> 
>     $ cat clog | git commit --file=-
> 
>  With GDB I get:
> 
>  $ gdb --args git ci --file=clog
>  This GDB was configured as "i686-pc-cygwin"...
>  (gdb) break strbuf_read_file
>  Breakpoint 1 at 0x44e64a: file strbuf.c, line 301.
>  (gdb) run
>  Starting program: /usr/local/bin/git.exe ci --file=clog
> 
>  Breakpoint 1, strbuf_read_file (sb=0x22cac0, path=0x22ccfa "clog", hint=0) 
>  at strbuf.c:301
>  301             fd = open(path, O_RDONLY);
>  (gdb) print path
>  $1 = 0x0
> 
>  ???? outch, this is strange, or a gdb artifact?
gdb artifact. The breakpoint info shows the correct value.
Could you at that point run the following?

(gdb) p get_current_dir_name() $1 = ... "..."

If the returned value doesn't match the directory you were in, then there's a bug in git.

If the function doesn't exist, try: (gdb) p getwd(malloc(2048))

Regards, Luciano Rocha

-- 
Luciano Rocha <luciano@eurotux.com>
Eurotux Informática, S.A. <http://www.eurotux.com/>
Pascal Obry· Aug 6, 2008, 11:01 UTC · re: Luciano Rocha · lore

Re: something fishy with Git commit and log from file

Luciano,
> If the function doesn't exist, try:
> (gdb) p getwd(malloc(2048))
I got:

(gdb) p (char *)getwd(malloc(2048)) $3 = 0x903680 "/home/obry/dev/repositories/git/AWS"

And this is not the directory I was in. It is the Git root. I was under /home/obry/dev/repositories/git/AWS/regtests when running the command.

So definitely a Git bug! Can be reproduced with:
    $ mkdir repo && cd repo
    $ git init
    $ mkdir dir
    $ cd dir
    $ echo file > file
    $ echo log > log
    $ git add file
    $ git commit --file=log
    fatal: could not read log file 'log': No such file or directory
Pascal.
-- 
--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
Junio C Hamano· Aug 6, 2008, 15:38 UTC · re: Pascal Obry · lore

Re: something fishy with Git commit and log from file

Pascal Obry <pascal@obry.net> writes:
Show 11 quoted lines
> So definitely a Git bug! Can be reproduced with:
>
>    $ mkdir repo && cd repo
>    $ git init
>    $ mkdir dir
>    $ cd dir
>    $ echo file > file
>    $ echo log > log
>    $ git add file
>    $ git commit --file=log
>    fatal: could not read log file 'log': No such file or directory
Try it without cding down to "dir".
Pascal Obry· Aug 6, 2008, 16:28 UTC · re: Junio C Hamano · lore

Re: something fishy with Git commit and log from file

Junio C Hamano a écrit :
Show 15 quoted lines
> Pascal Obry <pascal@obry.net> writes:
> 
>> So definitely a Git bug! Can be reproduced with:
>>
>>    $ mkdir repo && cd repo
>>    $ git init
>>    $ mkdir dir
>>    $ cd dir
>>    $ echo file > file
>>    $ echo log > log
>>    $ git add file
>>    $ git commit --file=log
>>    fatal: could not read log file 'log': No such file or directory
> 
> Try it without cding down to "dir".
Yes it works. It also works if I do:
     $ echo log > ../log
instead of
     $ echo log > log
Git is looking for the log file at the Git root.
Pascal.
-- 
--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
Junio C Hamano· Aug 6, 2008, 16:50 UTC · re: Pascal Obry · lore

Re: something fishy with Git commit and log from file

Pascal Obry <pascal@obry.net> writes:
Show 18 quoted lines
> Junio C Hamano a écrit :
>> Pascal Obry <pascal@obry.net> writes:
>>
>>> So definitely a Git bug! Can be reproduced with:
>>>
>>>    $ mkdir repo && cd repo
>>>    $ git init
>>>    $ mkdir dir
>>>    $ cd dir
>>>    $ echo file > file
>>>    $ echo log > log
>>>    $ git add file
>>>    $ git commit --file=log
>>>    fatal: could not read log file 'log': No such file or directory
>>
>> Try it without cding down to "dir".
>
> Yes it works.

Perhaps something like this. This must be another one of those regressions introduced in C rewrite.

diff --git a/builtin-commit.c b/builtin-commit.c
index b783e6e..fcc9c59 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -469,7 +469,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 			die("could not read log from standard input");
 		hook_arg1 = "message";
 	} else if (logfile) {
-		if (strbuf_read_file(&sb, logfile, 0) < 0)
+		const char *lf = logfile;
+		if (prefix)
+			lf = prefix_filename(prefix, strlen(prefix), logfile);
+		if (strbuf_read_file(&sb, lf, 0) < 0)
 			die("could not read log file '%s': %s",
 			    logfile, strerror(errno));
 		hook_arg1 = "message";
Pascal Obry· Aug 6, 2008, 17:08 UTC · re: Junio C Hamano · lore

Re: something fishy with Git commit and log from file

Junio C Hamano a écrit :
> Perhaps something like this.  This must be another one of those
> regressions introduced in C rewrite.
Works fine now. Thanks.
Pascal.
-- 
--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
Junio C Hamano· Aug 6, 2008, 18:43 UTC · re: Junio C Hamano · lore

[PATCH] files given on the command line are relative to $cwd

When running "git commit -F file" and "git tag -F file" from a subdirectory, we should take it as relative to the directory we started from, not relative to the top-level directory.

This adds a helper function "parse_options_fix_filename()" to make it more convenient to fix this class of issues. Ideally, parse_options() should support a new type of option, "OPT_FILENAME", to do this uniformly, but this patch is meant to go to 'maint' to fix it minimally.

One thing to note is that value for "commit template file" that comes from the command line is taken as relative to $cwd just like other parameters, but when it comes from the configuration varilable 'commit.template', it is taken as relative to the working tree root as before. I think this difference actually is sensible (not that I particularly think commit.template itself is sensible).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-commit.c  |   11 +++++++----
 builtin-tag.c     |    1 +
 parse-options.c   |   12 ++++++++++++
 parse-options.h   |    2 ++
 t/t7500-commit.sh |   11 +++++++++++
 5 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/builtin-commit.c b/builtin-commit.c
index bcbea38..0c6d1f4 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -45,7 +45,7 @@ static enum {
 	COMMIT_PARTIAL,
 } commit_style;
 
-static char *logfile, *force_author;
+static const char *logfile, *force_author;
 static const char *template_file;
 static char *edit_message, *use_message;
 static char *author_name, *author_email, *author_date;
@@ -700,11 +700,14 @@ static int message_is_empty(struct strbuf *sb, int start)
 }
 
 static int parse_and_validate_options(int argc, const char *argv[],
-				      const char * const usage[])
+				      const char * const usage[],
+				      const char *prefix)
 {
 	int f = 0;
 
 	argc = parse_options(argc, argv, builtin_commit_options, usage, 0);
+	logfile = parse_options_fix_filename(prefix, logfile);
+	template_file = parse_options_fix_filename(prefix, template_file);
 
 	if (logfile || message.len || use_message)
 		use_editor = 0;
@@ -814,7 +817,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 	if (wt_status_use_color == -1)
 		wt_status_use_color = git_use_color_default;
 
-	argc = parse_and_validate_options(argc, argv, builtin_status_usage);
+	argc = parse_and_validate_options(argc, argv, builtin_status_usage, prefix);
 
 	index_file = prepare_index(argc, argv, prefix);
 
@@ -907,7 +910,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	git_config(git_commit_config, NULL);
 
-	argc = parse_and_validate_options(argc, argv, builtin_commit_usage);
+	argc = parse_and_validate_options(argc, argv, builtin_commit_usage, prefix);
 
 	index_file = prepare_index(argc, argv, prefix);
 
diff --git a/builtin-tag.c b/builtin-tag.c
index 3c97c69..3f77ba9 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -411,6 +411,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	git_config(git_tag_config, NULL);
 
 	argc = parse_options(argc, argv, options, git_tag_usage, 0);
+	msgfile = parse_options_fix_filename(prefix, msgfile);
 
 	if (keyid) {
 		sign = 1;
diff --git a/parse-options.c b/parse-options.c
index f8d52e2..d771bf4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -425,3 +425,15 @@ int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
 	*(unsigned long *)(opt->value) = approxidate(arg);
 	return 0;
 }
+
+/*
+ * This should really be OPTION_FILENAME type as a part of
+ * parse_options that take prefix to do this while parsing.
+ */
+extern const char *parse_options_fix_filename(const char *prefix, const char *file)
+{
+	if (!file || !prefix || is_absolute_path(file))
+		return file;
+	return prefix_filename(prefix, strlen(prefix), file);
+}
+
diff --git a/parse-options.h b/parse-options.h
index 4ee443d..13ad158 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -123,4 +123,6 @@ extern int parse_opt_approxidate_cb(const struct option *, const char *, int);
 	  "use <n> digits to display SHA-1s", \
 	  PARSE_OPT_OPTARG, &parse_opt_abbrev_cb, 0 }
 
+extern const char *parse_options_fix_filename(const char *prefix, const char *file);
+
 #endif
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index baed6ce..026d787 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -138,4 +138,15 @@ test_expect_success '--signoff' '
 	diff expect output
 '
 
+test_expect_success 'commit message from file' '
+	mkdir subdir &&
+	echo "Log in top directory" >log &&
+	echo "Log in sub directory" >subdir/log &&
+	(
+		cd subdir &&
+		git commit --allow-empty -F log
+	) &&
+	commit_msg_is "Log in sub directory"
+'
+
 test_done
Olivier Marin· Aug 6, 2008, 20:14 UTC · re: Junio C Hamano · lore

Re: [PATCH] files given on the command line are relative to $cwd

Junio C Hamano a écrit :
Show 10 quoted lines
>  
>  static int parse_and_validate_options(int argc, const char *argv[],
> -				      const char * const usage[])
> +				      const char * const usage[],
> +				      const char *prefix)
>  {
>  	int f = 0;
>  
>  	argc = parse_options(argc, argv, builtin_commit_options, usage, 0);
> +	logfile = parse_options_fix_filename(prefix, logfile);
It breaks the "git commit -F -" case, no?
Olivier.
Junio C Hamano· Aug 6, 2008, 20:19 UTC · re: Olivier Marin · lore

Re: [PATCH] files given on the command line are relative to $cwd

Olivier Marin <dkr+ml.git@free.fr> writes:
Show 13 quoted lines
> Junio C Hamano a écrit :
>>  
>>  static int parse_and_validate_options(int argc, const char *argv[],
>> -				      const char * const usage[])
>> +				      const char * const usage[],
>> +				      const char *prefix)
>>  {
>>  	int f = 0;
>>  
>>  	argc = parse_options(argc, argv, builtin_commit_options, usage, 0);
>> +	logfile = parse_options_fix_filename(prefix, logfile);
>
> It breaks the "git commit -F -" case, no?
Does it?  Ah, yeah, t7500 #15 does not go down to a subdirectory.
Junio C Hamano· Aug 6, 2008, 20:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] files given on the command line are relative to $cwd

Junio C Hamano <gitster@pobox.com> writes:
Show 17 quoted lines
> Olivier Marin <dkr+ml.git@free.fr> writes:
>
>> Junio C Hamano a écrit :
>>>  
>>>  static int parse_and_validate_options(int argc, const char *argv[],
>>> -				      const char * const usage[])
>>> +				      const char * const usage[],
>>> +				      const char *prefix)
>>>  {
>>>  	int f = 0;
>>>  
>>>  	argc = parse_options(argc, argv, builtin_commit_options, usage, 0);
>>> +	logfile = parse_options_fix_filename(prefix, logfile);
>>
>> It breaks the "git commit -F -" case, no?
>
> Does it?  Ah, yeah, t7500 #15 does not go down to a subdirectory.
Ok, this squashed in on top of the previous one should cover the case.

Thanks for saving me in time from a major embarrassment. I already tagged 1.5.6.5 with the botched one but haven't pushed it out, so I can safely rewind.

---
 parse-options.c   |    2 +-
 t/t7500-commit.sh |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index d771bf4..12c8822 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -432,7 +432,7 @@ int parse_opt_approxidate_cb(const struct option *opt, const char *arg,
  */
 extern const char *parse_options_fix_filename(const char *prefix, const char *file)
 {
-	if (!file || !prefix || is_absolute_path(file))
+	if (!file || !prefix || is_absolute_path(file) || !strcmp("-", file))
 		return file;
 	return prefix_filename(prefix, strlen(prefix), file);
 }
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 2ab791b..823256a 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -159,4 +159,12 @@ test_expect_success 'commit message from file (2)' '
 	commit_msg_is "Log in sub directory"
 '
 
+test_expect_success 'commit message from stdin' '
+	(
+		cd subdir &&
+		echo "Log with foo word" | git commit --allow-empty -F -
+	) &&
+	commit_msg_is "Log with foo word"
+'
+
 test_done
Pierre Habouzit· Aug 6, 2008, 20:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] files given on the command line are relative to $cwd

On Wed, Aug 06, 2008 at 06:43:47PM +0000, Junio C Hamano wrote:
Show 8 quoted lines
> When running "git commit -F file" and "git tag -F file" from a
> subdirectory, we should take it as relative to the directory we started
> from, not relative to the top-level directory.
> 
> This adds a helper function "parse_options_fix_filename()" to make it more
> convenient to fix this class of issues.  Ideally, parse_options() should
> support a new type of option, "OPT_FILENAME", to do this uniformly, but
> this patch is meant to go to 'maint' to fix it minimally.
  I'm going in vacation tomorrow so I'm not likely to do that soon, but
I agree it's sensible.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org
Samuel Tardieu· Aug 7, 2008, 08:45 UTC · re: Junio C Hamano · lore

Re: [PATCH] files given on the command line are relative to $cwd

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

Junio> When running "git commit -F file" and "git tag -F file" from a Junio> subdirectory, we should take it as relative to the directory we Junio> started from, not relative to the top-level directory.

Don't we have the same problem with "git show"? If you go into the "gitweb" directory of the GIT source, "git show HEAD:README" will show you the toplevel "README" instead of the one in the "gitweb" directory.

  Sam
-- 
Samuel Tardieu -- sam@rfc1149.net -- http://www.rfc1149.net/
Luciano Rocha· Aug 7, 2008, 09:03 UTC · re: Samuel Tardieu · lore

Re: [PATCH] files given on the command line are relative to $cwd

On Thu, Aug 07, 2008 at 10:45:21AM +0200, Samuel Tardieu wrote:
Show 10 quoted lines
> >>>>> "Junio" == Junio C Hamano <gitster@pobox.com> writes:
> 
> Junio> When running "git commit -F file" and "git tag -F file" from a
> Junio> subdirectory, we should take it as relative to the directory we
> Junio> started from, not relative to the top-level directory.
> 
> Don't we have the same problem with "git show"? If you go into
> the "gitweb" directory of the GIT source, "git show HEAD:README" will
> show you the toplevel "README" instead of the one in the "gitweb"
> directory.

No, git show has different semantics. It has been discussed often in this list.

For instance: http://thread.gmane.org/gmane.comp.version-control.git/68786/focus=68852

-- 
Luciano Rocha <luciano@eurotux.com>
Eurotux Informática, S.A. <http://www.eurotux.com/>
Junio C Hamano· Aug 7, 2008, 09:47 UTC · re: Luciano Rocha · lore

Re: [PATCH] files given on the command line are relative to $cwd

Luciano Rocha <luciano@eurotux.com> writes:
Show 14 quoted lines
> On Thu, Aug 07, 2008 at 10:45:21AM +0200, Samuel Tardieu wrote:
>> >>>>> "Junio" == Junio C Hamano <gitster@pobox.com> writes:
>> 
>> Junio> When running "git commit -F file" and "git tag -F file" from a
>> Junio> subdirectory, we should take it as relative to the directory we
>> Junio> started from, not relative to the top-level directory.
>> 
>> Don't we have the same problem with "git show"? If you go into
>> the "gitweb" directory of the GIT source, "git show HEAD:README" will
>> show you the toplevel "README" instead of the one in the "gitweb"
>> directory.
>
> No, git show has different semantics. It has been discussed often in
> this list.
You are half correct --- it is not show but tree:path syntax.

← back to recent threads