suzuki toshiya <mpsuzuki@xxxxxxxxxxxxxxxxx> writes: > Current tar output by git-archive has always root:root. Thanks for a patch. On top of Ævar's comments... > ... > * t/t5005-archive-uid-gid.sh: a test script comparing > uid, gid, uname, gname between the options and > generated tar file. > --- Before the "---" line, we need to get the patch signed off by the author (see Documentation/SubmittingPatches). > diff --git a/archive-tar.c b/archive-tar.c > index c6ed96ee7..8546a6229 100644 > --- a/archive-tar.c > +++ b/archive-tar.c > @@ -204,10 +204,10 @@ static void prepare_header(struct archiver_args *args, > xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ? size : 0); > xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", (unsigned long) args->time); > > - xsnprintf(header->uid, sizeof(header->uid), "%07o", 0); > - xsnprintf(header->gid, sizeof(header->gid), "%07o", 0); > - strlcpy(header->uname, "root", sizeof(header->uname)); > - strlcpy(header->gname, "root", sizeof(header->gname)); > + xsnprintf(header->uid, sizeof(header->uid), "%07o", args->uid); > + xsnprintf(header->gid, sizeof(header->gid), "%07o", args->gid); > + strlcpy(header->uname, args->uname ? args->uname : "root", sizeof(header->uname)); > + strlcpy(header->gname, args->gname ? args->gname : "root", sizeof(header->gname)); Would it be cleaner to make sure aregs->[gu]name is always set (i.e. stuff "root" when it is not given)? > xsnprintf(header->devmajor, sizeof(header->devmajor), "%07o", 0); > xsnprintf(header->devminor, sizeof(header->devminor), "%07o", 0); > > diff --git a/archive.c b/archive.c > index 0b7b62af0..db69041f1 100644 > --- a/archive.c > +++ b/archive.c > @@ -8,6 +8,7 @@ > #include "parse-options.h" > #include "unpack-trees.h" > #include "dir.h" > +#include "git-compat-util.h" The coding guideline says that "git-compat-util.h" (or one of the well-known header that includes it) should be the first file to be included, and we already include "cache.h" as the first thing, so I do not think you want this addition here. > @@ -417,6 +418,57 @@ 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) } > > +static void set_args_uname_uid(struct archiver_args *args, > + const char* tar_owner, int set_gid_too) The asterisk sticks to the variable name, not the type name, i.e. const char *tar_owner > +{ > + if (!args || !tar_owner) > + return; > + > + const char* col_pos = strchr(tar_owner, ':'); > + struct passwd* pw = NULL; Decl after statement. > + if (col_pos) { > + args->uname = xstrndup(tar_owner, col_pos - tar_owner); > + args->uid = atoi(col_pos + 1); > + return; > + } > + > + args->uname = xstrndup(tar_owner, strlen(tar_owner)); > + pw = getpwnam(tar_owner); > + if (!pw) > + return; This means that upon error, the caller gets a half-filled args structure and without any indication. > diff --git a/t/parse-tar-file.py b/t/parse-tar-file.py Hmph. Do we still use Python around here?