Git archive and trailing "/" in prefix

Git archive and trailing "/" in prefix

[Sergio Callegari]

Hi!

The git-archive man page indicates that if the --prefix option is passed to
git-archive, it is compulsory to end the prefix with a "/"

 git archive [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>] ...

As a matter of fact, the archiver behaves quite strangely if that slash is
missing. Files in the root of the working dir are added to the archive with
their own name modified by the prefix and the same happens for working dir
sub-directories. However, no file present in the sub-directories, nor
sub-sub-directories are added.

I would like to know if there some reason why a trailing "/" is not added
automatically to the prefix when it is missing and the prefix is not empty.
Would that break anything?

Thanks!

Sergio

Re: Git archive and trailing "/" in prefix

[Junio C Hamano]

Sergio Callegari <sergio.callegari@gmail.com> writes:

> The git-archive man page indicates that if the --prefix option is passed to
> git-archive, it is compulsory to end the prefix with a "/"

No, it does not have to.

	$ git archive --prefix=v1.6.0- v1.6.0 Makefile | tar xf -
	$ make -f v1.6.0-Makefile

This is consistent with the way the same --prefix option can be used with
checkout-index.  e.g. to swap Makefile in work tree and in the index:

	$ edit Makefile
	$ git checkout-index --prefix=old- Makefile
	$ git update-index Makefile
        $ mv old-Makefile Makefile

These may or may not be useful examples, but this feature has been with us
for a long time.  I wouldn't be surprised if removing the ability to
archive or checkout with filename prefix (not leading directory path
prefix) causes grief to existing scripts of people.

Re: Git archive and trailing "/" in prefix

[René Scharfe]

Sergio Callegari schrieb:
> Hi!
> 
> The git-archive man page indicates that if the --prefix option is passed to
> git-archive, it is compulsory to end the prefix with a "/"
> 
>  git archive [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>] ...
> 
> As a matter of fact, the archiver behaves quite strangely if that slash is
> missing. Files in the root of the working dir are added to the archive with
> their own name modified by the prefix and the same happens for working dir
> sub-directories. However, no file present in the sub-directories, nor
> sub-sub-directories are added.

The latter is a bug.

> I would like to know if there some reason why a trailing "/" is not added
> automatically to the prefix when it is missing and the prefix is not empty.
> Would that break anything?

The --prefix option is intended to add a string to the beginning (i.e. "to
prefix") of the name of the archive entries.  I'm not sure if there's a use
case for anything else than adding a fake directory for all entries to live
in (thus requiring a trailing slash), but I also don't see why we should
disallow it.

The following patch fixes handling of prefixes without trailing slashes by
taking it out of the hands of get_pathspec() and read_tree_recursive() --
which can only handle prefixes that are path components -- and adding the
prefix later, in write_archive_entry().  

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 archive.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/archive.c b/archive.c
index 73b8e8a..0cc79d2 100644
--- a/archive.c
+++ b/archive.c
@@ -115,6 +115,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,
 
 	strbuf_reset(&path);
 	strbuf_grow(&path, PATH_MAX);
+	strbuf_add(&path, args->base, args->baselen);
 	strbuf_add(&path, base, baselen);
 	strbuf_addstr(&path, filename);
 	path_without_prefix = path.buf + args->baselen;
@@ -187,8 +188,8 @@ int write_archive_entries(struct archiver_args *args,
 		git_attr_set_direction(GIT_ATTR_INDEX, &the_index);
 	}
 
-	err =  read_tree_recursive(args->tree, args->base, args->baselen, 0,
-			args->pathspec, write_archive_entry, &context);
+	err = read_tree_recursive(args->tree, "", 0, 0, args->pathspec,
+				  write_archive_entry, &context);
 	if (err == READ_TREE_RECURSIVE)
 		err = 0;
 	return err;
@@ -211,7 +212,7 @@ static const struct archiver *lookup_archiver(const char *name)
 static void parse_pathspec_arg(const char **pathspec,
 		struct archiver_args *ar_args)
 {
-	ar_args->pathspec = get_pathspec(ar_args->base, pathspec);
+	ar_args->pathspec = get_pathspec("", pathspec);
 }
 
 static void parse_treeish_arg(const char **argv,

Re: Git archive and trailing "/" in prefix

[Linus Torvalds]

On Thu, 8 Oct 2009, Sergio Callegari wrote:
> 
> The git-archive man page indicates that if the --prefix option is passed to
> git-archive, it is compulsory to end the prefix with a "/"

Yeah, that part is intentional. And:

> As a matter of fact, the archiver behaves quite strangely if that slash is
> missing. Files in the root of the working dir are added to the archive with
> their own name modified by the prefix and the same happens for working dir
> sub-directories.

So far so good. But

> However, no file present in the sub-directories, nor sub-sub-directories 
> are added.

Ok, that is a bug. It's supposed to just add the prefix to everything, and 
it sounds like it's simply broken. I wonder how long it's been broken? 
Perhaps forever. 

> I would like to know if there some reason why a trailing "/" is not added
> automatically to the prefix when it is missing and the prefix is not empty.
> Would that break anything?

It really was meant to be useful to prefix things without forcing a 
directory structure. IOW, being able to use "--prefix=compat-" and just 
have everything unpack with their own names, but with the prefix.

Whether anybody uses that, and whether it's worth it, I can't say.

			Linus

Re: Git archive and trailing "/" in prefix

[Sergio Callegari]

Junio C Hamano wrote:
> Sergio Callegari <sergio.callegari@gmail.com> writes:
>
>   
>> The git-archive man page indicates that if the --prefix option is passed to
>> git-archive, it is compulsory to end the prefix with a "/"
>>     
>
> No, it does not have to.
>
> 	$ git archive --prefix=v1.6.0- v1.6.0 Makefile | tar xf -
> 	$ make -f v1.6.0-Makefile
>   
Thanks... I now see better all the possible uses.

> This is consistent with the way the same --prefix option can be used with
> checkout-index.  e.g. to swap Makefile in work tree and in the index:
>
> 	$ edit Makefile
> 	$ git checkout-index --prefix=old- Makefile
> 	$ git update-index Makefile
>         $ mv old-Makefile Makefile
>
> These may or may not be useful examples, but this feature has been with us
> for a long time.  I wouldn't be surprised if removing the ability to
> archive or checkout with filename prefix (not leading directory path
> prefix) causes grief to existing scripts of people.
>   
That's why I asked btw. I did not want to start experimenting 
modification like auto adding a "/"
after the prefix without knowing whether they could have limited some 
other uses or broken some
consistency. I now see they would do.

I guess the bug in using --prefix on a worktree with subdirs without 
specifying a path is not specific
to git archive, then.

Sergio
>   
>
>   

Re: Git archive and trailing "/" in prefix

[Junio C Hamano]

René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:

> Sergio Callegari schrieb:
> ...
>> As a matter of fact, the archiver behaves quite strangely if that slash is
>> missing. Files in the root of the working dir are added to the archive with
>> their own name modified by the prefix and the same happens for working dir
>> sub-directories. However, no file present in the sub-directories, nor
>> sub-sub-directories are added.
>
> The latter is a bug.
> ...
> The following patch fixes...

Thanks.  Applied with a trivial test.

Re: Git archive and trailing "/" in prefix

[René Scharfe]

Sergio Callegari schrieb:
> I guess the bug in using --prefix on a worktree with subdirs without
> specifying a path is not specific to git archive, then.

The bug should be limited to archive; after my patch all calls to
read_tree_recursive() specify an empty base parameter (except in tree.c,
where the function itself lives).

René