Re: [PATCH] git-archive: accept --owner and --group like GNU tar
- From
René Scharfe <l.s.r@web.de>
- Date
- Jan 1, 2018, 18:29 UTC
- Message-ID
- <81b882a5-0c35-f3c4-78e2-d3e36290fec1@web.de>
- In-Reply-To
- <20171229140535.10746-1-mpsuzuki@hiroshima-u.ac.jp>
Am 29.12.2017 um 15:05 schrieb suzuki toshiya:
> The ownership of files created by git-archive is always > root:root. Add --owner and --group options which work > like the GNU tar equivalent to allow overriding these > defaults.
In which situations do you use the new options?
(The sender would need to know the names and/or IDs on the receiving end. And the receiver would need to be root to set both IDs, or be a group member to set the group ID; I guess the latter is more common.)
Show 12 quoted lines
> Signed-off-by: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp> > --- > Documentation/git-archive.txt | 13 +++ > archive-tar.c | 8 +- > archive.c | 224 ++++++++++++++++++++++++++++++++++++++++++ > archive.h | 4 + > t/t5005-archive-uid-gid.sh | 140 ++++++++++++++++++++++++++ > t/t5005/parse-tar-file.py | 60 +++++++++++ > tar.h | 2 + > 7 files changed, 447 insertions(+), 4 deletions(-) > create mode 100755 t/t5005-archive-uid-gid.sh > create mode 100755 t/t5005/parse-tar-file.py
Would it make sense to support the new options for ZIP files as well?
Show 28 quoted lines
> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt > index cfa1e4ebe..0d156f6c1 100644 > --- a/Documentation/git-archive.txt > +++ b/Documentation/git-archive.txt > @@ -11,6 +11,7 @@ SYNOPSIS > [verse] > 'git archive' [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>] > [-o <file> | --output=<file>] [--worktree-attributes] > + [--owner [username[:uid]] [--group [groupname[:gid]] > [--remote=<repo> [--exec=<git-upload-archive>]] <tree-ish> > [<path>...] > > @@ -63,6 +64,18 @@ OPTIONS > This can be any options that the archiver backend understands. > See next section. > > +--owner=<name>[:<uid>]:: > + Force <name> as owner and <uid> as uid for the files in the tar > + archive. If <uid> is not supplied, <name> can be either a user > + name or numeric UID. In this case the missing part (UID or > + name) will be inferred from the current host's user database. > + > +--group=<name>[:<gid>]:: > + Force <name> as group and <gid> as gid for the files in the tar > + archive. If <gid> is not supplied, <name> can be either a group > + name or numeric GID. In this case the missing part (GID or > + name) will be inferred from the current host's group database. > +
IIUC the default behavior is kept, i.e. without these options the archive entries appear to be owned by root:root. I think it's a good idea to mention this here.
bsdtar has --uname, --uid, --gname, and -gid, which seem simpler. At least you could use OPT_STRING and OPT_INTEGER with them (plus a range check). And they should be easier to explain.
Show 49 quoted lines
> diff --git a/archive.c b/archive.c
> index 0b7b62af0..aa4b16b75 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -8,6 +8,7 @@
> #include "parse-options.h"
> #include "unpack-trees.h"
> #include "dir.h"
> +#include "tar.h"
>
> static char const * const archive_usage[] = {
> N_("git archive [<options>] <tree-ish> [<path>...]"),
> @@ -417,6 +418,223 @@ static void parse_treeish_arg(const char **argv,
> { OPTION_SET_INT, (s), NULL, (v), NULL, "", \
> PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }
>
> +/*
> + * GNU tar --owner, --group options reject hexdigit, signed int values.
> + * strtol(), atoi() are too permissive to simulate the behaviour.
> + */
> +#define STR_IS_DIGIT_OK 0
> +#define STR_IS_NOT_DIGIT -1
> +#define STR_IS_DIGIT_TOO_LARGE -2
> +
> +static int try_as_simple_digit(const char *s, unsigned long *dst)
> +{
> + unsigned long ul;
> + char *endptr;
> +
> + if (strlen(s) != strspn(s, "0123456789"))
> + return STR_IS_NOT_DIGIT;
> +
> + errno = 0;
> + ul = strtoul(s, &endptr, 10);
> +
> + /* catch ERANGE */
> + if (errno) {
> + errno = 0;
> + return STR_IS_DIGIT_TOO_LARGE;
> + }
> +
> +#if ULONG_MAX > 0xFFFFFFFFUL
> + /*
> + * --owner, --group rejects uid/gid greater than 32-bit
> + * limits, even on 64-bit platforms.
> + */
> + if (ul > 0xFFFFFFFFUL)
> + return STR_IS_DIGIT_TOO_LARGE;
> +#endifThe #if is not really necessary, is it? Compilers should be able to optimize the conditional out on 32-bit platforms.
Show 45 quoted lines
> +static int set_args_uname_uid(struct archiver_args *args,
> + const char *tar_owner)
> +{
> + int r;
> + struct passwd *pw = NULL;
> +
> + if (!args || !tar_owner)
> + return NAME_ID_ERR_PARAMS;
> +
> + r = try_as_name_colon_digit(tar_owner, &(args->uname),
> + &(args->uid));
> + switch (r) {
> + case STR_IS_NAME_COLON_DIGIT:
> + return NAME_ID_BOTH_GIVEN;
> + case STR_HAS_DIGIT_TOO_LARGE:
> + return NAME_ID_ERR_ID_TOO_LARGE;
> + case STR_HAS_DIGIT_BROKEN:
> + return NAME_ID_ERR_SYNTAX;
> + }
> +
> + /* the operand is known to be single token */
> +
> + r = try_as_simple_digit(tar_owner, &(args->uid));
> + switch (r) {
> + case STR_IS_DIGIT_TOO_LARGE:
> + return NAME_ID_ERR_ID_TOO_LARGE;
> + case STR_IS_DIGIT_OK:
> + pw = getpwuid(args->uid);
> + if (!pw) {
> + args->uname = xstrdup("");
> + return NAME_ID_NAME_EMPTY;
> + }
> + args->uname = xstrdup(pw->pw_name);
> + return NAME_ID_NAME_GUESSED;
> + }
> +
> + /* the operand is known to be non-digit */
> +
> + args->uname = xstrdup(tar_owner);
> + pw = getpwnam(tar_owner);
> + if (!pw)
> + return NAME_ID_ID_UNTOUCHED;
> + args->uid = pw->pw_uid;
> + return NAME_ID_ID_GUESSED;
> +}How well does this work on Windows? In daemon.c we avoid calling getpwnam(3), getgrnam(3) etc. if NO_POSIX_GOODIES is not defined.
Show 58 quoted lines
> diff --git a/t/t5005-archive-uid-gid.sh b/t/t5005-archive-uid-gid.sh
> new file mode 100755
> index 000000000..c5e08d890
> --- /dev/null
> +++ b/t/t5005-archive-uid-gid.sh
> @@ -0,0 +1,140 @@
> +#!/bin/sh
> +
> +test_description='test --owner --group options for git-archive'
> +. ./test-lib.sh
> +
> +test_expect_success 'create commit with a few empty files' '
> + git init . 1>/dev/null 2>/dev/null &&
> + touch uid-gid-test.001 &&
> + mkdir uid-gid-test.002 &&
> + mkdir uid-gid-test.002/uid-gid-test.003 &&
> + git add uid-gid-test.001 &&
> + git add uid-gid-test.002 &&
> + git add uid-gid-test.002/uid-gid-test.003 &&
> + git commit -m "uid-gid-test" 2>/dev/null 1>/dev/null
> +'
> +
> +check_uid_gid_uname_gname_in_tar() {
> + # $1 tar pathname
> + # $2 uid (digit in string)
> + # $3 gid (digit in string)
> + # $4 uname (string)
> + # $5 gname (string)
> + uid=`python "$TEST_DIRECTORY"/t5005/parse-tar-file.py --print=uid --fail-if-multi $1`
> + if test $? != 0 -o x"${uid}" != "x"$2
> + then
> + echo "(some) uid differs from the specified value"
> + return $?
> + fi
> +
> + gid=`python "$TEST_DIRECTORY"/t5005/parse-tar-file.py --print=gid --fail-if-multi $1`
> + if test $? != 0 -o x"${gid}" != "x"$3
> + then
> + echo "(some) gid differs from the specified value"
> + return $?
> + fi
> +
> + uname=`python "$TEST_DIRECTORY"/t5005/parse-tar-file.py --print=uname --fail-if-multi $1`
> + if test $? != 0 -o x"${uname}" != "x"$4
> + then
> + echo "(some) uname differs from the specified value"
> + return $?
> + fi
> +
> + gname=`python "$TEST_DIRECTORY"/t5005/parse-tar-file.py --print=gname --fail-if-multi $1`
> + if test $? != 0 -o x"${gname}" != "x"$5
> + then
> + echo "(some) gname differs from the specified value"
> + return $?
> + fi
> +
> + return 0
> +}GNU tar and bsdtar show the names of owner and group with -t -v at least, albeit in slightly different formats. Can this help avoid parsing the archive on our own?
But getting a short program like zipdetails for tar would be nice as well of course. :)
René