[PATCH 4/9] archive: omit the shell for built-in "command" filters
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Feb 2, 2023, 09:32 UTC
- Message-ID
- <patch-4.9-8bc1bfd1fe2-20230202T093212Z-avarab@gmail.com>
- In-Reply-To
- <cover-0.9-00000000000-20230202T093212Z-avarab@gmail.com>
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@gmail.com> --- 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