{"thread":{"id":"27069","subject":"git archive: tar.umask ignored","startedAt":"2011-04-12T09:39:41Z","lastAt":"2011-04-14T22:57:34Z","messageCount":6,"participants":["Jacek Masiulaniec","René Scharfe","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"165680","messageId":"BANLkTimTs+Qh9fNs6pLcZidYy-YjXNR2iw@mail.gmail.com","threadId":"27069","inReplyTo":null,"subject":"git archive: tar.umask ignored","fromName":"Jacek Masiulaniec","fromEmail":"jacekm@dobremiasto.net","sentAt":"2011-04-12T09:39:41Z","receivedAt":"2011-04-12T09:39:41Z","isPatch":false,"sender":{"key":"jacekm@dobremiasto.net","avatar":null},"body":"Hello git@,\n\nI believe git has an issue with command line override of configuration options.\nNotice how in both cases below the file mode is 775 despite tar.umask\ndifference:\n\n[jacekm@localhost git]$ ./git --version\ngit version 1.7.4.4\n[jacekm@localhost git]$ ./git archive --remote=/repo/website.git HEAD\n| tar -tvf - | head -1\ndrwxrwxr-x root/root 0 2011-04-11 13:47:09 bin/\n[jacekm@localhost git]$ ./git -c tar.umask=0022 archive\n--remote=/repo/website.git HEAD | tar -tvf - | head -1\ndrwxrwxr-x root/root 0 2011-04-11 13:47:09 bin/\n[jacekm@localhost git]$\n\nIn contrast, tweaking tar.umask in ~/.gitconfig does have the desired\nimpact on the file mode.\n\nJacek\n"},{"id":"165701","messageId":"4DA497B7.706@lsrfire.ath.cx","threadId":"27069","inReplyTo":"BANLkTimTs+Qh9fNs6pLcZidYy-YjXNR2iw@mail.gmail.com","subject":"Re: git archive: tar.umask ignored","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-04-12T18:19:35Z","receivedAt":"2011-04-12T18:19:35Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 12.04.2011 11:39, schrieb Jacek Masiulaniec:\n> Hello git@,\n>\n> I believe git has an issue with command line override of configuration options.\n> Notice how in both cases below the file mode is 775 despite tar.umask\n> difference:\n>\n> [jacekm@localhost git]$ ./git --version\n> git version 1.7.4.4\n> [jacekm@localhost git]$ ./git archive --remote=/repo/website.git HEAD\n> | tar -tvf - | head -1\n> drwxrwxr-x root/root 0 2011-04-11 13:47:09 bin/\n> [jacekm@localhost git]$ ./git -c tar.umask=0022 archive\n> --remote=/repo/website.git HEAD | tar -tvf - | head -1\n> drwxrwxr-x root/root 0 2011-04-11 13:47:09 bin/\n> [jacekm@localhost git]$\n>\n> In contrast, tweaking tar.umask in ~/.gitconfig does have the desired\n> impact on the file mode.\n\nThe local setting of tar.umask does not affect the archive created at \nthe remote end. If your \"remote\" repository is in fact located on the \nsame machine and accessed with the same user then of course settings in \n~/.gitconfig will take effect.\n\nIf --remote is specified then the local archive process just passes all \ncommand line options (except --remote itself, --output and --exec) to \nthe other side which then does all the actual work. The config option \ntar.umask is not even read in that case.\n\nWe'd need to read all relevant config settings (from the command line, \nand perhaps the user's config file and the system-wide one, but not the \nones from any local repository) and then pass them over somehow. We \ncould piggy-back on the existing command line option passing, but for \nthat we'd need to add new command line options. OK, just a single one, \nbecause archive only recognizes tar.umask currently.\n\nWould a new --umask command line option alone (that overrides any config \nsetting) be sufficient for your use case?\n\nThanks,\nRené\n"},{"id":"165709","messageId":"4DA4AFDB.7040606@lsrfire.ath.cx","threadId":"27069","inReplyTo":"4DA497B7.706@lsrfire.ath.cx","subject":"Re: git archive: tar.umask ignored","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-04-12T20:02:35Z","receivedAt":"2011-04-12T20:02:35Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 12.04.2011 20:19, schrieb René Scharfe:\n> Would a new --umask command line option alone (that overrides any config \n> setting) be sufficient for your use case?\n\nSomething like this (docs and tests missing)?\n\nI'm not so sure about PARSE_OPT_OPTARG. E.g. this is confusing and even\nbecomes slightly dangerous if you have a branch named 022 with a file\nnamed HEAD (the = is required):\n\n\t$ git archive --umask 022 HEAD\n\tfatal: Not a valid object name\n\nAlso I'm and not sure what --no-umask should do. In this patch it is\nequivalent to --umask=0; perhaps it should rather make archive use the\nconfigured tar.umask value, i.e. neutralize any previous --umask\noption.\n---\n archive-tar.c | 5 ++++-\n archive.c | 34 +++++++++++++++++++++++++++++++++-\n archive.h | 1 +\n 3 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex cee06ce..1a29f3c 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -238,7 +238,10 @@ int write_tar_archive(struct archiver_args *args)\n {\n \tint err = 0;\n \n-\tgit_config(git_tar_config, NULL);\n+\tif (args->umask == -1)\n+\t\tgit_config(git_tar_config, NULL);\n+\telse\n+\t\ttar_umask = args->umask;\n \n \tif (args->commit_sha1)\n \t\terr = write_global_extended_header(args);\ndiff --git a/archive.c b/archive.c\nindex 1944ed4..2ec5391 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -15,13 +15,14 @@ static char const * const archive_usage[] = {\n };\n \n #define USES_ZLIB_COMPRESSION 1\n+#define USES_UMASK 2\n \n static const struct archiver {\n \tconst char *name;\n \twrite_archive_fn_t write_archive;\n \tunsigned int flags;\n } archivers[] = {\n-\t{ \"tar\", write_tar_archive },\n+\t{ \"tar\", write_tar_archive, USES_UMASK },\n \t{ \"zip\", write_zip_archive, USES_ZLIB_COMPRESSION },\n };\n \n@@ -285,6 +286,30 @@ static void parse_treeish_arg(const char **argv,\n \tar_args->time = archive_time;\n }\n \n+static int umask_callback(const struct option *opt, const char *arg, int unset)\n+{\n+\tint *target = opt->value;\n+\tint value;\n+\tconst char *endp;\n+\n+\tif (unset) {\n+\t\t*target = 0;\n+\t\treturn 0;\n+\t}\n+\tif (arg) {\n+\t\tvalue = strtol(arg, (char **)&endp, 0);\n+\t\tif (*endp || value < 0) {\n+\t\t\treturn error(\"option `%s' %s\", \"umask\",\n+\t\t\t\t \"expects a non-negative numerical value\");\n+\t\t}\n+\t} else {\n+\t\tvalue = umask(0);\n+\t\tumask(value);\n+\t}\n+\t*target = value;\n+\treturn 0;\n+}\n+\n #define OPT__COMPR(s, v, h, p) \\\n \t{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \\\n \t PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }\n@@ -305,6 +330,7 @@ static int parse_archive_args(int argc, const char **argv,\n \tint i;\n \tint list = 0;\n \tint worktree_attributes = 0;\n+\tint umask = -1;\n \tstruct option opts[] = {\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_STRING(0, \"format\", &format, \"fmt\", \"archive format\"),\n@@ -325,6 +351,9 @@ static int parse_archive_args(int argc, const char **argv,\n \t\tOPT__COMPR_HIDDEN('7', &compression_level, 7),\n \t\tOPT__COMPR_HIDDEN('8', &compression_level, 8),\n \t\tOPT__COMPR('9', &compression_level, \"compress better\", 9),\n+\t\t{ OPTION_CALLBACK, 0, \"umask\", &umask, \"umask\",\n+\t\t\t\"apply user's umask or to archived files\",\n+\t\t\tPARSE_OPT_OPTARG, umask_callback },\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOLEAN('l', \"list\", &list,\n \t\t\t\"list supported archive formats\"),\n@@ -370,10 +399,13 @@ static int parse_archive_args(int argc, const char **argv,\n \t\t\t\t\tformat, compression_level);\n \t\t}\n \t}\n+\tif (umask != -1 && !((*ar)->flags & USES_UMASK))\n+\t\tdie(\"Argument not supported for format '%s': --umask\", format);\n \targs->verbose = verbose;\n \targs->base = base;\n \targs->baselen = strlen(base);\n \targs->worktree_attributes = worktree_attributes;\n+\targs->umask = umask;\n \n \treturn argc;\n }\ndiff --git a/archive.h b/archive.h\nindex 038ac35..083675c 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -12,6 +12,7 @@ struct archiver_args {\n \tunsigned int verbose : 1;\n \tunsigned int worktree_attributes : 1;\n \tint compression_level;\n+\tint umask;\n };\n \n typedef int (*write_archive_fn_t)(struct archiver_args *);\n-- \n1.7.4\n"},{"id":"165710","messageId":"BANLkTinEFPZoaUAc+8F1sPGE636u1vDOZw@mail.gmail.com","threadId":"27069","inReplyTo":"4DA497B7.706@lsrfire.ath.cx","subject":"Re: git archive: tar.umask ignored","fromName":"Jacek Masiulaniec","fromEmail":"jacek.masiulaniec@gmail.com","sentAt":"2011-04-12T20:39:48Z","receivedAt":"2011-04-12T20:39:48Z","isPatch":false,"sender":{"key":"jacek.masiulaniec@gmail.com","avatar":null},"body":"On 12 April 2011 19:19, René Scharfe wrote:\n> The local setting of tar.umask does not affect the archive created at the\n> remote end.  If your \"remote\" repository is in fact located on the same\n> machine and accessed with the same user then of course settings in\n> ~/.gitconfig will take effect.\n\nI've eventually realised this by reading the source code. The command\nI actually needed was:\n\n cd /repo/website.git && git -c tar.umask=0022 archive HEAD\n\nCan't comment on the proposed diff without getting better handle on\nthe whole system, but my knee-jerk reaction would be to resist the\nurge to add more switches. Maybe the existing -c switch could be made\nto cooperate with --remote, or maybe the documentation could be edited\nto make it clearer that -c and --remote are not related in the way I\nhad originally assumed.\n\nJacek\n"},{"id":"165823","messageId":"4DA73749.40303@lsrfire.ath.cx","threadId":"27069","inReplyTo":"BANLkTinEFPZoaUAc+8F1sPGE636u1vDOZw@mail.gmail.com","subject":"Re: git archive: tar.umask ignored","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-04-14T18:04:57Z","receivedAt":"2011-04-14T18:04:57Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 12.04.2011 22:39, schrieb Jacek Masiulaniec:\n> On 12 April 2011 19:19, René Scharfe wrote:\n>> The local setting of tar.umask does not affect the archive created at the\n>> remote end. If your \"remote\" repository is in fact located on the same\n>> machine and accessed with the same user then of course settings in\n>> ~/.gitconfig will take effect.\n> \n> I've eventually realised this by reading the source code. The command\n> I actually needed was:\n> \n> cd /repo/website.git&& git -c tar.umask=0022 archive HEAD\n\nYes, there's no point in using --remote if you can read the files directly.\nI assumed you did that to make the command easier to reproduce by avoiding\nto set up a git daemon etc..\n\n> Can't comment on the proposed diff without getting better handle on\n> the whole system, but my knee-jerk reaction would be to resist the\n> urge to add more switches. Maybe the existing -c switch could be made\n> to cooperate with --remote, or maybe the documentation could be edited\n> to make it clearer that -c and --remote are not related in the way I\n> had originally assumed.\n\nYes, the documentation should be updated first.\n\n-- >8 --\nSubject: archive: document limitation of tar.umask config setting\n\nThe local value of the config variable tar.umask is not passed to the\nother side with --remote. We may want to change that, but for now just\ndocument this fact.\n\nReported-by: Jacek Masiulaniec \nSigned-off-by: Rene Scharfe \n---\n Documentation/git-archive.txt | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex f2b8684..9c750e2 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -98,7 +98,8 @@ tar.umask::\n \ttar archive entries. The default is 0002, which turns off the\n \tworld write bit. The special value \"user\" indicates that the\n \tarchiving user's umask will be used instead. See umask(2) for\n-\tdetails.\n+\tdetails. If `--remote` is used then only the configuration of\n+\tthe remote repository takes effect.\n \n ATTRIBUTES\n ----------\n"},{"id":"165863","messageId":"7vhba0fnkx.fsf@alter.siamese.dyndns.org","threadId":"27069","inReplyTo":"4DA73749.40303@lsrfire.ath.cx","subject":"Re: git archive: tar.umask ignored","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-14T22:57:34Z","receivedAt":"2011-04-14T22:57:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Yes, the documentation should be updated first.\n\nThanks; applied.\n"}]}