Subject: Re: git archive: tar.umask ignored

From: René Scharfe <[email-masked]>
Date: Tue, 12 Apr 2011 22:02:35 +0200

Am 12.04.2011 20:19, schrieb René Scharfe:
> Would a new --umask command line option alone (that overrides any config 
> setting) be sufficient for your use case?

Something like this (docs and tests missing)?

I'm not so sure about PARSE_OPT_OPTARG.  E.g. this is confusing and even
becomes slightly dangerous if you have a branch named 022 with a file
named HEAD (the = is required):

	$ git archive --umask 022 HEAD
	fatal: Not a valid object name

Also I'm and not sure what --no-umask should do.  In this patch it is
equivalent to --umask=0; perhaps it should rather make archive use the
configured tar.umask value, i.e. neutralize any previous --umask
option.
---
 archive-tar.c |    5 ++++-
 archive.c     |   34 +++++++++++++++++++++++++++++++++-
 archive.h     |    1 +
 3 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/archive-tar.c b/archive-tar.c
index cee06ce..1a29f3c 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -238,7 +238,10 @@ int write_tar_archive(struct archiver_args *args)
 {
 	int err = 0;
 
-	git_config(git_tar_config, NULL);
+	if (args->umask == -1)
+		git_config(git_tar_config, NULL);
+	else
+		tar_umask = args->umask;
 
 	if (args->commit_sha1)
 		err = write_global_extended_header(args);
diff --git a/archive.c b/archive.c
index 1944ed4..2ec5391 100644
--- a/archive.c
+++ b/archive.c
@@ -15,13 +15,14 @@ static char const * const archive_usage[] = {
 };
 
 #define USES_ZLIB_COMPRESSION 1
+#define USES_UMASK 2
 
 static const struct archiver {
 	const char *name;
 	write_archive_fn_t write_archive;
 	unsigned int flags;
 } archivers[] = {
-	{ "tar", write_tar_archive },
+	{ "tar", write_tar_archive, USES_UMASK },
 	{ "zip", write_zip_archive, USES_ZLIB_COMPRESSION },
 };
 
@@ -285,6 +286,30 @@ static void parse_treeish_arg(const char **argv,
 	ar_args->time = archive_time;
 }
 
+static int umask_callback(const struct option *opt, const char *arg, int unset)
+{
+	int *target = opt->value;
+	int value;
+	const char *endp;
+
+	if (unset) {
+		*target = 0;
+		return 0;
+	}
+	if (arg) {
+		value = strtol(arg, (char **)&endp, 0);
+		if (*endp || value < 0) {
+			return error("option `%s' %s", "umask",
+				     "expects a non-negative numerical value");
+		}
+	} else {
+		value = umask(0);
+		umask(value);
+	}
+	*target = value;
+	return 0;
+}
+
 #define OPT__COMPR(s, v, h, p) \
 	{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
 	  PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
@@ -305,6 +330,7 @@ static int parse_archive_args(int argc, const char **argv,
 	int i;
 	int list = 0;
 	int worktree_attributes = 0;
+	int umask = -1;
 	struct option opts[] = {
 		OPT_GROUP(""),
 		OPT_STRING(0, "format", &format, "fmt", "archive format"),
@@ -325,6 +351,9 @@ static int parse_archive_args(int argc, const char **argv,
 		OPT__COMPR_HIDDEN('7', &compression_level, 7),
 		OPT__COMPR_HIDDEN('8', &compression_level, 8),
 		OPT__COMPR('9', &compression_level, "compress better", 9),
+		{ OPTION_CALLBACK, 0, "umask", &umask, "umask",
+			"apply user's umask or <umask> to archived files",
+			PARSE_OPT_OPTARG, umask_callback },
 		OPT_GROUP(""),
 		OPT_BOOLEAN('l', "list", &list,
 			"list supported archive formats"),
@@ -370,10 +399,13 @@ static int parse_archive_args(int argc, const char **argv,
 					format, compression_level);
 		}
 	}
+	if (umask != -1 && !((*ar)->flags & USES_UMASK))
+		die("Argument not supported for format '%s': --umask", format);
 	args->verbose = verbose;
 	args->base = base;
 	args->baselen = strlen(base);
 	args->worktree_attributes = worktree_attributes;
+	args->umask = umask;
 
 	return argc;
 }
diff --git a/archive.h b/archive.h
index 038ac35..083675c 100644
--- a/archive.h
+++ b/archive.h
@@ -12,6 +12,7 @@ struct archiver_args {
 	unsigned int verbose : 1;
 	unsigned int worktree_attributes : 1;
 	int compression_level;
+	int umask;
 };
 
 typedef int (*write_archive_fn_t)(struct archiver_args *);
-- 
1.7.4