From: suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp>
To: Junio C Hamano <gitster@pobox.com>, avarab@gmail.com
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: [PATCH] git-archive: accepts --owner --group aslike GNU tar.
Date: Fri, 29 Dec 2017 23:04:29 +0900 [thread overview]
Message-ID: <5A464B6D.1010102@hiroshima-u.ac.jp> (raw)
In-Reply-To: <548d442f05e24e22a1cf4e0074f23f16@OS2PR01MB1147.jpnprd01.prod.outlook.com>
Dear Junio, Ævar
Thank you very much for your reviews, in spite of my many
overlooking of the requirements written in the documents.
To classify various cases, I modified my patch heavily.
It would be posted soon.
I found that some Python scripts are included in the git
repository, but nothing in t/ directories. A helper written
in Python is not welcomed? If so, I would rewrite the
helper in C (perl's standard library does not have a parse
of tarfile). So, please do not review parse-tar-file.py.
Regards,
mpsuzuki
Junio C Hamano wrote:
> suzuki toshiya <mpsuzuki@hiroshima-u.ac.jp> 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?
>
prev parent reply other threads:[~2017-12-29 14:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-28 9:05 [PATCH] git-archive: accepts --owner --group aslike GNU tar suzuki toshiya
2017-12-28 14:42 ` Ævar Arnfjörð Bjarmason
2017-12-28 19:23 ` Junio C Hamano
[not found] ` <548d442f05e24e22a1cf4e0074f23f16@OS2PR01MB1147.jpnprd01.prod.outlook.com>
2017-12-29 14:04 ` suzuki toshiya [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5A464B6D.1010102@hiroshima-u.ac.jp \
--to=mpsuzuki@hiroshima-u.ac.jp \
--cc=avarab@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).