Re: Bug in git archive + .gitattributes + relative path
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 6, 2023, 22:34 UTC
- Message-ID
- <xmqq4jqx8q6q.fsf@gitster.g>
- In-Reply-To
- <3da35216-ca42-9759-d4f9-20451a44c231@web.de>
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 :/".
Show 143 quoted lines
> 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);