Re: [PATCH 2/7] archive: add user-configurable tar-filter infrastructure

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> Archive supports two output formats: tar and zip. The tar
> ...
> +static struct tar_filter *tar_filter_by_namelen(const char *name,
> +						int len)
> +{
> +	struct tar_filter *p;
> +	for (p = tar_filters; p; p = p->next)
> +		if (!strncmp(p->name, name, len) && !p->name[len])
> +			return p;
> +	return NULL;
> +}

Makes me wonder if we want to have a generic table that is keyed by name
whose contents can be looked up by counted string. string_list is the
closest thing we already have, but I do not think it has counted string
interface (shouldn't be a rocket surgery to add it, though).

> +static int tar_filter_config(const char *var, const char *value, void *data)
> +{
> ...
> +	if (!strcmp(type, "command")) {
> +		if (!value)
> +			return config_error_nonbool(var);
> +		tf->command = xstrdup(value);

Does this result in small leak if the same filter is multiply defined, say
in /etc/gitconfig and then in ~/.gitconfig?

> diff --git a/archive.h b/archive.h
> index 038ac35..8386c46 100644
> --- a/archive.h
> +++ b/archive.h
> @@ -1,6 +1,8 @@
>  #ifndef ARCHIVE_H
>  #define ARCHIVE_H
>  
> +#include "string-list.h"
> +
>  struct archiver_args {
>  	const char *base;
>  	size_t baselen;
> @@ -27,4 +29,17 @@ extern int write_zip_archive(struct archiver_args *);
>  extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);
>  extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);
>  
> +struct tar_filter {
> +	char *name;
> +	char *command;
> +	struct string_list extensions;
> +	unsigned use_compression:1;

I suspect that you plan to pass sprintf("-%d", level) for the ones marked
with this bit, but I wonder if we want to give a bit more control on how a
compression level option is shaped for the particular command, and where
on the command line the option comes.  As long as we are targetting gzip
and nothing else it is fine, and I suspect newer compression commands
would try to mimic the -[0-9] command line interface gzip has (e.g. xz),
so this probably is not an issue in practice.