[PATCH 4/9] archive: omit the shell for built-in "command" filters

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Since the "tar.<format.command" interface was added in [1] we've
promised to invoke the shell to run if e.g. "gzip -cn" is
configured. That common format was then added as a default in [2].

But if we have no such configuration we can safely assume that the
user isn't expecting the "gzip" to be invoked via a shell, and we can
skip the "sh" process.

We are intentionally not treating a configured
"tar.<format>.command=<cmd>" where "<cmd>" is equivalent to our
hardcoded "<cmd>" the same as when the same "<cmd>" is specified in
the config. If the user has configured e.g. "gzip -cn" they may be
relying on what the shell gives them over a direct execve() of "gzip".

This makes us marginally faster, but the real point is to make the
error handling easier to deal with. When we're using the shell we
don't know if e.g. the "gzip" we spawned fails as easily,
i.e. "start_command()" won't fail, because we can find the "sh".

A subsequent commit will tweak the default that [3] introduced to be a
fallback instead, at which point we'll need this for correctness.

1. 767cf4579f0 (archive: implement configurable tar filters, 2011-06-21)
2. 0e804e09938 (archive: provide builtin .tar.gz filter, 2011-06-21)
3. 4f4be00d302 (archive-tar: use internal gzip by default, 2022-06-15)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
 Documentation/config/tar.txt |  3 +++
 archive-tar.c                | 17 +++++++++++++----
 archive.h                    |  1 +
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/config/tar.txt b/Documentation/config/tar.txt
index 894c1163bb9..5456fc617a2 100644
--- a/Documentation/config/tar.txt
+++ b/Documentation/config/tar.txt
@@ -18,6 +18,9 @@ tar.<format>.command::
 The `tar.gz` and `tgz` formats are defined automatically and use the
 magic command `git archive gzip` by default, which invokes an internal
 implementation of gzip.
++
+The automatically defined commands do not invoke the shell, avoiding
+the minor overhead of an extra sh(1) process.
 
 tar.<format>.remote::
 	If true, enable the format for use by remote clients via
diff --git a/archive-tar.c b/archive-tar.c
index f8fad2946ef..8c5de949c64 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -367,12 +367,13 @@ static struct archiver *find_tar_filter(const char *name, size_t len)
 }
 
 static int tar_filter_config(const char *var, const char *value,
-			     void *data UNUSED)
+			     void *data)
 {
 	struct archiver *ar;
 	const char *name;
 	const char *type;
 	size_t namelen;
+	int *configured = data;
 
 	if (parse_config_key(var, "tar", &name, &namelen, &type) < 0 || !name)
 		return 0;
@@ -388,6 +389,9 @@ static int tar_filter_config(const char *var, const char *value,
 		tar_filters[nr_tar_filters++] = ar;
 	}
 
+	if (configured && *configured)
+		ar->flags |= ARCHIVER_COMMAND_FROM_CONFIG;
+
 	if (!strcmp(type, "command")) {
 		if (!value)
 			return config_error_nonbool(var);
@@ -495,8 +499,12 @@ static int write_tar_filter_archive(const struct archiver *ar,
 	if (args->compression_level >= 0)
 		strbuf_addf(&cmd, " -%d", args->compression_level);
 
-	strvec_push(&filter.args, cmd.buf);
-	filter.use_shell = 1;
+	if (ar->flags & ARCHIVER_COMMAND_FROM_CONFIG) {
+		strvec_push(&filter.args, cmd.buf);
+		filter.use_shell = 1;
+	} else {
+		strvec_split(&filter.args, cmd.buf);
+	}
 	filter.in = -1;
 	filter.silent_exec_failure = 1;
 
@@ -526,13 +534,14 @@ static struct archiver tar_archiver = {
 void init_tar_archiver(void)
 {
 	int i;
+	int configured = 1;
 	register_archiver(&tar_archiver);
 
 	tar_filter_config("tar.tgz.command", internal_gzip_command, NULL);
 	tar_filter_config("tar.tgz.remote", "true", NULL);
 	tar_filter_config("tar.tar.gz.command", internal_gzip_command, NULL);
 	tar_filter_config("tar.tar.gz.remote", "true", NULL);
-	git_config(git_tar_config, NULL);
+	git_config(git_tar_config, &configured);
 	for (i = 0; i < nr_tar_filters; i++) {
 		/* omit any filters that never had a command configured */
 		if (tar_filters[i]->filter_command)
diff --git a/archive.h b/archive.h
index 6b51288c2ed..9686b3b5cc1 100644
--- a/archive.h
+++ b/archive.h
@@ -40,6 +40,7 @@ enum archiver_flags {
 	ARCHIVER_WANT_COMPRESSION_LEVELS = 1<<0,
 	ARCHIVER_REMOTE = 1<<1,
 	ARCHIVER_HIGH_COMPRESSION_LEVELS = 1<<2,
+	ARCHIVER_COMMAND_FROM_CONFIG = 1<<3,
 };
 struct archiver {
 	const char *name;
-- 
2.39.1.1392.g63e6d408230




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux