Subject: Re: Commit ID in exported Tar Ball

From: René Scharfe <[email-masked]>
Date: Sun, 20 May 2007 02:15:41 +0200

Junio C Hamano schrieb:
> René Scharfe <[email-masked]> writes:
> 
>> Something like the following patch?  Since we're already embedding
>> the commit ID in a comment, we might as well offer creating a
>> synthetic file for it, too, if that solves a user's problem that
>> might be difficult to work around otherwise.
> 
> Are you detecting and erroring out if the named file already exists
> in the tree being archived?
> 
> Should we?  Maybe we should just replace with warning?

Currently the commit ID file is appended to the archive, so if there is
a name conflict, we keep both files.  unzip offers a choice between
renaming, overwriting and keeping the first extracted version when the
second one is about to be extracted.  tar has a -k option: with -k you
get the first version, without -k you get the second one.

To avoid confusion, we should disallow a name that already comes from
the tree.  Only I can't see an easy way to implement this.  Perhaps a
check using get_tree_entry is enough -- it doesn't take pathspecs into
account, though.  That means we would disallow all names in the tree,
even if a pathspec excludes the chosen commit ID file name.

Before I roll my own path existence checker with pathspec support, is
there something like that already implemented?  I suspect it's hiding in
the diff code, but I don't dare go near it. ;-)

> Also should we silently ignore the request if the tree-ish is not a
> commit-ish, or error out?

As Angry said, erroring out is better.  It's also easy to do.

 Documentation/git-archive.txt |    4 ++++
 archive-tar.c                 |    7 +++++++
 archive-zip.c                 |    7 +++++++
 archive.h                     |    1 +
 builtin-archive.c             |   32 ++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index 721e035..7016d1e 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -43,6 +43,10 @@ OPTIONS
 --prefix=<prefix>/::
 	Prepend <prefix>/ to each filename in the archive.
 
+--commit-id-file=<filename>::
+	Adds a file to the archive containing the commit ID.  This option
+	is can only be used if <tree-ish> references a commit or tag.
+
 <extra>::
 	This can be any options that the archiver backend understand.
 	See next section.
diff --git a/archive-tar.c b/archive-tar.c
index 33e7657..555850a 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -319,6 +319,13 @@ int write_tar_archive(struct archiver_args *args)
 	}
 	read_tree_recursive(args->tree, args->base, plen, 0,
 			    args->pathspec, write_tar_entry);
+	if (args->commit_sha1 && args->commit_sha1_file) {
+		unsigned char fake_sha1[20];
+		pretend_sha1_file(sha1_to_hex(args->commit_sha1), 40,
+		                  OBJ_BLOB, fake_sha1);
+		write_tar_entry(fake_sha1, args->base, plen,
+		                args->commit_sha1_file, 0100666, 0);
+	}
 	write_trailer();
 
 	return 0;
diff --git a/archive-zip.c b/archive-zip.c
index 3cbf6bb..88c5dfa 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -328,6 +328,13 @@ int write_zip_archive(struct archiver_args *args)
 	}
 	read_tree_recursive(args->tree, args->base, plen, 0,
 			    args->pathspec, write_zip_entry);
+	if (args->commit_sha1 && args->commit_sha1_file) {
+		unsigned char fake_sha1[20];
+		pretend_sha1_file(sha1_to_hex(args->commit_sha1), 40,
+		                  OBJ_BLOB, fake_sha1);
+		write_zip_entry(fake_sha1, args->base, plen,
+		                args->commit_sha1_file, 0100666, 0);
+	}
 	write_zip_trailer(args->commit_sha1);
 
 	free(zip_dir);
diff --git a/archive.h b/archive.h
index 6838dc7..020f82f 100644
--- a/archive.h
+++ b/archive.h
@@ -8,6 +8,7 @@ struct archiver_args {
 	const char *base;
 	struct tree *tree;
 	const unsigned char *commit_sha1;
+	const char *commit_sha1_file;
 	time_t time;
 	const char **pathspec;
 	unsigned int verbose : 1;
diff --git a/builtin-archive.c b/builtin-archive.c
index 7f4e409..6bb0781 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -151,6 +151,7 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
 	int extra_argc = 0;
 	const char *format = "tar";
 	const char *base = "";
+	const char *commit_sha1_file = NULL;
 	int verbose = 0;
 	int i;
 
@@ -174,6 +175,10 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
 			base = arg + 9;
 			continue;
 		}
+		if (!prefixcmp(arg, "--commit-id-file=")) {
+			commit_sha1_file = arg + 17;
+			continue;
+		}
 		if (!strcmp(arg, "--")) {
 			i++;
 			break;
@@ -192,6 +197,11 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
 		usage(archive_usage);
 	if (init_archiver(format, ar) < 0)
 		die("Unknown archive format '%s'", format);
+	if (commit_sha1_file) {
+		size_t namelen = strlen(commit_sha1_file);
+		if (namelen == 0 || commit_sha1_file[namelen - 1] == '/')
+			die("Invalid commit ID file name: %s", commit_sha1_file);
+	}
 
 	if (extra_argc) {
 		if (!ar->parse_extra)
@@ -201,6 +211,7 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
 	}
 	ar->args.verbose = verbose;
 	ar->args.base = base;
+	ar->args.commit_sha1_file = commit_sha1_file;
 
 	return i;
 }
@@ -236,6 +247,20 @@ static const char *extract_remote_arg(int *ac, const char **av)
 	return remote;
 }
 
+static int is_path_in_spec(struct tree *tree, const char **pathspec,
+                           const char *path)
+{
+	unsigned char sha1[20];
+	unsigned int mode;
+
+	if (get_tree_entry(tree->object.sha1, path, sha1, &mode))
+		return 0;
+	if (!pathspec)
+		return 1;
+	/* TODO: the actual pathspec check */
+	return 1;
+}
+
 int cmd_archive(int argc, const char **argv, const char *prefix)
 {
 	struct archiver ar;
@@ -257,5 +282,12 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 	parse_treeish_arg(argv, &ar.args, prefix);
 	parse_pathspec_arg(argv + 1, &ar.args);
 
+	if (ar.args.commit_sha1_file) {
+		if (is_path_in_spec(ar.args.tree, ar.args.pathspec, ar.args.commit_sha1_file))
+			die("Commit ID file name already exists in archive.");
+		if (!ar.args.commit_sha1)
+			die("Need a commit to use --commit-id-file, and not a tree.");
+	}
+
 	return ar.write_archive(&ar.args);
 }