Re: Bug in git archive + .gitattributes + relative path

[Junio C Hamano <gitster@pobox.com>]

René Scharfe <l.s.r@web.de> writes:

> Neither of them resolves "../$PWD/" parts to "" like git ls-tree does,
> but I can accept that difference.  And then we'd need to keep leading
> "../", I suppose.  Still unsure.

I offhand do not know how well it would mix with --strip-components
if we leave the leading "../".

But it certainly would be nice if we somehow:

 * can keep the current behaviour where "git -C sub archive" records
   paths relative to "sub" for backward compatibility.

 * fail loudly when "git -C sub archive <pathspec>" makes us use
   "../" prefix because <pathspec> goes above the $PWD for backward
   compatibility and sanity.

 * with --some-option, make "git -C sub archive --some-option :/"
   act exactly like "git archive :/".

> And I don't know why PATHSPEC_PREFER_CWD is necessary.
>
> So no sign-off, yet.
>
>
>  archive.c | 60 +++++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/archive.c b/archive.c
> index 9aeaf2bd87..c7e9f58b02 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -139,6 +139,7 @@ static int write_archive_entry(const struct object_id *oid, const char *base,
>  		void *context)
>  {
>  	static struct strbuf path = STRBUF_INIT;
> +	static struct strbuf scratch = STRBUF_INIT;
>  	struct archiver_context *c = context;
>  	struct archiver_args *args = c->args;
>  	write_archive_entry_fn_t write_entry = c->write_entry;
> @@ -148,6 +149,14 @@ static int write_archive_entry(const struct object_id *oid, const char *base,
>  	void *buffer;
>  	enum object_type type;
>
> +	/*
> +	 * NEEDSWORK: variable names could be clearer:
> +	 * - args->prefix is the current working directory,
> +	 * - args->base with args->baselen is the --prefix value,
> +	 * - base with baselen is the path of the current tree,
> +	 * - args->base + base + filename is the path in the archive,
> +	 * - path_without_prefix is base + filename.
> +	 */
>  	args->convert = 0;
>  	strbuf_reset(&path);
>  	strbuf_grow(&path, PATH_MAX);
> @@ -166,6 +175,15 @@ static int write_archive_entry(const struct object_id *oid, const char *base,
>  		args->convert = check_attr_export_subst(check);
>  	}
>
> +	if (args->prefix) {
> +		const char *rel = relative_path(path_without_prefix,
> +						args->prefix, &scratch);
> +		if (!strcmp(rel, "./"))
> +			return S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0;
> +		strbuf_setlen(&path, args->baselen);
> +		strbuf_addstr(&path, rel);
> +	}
> +
>  	if (args->verbose)
>  		fprintf(stderr, "%.*s\n", (int)path.len, path.buf);
>
> @@ -401,14 +419,15 @@ static int reject_entry(const struct object_id *oid UNUSED,
>  	return ret;
>  }
>
> -static int path_exists(struct archiver_args *args, const char *path)
> +static int path_exists(struct archiver_args *args, const char *prefix,
> +		       const char *path)
>  {
>  	const char *paths[] = { path, NULL };
>  	struct path_exists_context ctx;
>  	int ret;
>
>  	ctx.args = args;
> -	parse_pathspec(&ctx.pathspec, 0, 0, "", paths);
> +	parse_pathspec(&ctx.pathspec, 0, 0, prefix, paths);
>  	ctx.pathspec.recursive = 1;
>  	ret = read_tree(args->repo, args->tree,
>  			&ctx.pathspec,
> @@ -417,30 +436,35 @@ static int path_exists(struct archiver_args *args, const char *path)
>  	return ret != 0;
>  }
>
> -static void parse_pathspec_arg(const char **pathspec,
> +static void parse_pathspec_arg(const char **pathspec, const char *prefix,
>  		struct archiver_args *ar_args)
>  {
> +	const char *match_all[] = { ".", NULL };
> +
> +	if (prefix && !*pathspec)
> +		pathspec = match_all;
> +
>  	/*
>  	 * must be consistent with parse_pathspec in path_exists()
>  	 * Also if pathspec patterns are dependent, we're in big
>  	 * trouble as we test each one separately
>  	 */
>  	parse_pathspec(&ar_args->pathspec, 0,
> -		       PATHSPEC_PREFER_FULL,
> -		       "", pathspec);
> +		       PATHSPEC_PREFER_CWD,
> +		       prefix, pathspec);
>  	ar_args->pathspec.recursive = 1;
>  	if (pathspec) {
>  		while (*pathspec) {
> -			if (**pathspec && !path_exists(ar_args, *pathspec))
> +			if (**pathspec &&
> +			    !path_exists(ar_args, prefix, *pathspec))
>  				die(_("pathspec '%s' did not match any files"), *pathspec);
>  			pathspec++;
>  		}
>  	}
>  }
>
> -static void parse_treeish_arg(const char **argv,
> -		struct archiver_args *ar_args, const char *prefix,
> -		int remote)
> +static void parse_treeish_arg(const char **argv, struct archiver_args *ar_args,
> +			      int remote)
>  {
>  	const char *name = argv[0];
>  	const struct object_id *commit_oid;
> @@ -479,20 +503,6 @@ static void parse_treeish_arg(const char **argv,
>  	if (!tree)
>  		die(_("not a tree object: %s"), oid_to_hex(&oid));
>
> -	if (prefix) {
> -		struct object_id tree_oid;
> -		unsigned short mode;
> -		int err;
> -
> -		err = get_tree_entry(ar_args->repo,
> -				     &tree->object.oid,
> -				     prefix, &tree_oid,
> -				     &mode);
> -		if (err || !S_ISDIR(mode))
> -			die(_("current working directory is untracked"));
> -
> -		tree = parse_tree_indirect(&tree_oid);
> -	}
>  	ar_args->refname = ref;
>  	ar_args->tree = tree;
>  	ar_args->commit_oid = commit_oid;
> @@ -710,8 +720,8 @@ int write_archive(int argc, const char **argv, const char *prefix,
>  		setup_git_directory();
>  	}
>
> -	parse_treeish_arg(argv, &args, prefix, remote);
> -	parse_pathspec_arg(argv + 1, &args);
> +	parse_treeish_arg(argv, &args, remote);
> +	parse_pathspec_arg(argv + 1, prefix, &args);
>
>  	rc = ar->write_archive(ar, &args);