"Johannes Schindelin via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:
> @@ -61,6 +61,17 @@ OPTIONS
> by concatenating the value for `--prefix` (if any) and the
> basename of <file>.
>
> +--add-virtual-file=<path>:<content>::
> + Add the specified contents to the archive. Can be repeated to add
> + multiple files. The path of the file in the archive is built
> + by concatenating the value for `--prefix` (if any) and the
> + basename of <file>.
This sentence was copy-pasted from --add-file without adjusting.
There is no <file>; this new feature gives <path>.
Also, I suspect that the feature is losing end-user supplied
information without a good reason. --add-file=<file> may have
prepared an input in a randomly named temporary directory and it
would make quite a lot of sense to strip the leading directory
components from <file> and use only the basename part. But the
<path> given to "--add-virtual-file" does not refer to anything on
the filesystem. Its ONLY use is to be used as the path in the
archive to store the content. There is no justification why we
would discard the leading path components from it. I am not
decided, but I am inclined to say that we should not honor
"--prefix".
$ git archive --prefix=2.36.0 v2.36.0
would be a way to create a single directory and put everything in
the tree-ish in there, but there probably are cases where the user
of an "extra file" feature wants to add untracked cruft _in_ that
directory, and there are other cases where an extra file wants to go
to the top-level next to the 2.36.0 directory. A user can use the
same string as --prefix=<base> in front of <path> if the extra file
should go next to the top-level of the tree-ish, or without such
prefixing to place the extra file at the top-level.
Hence
Add the specified contents to the archive. Can be repeated
to add multiple files. `<path>` is used as the path of the
file in the archive.
would be what I would expect in a version of this feature that is
reasonably designed.
> ++
> +The `<path>` cannot contain any colon, the file mode is limited to
> +a regular file, and the option may be subject to platform-dependent
> +command-line limits. For non-trivial cases, write an untracked file
> +and use `--add-file` instead.
OK.
> diff --git a/archive.c b/archive.c
> index a3bbb091256..d20e16fa819 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -263,6 +263,7 @@ static int queue_or_write_archive_entry(const struct object_id *oid,
> struct extra_file_info {
> char *base;
> struct stat stat;
> + void *content;
> };
>
> int write_archive_entries(struct archiver_args *args,
> @@ -337,7 +338,13 @@ int write_archive_entries(struct archiver_args *args,
> strbuf_addstr(&path_in_archive, basename(path));
>
> strbuf_reset(&content);
> - if (strbuf_read_file(&content, path, info->stat.st_size) < 0)
> + if (info->content)
We ended up with the problematic "leading <path> components are
discarded" design only because the implementation reuses the logic
path_in_archive computation (the last line is seen in precontext),
which is a bit unfortunate. I think we could rewrite the inside of
that "for each extra file" loop like so, instead:
for (i = 0; i < args->extra_files.nr; i++) {
struct string_list_item *item = args->extra_files.items + i;
char *path = item->string;
struct extra_file_info *info = item->util;
put_be64(fake_oid.hash, i + 1);
if (!info->content) {
strbuf_reset(&path_in_archive);
if (info->base)
strbuf_addstr(&path_in_archive, info->base);
strbuf_addstr(&path_in_archive, basename(path));
strbuf_reset(&content);
if (strbuf_read_file(&content, path, info->stat.st_size) < 0)
err = error_errno(_("could not read '%s'"), path);
else
err = write_entry(args, &fake_oid, path_in_archive.buf,
path_in_archive.len,
info->stat.st_mode,
content.buf, content.len);
} else {
err = write_entry(args, &fake_oid,
path, strlen(path),
info->stat.st_mode,
info->content, info->stat.st_size);
}
if (err)
break;
}
The first half is the original code for "--add-file", which clears
info->content to NULL. We mangle the filename to come up with the
name in the archive (i.e. take basename and prefix with info->base).
The "else" side is the new code. "--add-virtual-file" has the
"<path>" thing in item->string, and info has the contents, so we
just write it out.
