Re: [PATCH v6 1/7] archive: optionally add "virtual" files

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

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.

OK.

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.