{"thread":{"id":"27625","subject":"[PATCH 1/2] archive: factor out write phase of tar format","startedAt":"2011-06-14T18:17:33Z","lastAt":"2011-06-23T17:30:13Z","messageCount":56,"participants":["Jeff King","J.H.","René Scharfe","Junio C Hamano","Miles Bader","Chris Webb","John Szakmeister","Jakub Narebski","Thiago Farina"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"170018","messageId":"20110614181732.GA31635@sigill.intra.peff.net","threadId":"27625","inReplyTo":null,"subject":"[PATCH 1/2] archive: factor out write phase of tar format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-14T18:17:33Z","receivedAt":"2011-06-14T18:17:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The code to output the tar format for git-archive always\nassumes we are writing directly to stdout. Let's factor out\nthat bit of code so that we can put an in-process gzip\nfilter in place.\n\nSigned-off-by: Jeff King \n---\n archive-tar.c | 22 +++++++++++++++++-----\n 1 files changed, 17 insertions(+), 5 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex cee06ce..b1aea87 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -10,6 +10,7 @@\n \n static char block[BLOCKSIZE];\n static unsigned long offset;\n+static void (*output)(const char *buf, unsigned long size);\n \n static int tar_umask = 002;\n \n@@ -17,7 +18,7 @@ static int tar_umask = 002;\n static void write_if_needed(void)\n {\n \tif (offset == BLOCKSIZE) {\n-\t\twrite_or_die(1, block, BLOCKSIZE);\n+\t\toutput(block, BLOCKSIZE);\n \t\toffset = 0;\n \t}\n }\n@@ -42,7 +43,7 @@ static void write_blocked(const void *data, unsigned long size)\n \t\twrite_if_needed();\n \t}\n \twhile (size >= BLOCKSIZE) {\n-\t\twrite_or_die(1, buf, BLOCKSIZE);\n+\t\toutput(buf, BLOCKSIZE);\n \t\tsize -= BLOCKSIZE;\n \t\tbuf += BLOCKSIZE;\n \t}\n@@ -66,10 +67,10 @@ static void write_trailer(void)\n {\n \tint tail = BLOCKSIZE - offset;\n \tmemset(block + offset, 0, tail);\n-\twrite_or_die(1, block, BLOCKSIZE);\n+\toutput(block, BLOCKSIZE);\n \tif (tail < 2 * RECORDSIZE) {\n \t\tmemset(block, 0, offset);\n-\t\twrite_or_die(1, block, BLOCKSIZE);\n+\t\toutput(block, BLOCKSIZE);\n \t}\n }\n \n@@ -234,7 +235,7 @@ static int git_tar_config(const char *var, const char *value, void *cb)\n \treturn git_default_config(var, value, cb);\n }\n \n-int write_tar_archive(struct archiver_args *args)\n+static int write_tar_archive_internal(struct archiver_args *args)\n {\n \tint err = 0;\n \n@@ -248,3 +249,14 @@ int write_tar_archive(struct archiver_args *args)\n \t\twrite_trailer();\n \treturn err;\n }\n+\n+static void output_write(const char *buf, unsigned long len)\n+{\n+\twrite_or_die(1, buf, len);\n+}\n+\n+int write_tar_archive(struct archiver_args *args)\n+{\n+\toutput = output_write;\n+\treturn write_tar_archive_internal(args);\n+}\n-- \n1.7.6.rc1.37.g6d4ed.dirty\n"},{"id":"170019","messageId":"20110614181821.GA32685@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110614181732.GA31635@sigill.intra.peff.net","subject":"[PATCH 2/2] archive: support gzipped tar files","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-14T18:18:21Z","receivedAt":"2011-06-14T18:18:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"git-archive already supports the creation of tar files. For\nlocal cases, one can simply pipe the output to gzip, and\nhaving git-archive do the gzip is a minor convenience.\n\nHowever, when running git-archive against a remote site,\nhaving the remote side do the compression can save\nconsiderable bandwidth. Service providers could always wrap\ngit-archive to provide that functionality, but this makes it\nmuch simpler.\n\nCreating gzipped archives is of course more expensive than\nregular tar archives; however, the amount of work should be\ncomparable to that of creating a zip file, which is already\npossible. So there should be no new security implications\nwith respect to creating load on a remote server.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-archive.txt | 17 +++++++++++++++--\n archive-tar.c | 27 +++++++++++++++++++++++++++\n archive.c | 1 +\n archive.h | 1 +\n builtin/archive.c | 6 ++++++\n t/t5000-tar-tree.sh | 26 ++++++++++++++++++++++++++\n 6 files changed, 76 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 9c750e2..963bec4 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -34,10 +34,11 @@ OPTIONS\n -------\n \n --format=::\n-\tFormat of the resulting archive: 'tar' or 'zip'. If this option\n+\tFormat of the resulting archive: 'tar', 'tgz', or 'zip'. If this option\n \tis not given, and the output file is specified, the format is\n \tinferred from the filename if possible (e.g. writing to \"foo.zip\"\n-\tmakes the output to be in the zip format). Otherwise the output\n+\tcreates the output in the zip format; \"foo.tgz\" or \"foo.tar.gz\"\n+\tcreates the output in the tgz format). Otherwise the output\n \tformat is `tar`.\n \n -l::\n@@ -89,6 +90,12 @@ zip\n \tHighest and slowest compression level. You can specify any\n \tnumber from 1 to 9 to adjust compression speed and ratio.\n \n+tgz\n+~~~\n+-9::\n+\tHighest and slowest compression level. You can specify any\n+\tnumber from 1 to 9 to adjust compression speed and ratio.\n+\n \n CONFIGURATION\n -------------\n@@ -133,6 +140,12 @@ git archive --format=tar --prefix=git-1.4.0/ v1.4.0 | gzip >git-1.4.0.tar.gz::\n \n \tCreate a compressed tarball for v1.4.0 release.\n \n+git archive --prefix=git-1.4.0/ -o git-1.4.0.tar.gz v1.4.0\n+\n+\tSame as above, except that we use the internal gzip. Note that\n+\tthe output format is inferred by the extension of the output\n+\tfile.\n+\n git archive --format=tar --prefix=git-1.4.0/ v1.4.0{caret}\\{tree\\} | gzip >git-1.4.0.tar.gz::\n \n \tCreate a compressed tarball for v1.4.0 release, but without a\ndiff --git a/archive-tar.c b/archive-tar.c\nindex b1aea87..86c8aa9 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -260,3 +260,30 @@ int write_tar_archive(struct archiver_args *args)\n \toutput = output_write;\n \treturn write_tar_archive_internal(args);\n }\n+\n+static gzFile gz_file;\n+static void output_gz(const char *buf, unsigned long len)\n+{\n+\tif (!gzwrite(gz_file, buf, len))\n+\t\tdie(\"unable to write compressed stream: %s\",\n+\t\t gzerror(gz_file, NULL));\n+}\n+\n+int write_tgz_archive(struct archiver_args *args)\n+{\n+\tint r;\n+\n+\tgz_file = gzdopen(1, \"w\");\n+\tif (!gz_file)\n+\t\tdie_errno(\"unable to open compressed stream\");\n+\tgzsetparams(gz_file, args->compression_level, Z_DEFAULT_STRATEGY);\n+\n+\toutput = output_gz;\n+\tr = write_tar_archive_internal(args);\n+\tif (r == 0) {\n+\t\tint zerr = gzclose(gz_file);\n+\t\tif (zerr != Z_OK)\n+\t\t\tdie(\"unable to write compressed stream (err=%d)\", zerr);\n+\t}\n+\treturn r;\n+}\ndiff --git a/archive.c b/archive.c\nindex 42f2d2f..6073a8d 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -23,6 +23,7 @@ static const struct archiver {\n } archivers[] = {\n \t{ \"tar\", write_tar_archive },\n \t{ \"zip\", write_zip_archive, USES_ZLIB_COMPRESSION },\n+\t{ \"tgz\", write_tgz_archive, USES_ZLIB_COMPRESSION },\n };\n \n static void format_subst(const struct commit *commit,\ndiff --git a/archive.h b/archive.h\nindex 038ac35..c1bf72e 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -23,6 +23,7 @@ typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsign\n */\n extern int write_tar_archive(struct archiver_args *);\n extern int write_zip_archive(struct archiver_args *);\n+extern int write_tgz_archive(struct archiver_args *);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex b14eaba..4f60af5 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -71,6 +71,12 @@ static const char *format_from_name(const char *filename)\n \text++;\n \tif (!strcasecmp(ext, \"zip\"))\n \t\treturn \"--format=zip\";\n+\tif (!strcasecmp(ext, \"tgz\"))\n+\t\treturn \"--format=tgz\";\n+\tif (!strcasecmp(ext, \"gz\") &&\n+\t ext - 4 >= filename &&\n+\t !strcasecmp(ext - 4, \"tar.gz\"))\n+\t\treturn \"--format=tgz\";\n \treturn NULL;\n }\n \ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex cff1b3e..faf2784 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -26,6 +26,7 @@ commit id embedding:\n \n . ./test-lib.sh\n UNZIP=${UNZIP:-unzip}\n+GUNZIP=${GUNZIP:-gunzip}\n \n SUBSTFORMAT=%H%n\n \n@@ -252,4 +253,29 @@ test_expect_success 'git-archive --prefix=olde-' '\n \ttest -f h/olde-a/bin/sh\n '\n \n+test_expect_success 'git archive --format=tgz' '\n+\tgit archive --format=tgz HEAD >e.tgz\n+'\n+\n+test_expect_success 'infer tgz from .tgz filename' '\n+\tgit archive --output=e1.tgz HEAD &&\n+\ttest_cmp e.tgz e1.tgz\n+'\n+\n+test_expect_success 'infer tgz from .tar.gz filename' '\n+\tgit archive --output=e2.tar.gz HEAD &&\n+\ttest_cmp e.tgz e2.tar.gz\n+'\n+\n+if $GUNZIP --version >/dev/null 2>&1; then\n+\ttest_set_prereq GUNZIP\n+else\n+\tsay \"Skipping tgz tests because gunzip was not found\"\n+fi\n+\n+test_expect_success GUNZIP 'extract tgz file' '\n+\tgunzip -c e.tar &&\n+\ttest_cmp b.tar e.tar\n+'\n+\n test_done\n-- \n1.7.6.rc1.37.g6d4ed.dirty\n"},{"id":"170023","messageId":"4DF7B59D.30306@eaglescrag.net","threadId":"27625","inReplyTo":"20110614181821.GA32685@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"J.H.","fromEmail":"warthog19@eaglescrag.net","sentAt":"2011-06-14T19:25:17Z","receivedAt":"2011-06-14T19:25:17Z","isPatch":true,"sender":{"key":"warthog19@eaglescrag.net","avatar":null},"body":"On 06/14/2011 11:18 AM, Jeff King wrote:\n> git-archive already supports the creation of tar files. For\n> local cases, one can simply pipe the output to gzip, and\n> having git-archive do the gzip is a minor convenience.\n> \n> However, when running git-archive against a remote site,\n> having the remote side do the compression can save\n> considerable bandwidth. Service providers could always wrap\n> git-archive to provide that functionality, but this makes it\n> much simpler.\n> \n> Creating gzipped archives is of course more expensive than\n> regular tar archives; however, the amount of work should be\n> comparable to that of creating a zip file, which is already\n> possible. So there should be no new security implications\n> with respect to creating load on a remote server.\n\nWould it make sense to make this a little more generic and support bz2\nand xz as well?\n\n- John 'Warthog9' Hawley\n"},{"id":"170026","messageId":"20110614193033.GA1359@sigill.intra.peff.net","threadId":"27625","inReplyTo":"4DF7B59D.30306@eaglescrag.net","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-14T19:30:33Z","receivedAt":"2011-06-14T19:30:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2011 at 12:25:17PM -0700, J.H. wrote:\n\n> On 06/14/2011 11:18 AM, Jeff King wrote:\n> > git-archive already supports the creation of tar files. For\n> > local cases, one can simply pipe the output to gzip, and\n> > having git-archive do the gzip is a minor convenience.\n> > \n> > However, when running git-archive against a remote site,\n> > having the remote side do the compression can save\n> > considerable bandwidth. Service providers could always wrap\n> > git-archive to provide that functionality, but this makes it\n> > much simpler.\n> > \n> > Creating gzipped archives is of course more expensive than\n> > regular tar archives; however, the amount of work should be\n> > comparable to that of creating a zip file, which is already\n> > possible. So there should be no new security implications\n> > with respect to creating load on a remote server.\n> \n> Would it make sense to make this a little more generic and support bz2\n> and xz as well?\n\nI think it's a great idea if somebody wants to do it on top of my patch.\nThey should be able to hook into the tar code just like I did in 2/2.\nBut they will need library support that we don't already have in git.\nDoing gz was easy because we already require zlib.\n\nWe could also support them by piping to an external compressor, which\nwouldn't be too hard (you could do it for gzip, too, but given that we\nhave zlib, this was much simpler). There is a slight hitch with the\n\"--list\" command, though. Should git-archive advertise these formats,\nand if so, how should it know that they are available?\n\n-Peff\n"},{"id":"170027","messageId":"4DF7B90B.9050802@lsrfire.ath.cx","threadId":"27625","inReplyTo":"20110614181821.GA32685@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-06-14T19:39:55Z","receivedAt":"2011-06-14T19:39:55Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 14.06.2011 20:18, schrieb Jeff King:\n> git-archive already supports the creation of tar files. For\n> local cases, one can simply pipe the output to gzip, and\n> having git-archive do the gzip is a minor convenience.\n> \n> However, when running git-archive against a remote site,\n> having the remote side do the compression can save\n> considerable bandwidth. Service providers could always wrap\n> git-archive to provide that functionality, but this makes it\n> much simpler.\n\nThat's a good point and one that was overlooked when this topic came up\nearlier (see http://kerneltrap.org/mailarchive/git/2009/9/10/11507 and\nhttp://kerneltrap.org/mailarchive/git/2009/9/11/11577). That\nimplementation was ... heavier than yours, but it also avoided an\nunnecessary level of buffering. I wonder if it makes a measurable\ndifference, though.\n\n> Creating gzipped archives is of course more expensive than\n> regular tar archives; however, the amount of work should be\n> comparable to that of creating a zip file, which is already\n> possible. So there should be no new security implications\n> with respect to creating load on a remote server.\n> \n> Signed-off-by: Jeff King \n> ---\n> Documentation/git-archive.txt | 17 +++++++++++++++--\n> archive-tar.c | 27 +++++++++++++++++++++++++++\n> archive.c | 1 +\n> archive.h | 1 +\n> builtin/archive.c | 6 ++++++\n> t/t5000-tar-tree.sh | 26 ++++++++++++++++++++++++++\n> 6 files changed, 76 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\n> index 9c750e2..963bec4 100644\n> --- a/Documentation/git-archive.txt\n> +++ b/Documentation/git-archive.txt\n> @@ -34,10 +34,11 @@ OPTIONS\n> -------\n> \n> --format=::\n> -\tFormat of the resulting archive: 'tar' or 'zip'. If this option\n> +\tFormat of the resulting archive: 'tar', 'tgz', or 'zip'. If this option\n> \tis not given, and the output file is specified, the format is\n> \tinferred from the filename if possible (e.g. writing to \"foo.zip\"\n> -\tmakes the output to be in the zip format). Otherwise the output\n> +\tcreates the output in the zip format; \"foo.tgz\" or \"foo.tar.gz\"\n> +\tcreates the output in the tgz format). Otherwise the output\n> \tformat is `tar`.\n> \n> -l::\n> @@ -89,6 +90,12 @@ zip\n> \tHighest and slowest compression level. You can specify any\n> \tnumber from 1 to 9 to adjust compression speed and ratio.\n> \n> +tgz\n> +~~~\n> +-9::\n> +\tHighest and slowest compression level. You can specify any\n> +\tnumber from 1 to 9 to adjust compression speed and ratio.\n> +\n> \n> CONFIGURATION\n> -------------\n> @@ -133,6 +140,12 @@ git archive --format=tar --prefix=git-1.4.0/ v1.4.0 | gzip >git-1.4.0.tar.gz::\n> \n> \tCreate a compressed tarball for v1.4.0 release.\n> \n> +git archive --prefix=git-1.4.0/ -o git-1.4.0.tar.gz v1.4.0\n> +\n> +\tSame as above, except that we use the internal gzip. Note that\n> +\tthe output format is inferred by the extension of the output\n> +\tfile.\n> +\n> git archive --format=tar --prefix=git-1.4.0/ v1.4.0{caret}\\{tree\\} | gzip >git-1.4.0.tar.gz::\n> \n> \tCreate a compressed tarball for v1.4.0 release, but without a\n> diff --git a/archive-tar.c b/archive-tar.c\n> index b1aea87..86c8aa9 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -260,3 +260,30 @@ int write_tar_archive(struct archiver_args *args)\n> \toutput = output_write;\n> \treturn write_tar_archive_internal(args);\n> }\n> +\n> +static gzFile gz_file;\n> +static void output_gz(const char *buf, unsigned long len)\n> +{\n> +\tif (!gzwrite(gz_file, buf, len))\n> +\t\tdie(\"unable to write compressed stream: %s\",\n> +\t\t gzerror(gz_file, NULL));\n> +}\n\nDoes this do the right things when faced with interrupted writes or\ntruncated pipes? I ask because the earlier attempt had a\ngzwrite_or_die() which did that, but I don't know anymore if that is\nstrictly needed. Oh, and bridging the gap between unsigned long and int\nwas certainly another reason for the existence of this function.\n"},{"id":"170030","messageId":"20110614201433.GB1567@sigill.intra.peff.net","threadId":"27625","inReplyTo":"4DF7B90B.9050802@lsrfire.ath.cx","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-14T20:14:33Z","receivedAt":"2011-06-14T20:14:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2011 at 09:39:55PM +0200, René Scharfe wrote:\n\n> > However, when running git-archive against a remote site,\n> > having the remote side do the compression can save\n> > considerable bandwidth. Service providers could always wrap\n> > git-archive to provide that functionality, but this makes it\n> > much simpler.\n> \n> That's a good point and one that was overlooked when this topic came up\n> earlier (see http://kerneltrap.org/mailarchive/git/2009/9/10/11507 and\n> http://kerneltrap.org/mailarchive/git/2009/9/11/11577).\n\nHmph, I should have done my homework better. I totally missed that\nthread.\n\nYeah, I am unsurprised that doing it in a single process is actually\nslower. I do think because of the remote issue that we should provide\nsomething like this. But we could implement it by piping to an external\ngzip. That would make us just slightly less portable, but would give us\nthe multi-processor speedup, or even allow using something like pigz.\n\n> > +static void output_gz(const char *buf, unsigned long len)\n> > +{\n> > +\tif (!gzwrite(gz_file, buf, len))\n> > +\t\tdie(\"unable to write compressed stream: %s\",\n> > +\t\t gzerror(gz_file, NULL));\n> > +}\n> \n> Does this do the right things when faced with interrupted writes or\n> truncated pipes? I ask because the earlier attempt had a\n> gzwrite_or_die() which did that, but I don't know anymore if that is\n> strictly needed.\n\nNo, I blindly assumed that gzwrite was a little bit smart, but looking\nat the zlib code, it really is just propagating whatever it got from\nfwrite. I need to handle both errors and short writes myself. So we do\nneed gzwrite_or_die.\n\n> Oh, and bridging the gap between unsigned long and int\n> was certainly another reason for the existence of this function.\n\nUgh. I correctly saw that it took an unsigned long, but it actually\nreturns the number of bytes written as an int! Nice interface.\n\nAll of this can go away, though, if we switch to an external process.\nIt's tempting.\n\n-Peff\n"},{"id":"170031","messageId":"7vaadkkvew.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"20110614181821.GA32685@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-14T20:30:47Z","receivedAt":"2011-06-14T20:30:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\nI didn't know it was that easy (primarily because I didn't know zlib had a\nready-to-eat interface to do this).\n\n> +\tif (!strcasecmp(ext, \"tgz\"))\n> +\t\treturn \"--format=tgz\";\n> +\tif (!strcasecmp(ext, \"gz\") &&\n> +\t ext - 4 >= filename &&\n> +\t !strcasecmp(ext - 4, \"tar.gz\"))\n\nShouldn't this be\n\n\tif (!strcasecmp(ext, \"gz\") && filename < ext - 5 &&\n !strcasecmp(ext - 5, \".tar.gz\"))\n\nto exclude \"hellotar.gz\" and possibly \".tar.gz\" (\"<=\" vs \"<\")?\n\n> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\n> index cff1b3e..faf2784 100755\n> --- a/t/t5000-tar-tree.sh\n> +++ b/t/t5000-tar-tree.sh\n> @@ -26,6 +26,7 @@ commit id embedding:\n> \n> . ./test-lib.sh\n> UNZIP=${UNZIP:-unzip}\n> +GUNZIP=${GUNZIP:-gunzip}\n\nJust a personal preference but I find myself using \"gzip -d\" more often\nthan \"gunzip\".\n"},{"id":"170032","messageId":"20110614204521.GA12776@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110614201433.GB1567@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-14T20:45:21Z","receivedAt":"2011-06-14T20:45:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2011 at 04:14:33PM -0400, Jeff King wrote:\n\n> Yeah, I am unsurprised that doing it in a single process is actually\n> slower. I do think because of the remote issue that we should provide\n> something like this. But we could implement it by piping to an external\n> gzip. That would make us just slightly less portable, but would give us\n> the multi-processor speedup, or even allow using something like pigz.\n\nSo here's a relatively quick implementation of the pipe idea. It just\nhandles .tar.gz, but it would be trivial to do bz2 or other formats, as\nlong as they can act as a stdio filter.\n\nThe gzip path is not configurable at all. Probably it should read the\npath and arguments from the config file. In fact, we could even allow\narbitrary config like:\n\n [tarfilter \"tgz\"]\n command = gzip -c\n extension = tgz\n extension = tar.gz\n\nwhich also solves the \"don't advertise in --list if you don't have it\ninstalled problem\". At the same time, that is a lot to have to\nconfigure for somebody who is not providing remote service and just\nwants:\n\n git archive -o HEAD foo.tar.gz\n\nto work out of the box.\n\nI think we could probably allow arbitrary config, but provide a few\nsane, common defaults like gzip and bz2 unless the user specifically\nturns them off at build time.\n\n---\n archive-tar.c | 45 +++++++++++++++++++++++++++++++++++++++++++++\n archive.c | 1 +\n archive.h | 1 +\n builtin/archive.c | 6 ++++++\n t/t5000-tar-tree.sh | 26 ++++++++++++++++++++++++++\n 5 files changed, 79 insertions(+), 0 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex cee06ce..a77d605 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -4,6 +4,7 @@\n #include \"cache.h\"\n #include \"tar.h\"\n #include \"archive.h\"\n+#include \"run-command.h\"\n \n #define RECORDSIZE\t(512)\n #define BLOCKSIZE\t(RECORDSIZE * 20)\n@@ -248,3 +249,47 @@ int write_tar_archive(struct archiver_args *args)\n \t\twrite_trailer();\n \treturn err;\n }\n+\n+static int write_tar_to_filter(struct archiver_args *args, const char **argv)\n+{\n+\tstruct child_process filter;\n+\tint r;\n+\n+\tmemset(&filter, 0, sizeof(filter));\n+\tfilter.argv = argv;\n+\tfilter.in = -1;\n+\n+\tif (start_command(&filter) < 0)\n+\t\tdie_errno(\"unable to start '%s' filter\", argv[0]);\n+\tclose(1);\n+\tif (dup2(filter.in, 1) < 0)\n+\t\tdie_errno(\"unable to redirect descriptor\");\n+\tclose(filter.in);\n+\n+\tr = write_tar_archive(args);\n+\n+\tclose(1);\n+\tif (finish_command(&filter) != 0)\n+\t\tdie(\"'%s' filter reported error\", argv[0]);\n+\n+\treturn r;\n+}\n+\n+int write_tgz_archive(struct archiver_args *args)\n+{\n+\tchar compression[4];\n+\tconst char *argv[] = {\n+\t\t\"gzip\",\n+\t\t\"-c\",\n+\t\tNULL, /* compression level */\n+\t\tNULL\n+\t};\n+\n+\tif (args->compression_level >= 0) {\n+\t\tsnprintf(compression, sizeof(compression),\n+\t\t\t \"-%d\", args->compression_level);\n+\t\targv[2] = compression;\n+\t}\n+\n+\treturn write_tar_to_filter(args, argv);\n+}\ndiff --git a/archive.c b/archive.c\nindex 42f2d2f..6073a8d 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -23,6 +23,7 @@ static const struct archiver {\n } archivers[] = {\n \t{ \"tar\", write_tar_archive },\n \t{ \"zip\", write_zip_archive, USES_ZLIB_COMPRESSION },\n+\t{ \"tgz\", write_tgz_archive, USES_ZLIB_COMPRESSION },\n };\n \n static void format_subst(const struct commit *commit,\ndiff --git a/archive.h b/archive.h\nindex 038ac35..c1bf72e 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -23,6 +23,7 @@ typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsign\n */\n extern int write_tar_archive(struct archiver_args *);\n extern int write_zip_archive(struct archiver_args *);\n+extern int write_tgz_archive(struct archiver_args *);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex b14eaba..4f60af5 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -71,6 +71,12 @@ static const char *format_from_name(const char *filename)\n \text++;\n \tif (!strcasecmp(ext, \"zip\"))\n \t\treturn \"--format=zip\";\n+\tif (!strcasecmp(ext, \"tgz\"))\n+\t\treturn \"--format=tgz\";\n+\tif (!strcasecmp(ext, \"gz\") &&\n+\t ext - 4 >= filename &&\n+\t !strcasecmp(ext - 4, \"tar.gz\"))\n+\t\treturn \"--format=tgz\";\n \treturn NULL;\n }\n \ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex cff1b3e..faf2784 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -26,6 +26,7 @@ commit id embedding:\n \n . ./test-lib.sh\n UNZIP=${UNZIP:-unzip}\n+GUNZIP=${GUNZIP:-gunzip}\n \n SUBSTFORMAT=%H%n\n \n@@ -252,4 +253,29 @@ test_expect_success 'git-archive --prefix=olde-' '\n \ttest -f h/olde-a/bin/sh\n '\n \n+test_expect_success 'git archive --format=tgz' '\n+\tgit archive --format=tgz HEAD >e.tgz\n+'\n+\n+test_expect_success 'infer tgz from .tgz filename' '\n+\tgit archive --output=e1.tgz HEAD &&\n+\ttest_cmp e.tgz e1.tgz\n+'\n+\n+test_expect_success 'infer tgz from .tar.gz filename' '\n+\tgit archive --output=e2.tar.gz HEAD &&\n+\ttest_cmp e.tgz e2.tar.gz\n+'\n+\n+if $GUNZIP --version >/dev/null 2>&1; then\n+\ttest_set_prereq GUNZIP\n+else\n+\tsay \"Skipping tgz tests because gunzip was not found\"\n+fi\n+\n+test_expect_success GUNZIP 'extract tgz file' '\n+\tgunzip -c e.tar &&\n+\ttest_cmp b.tar e.tar\n+'\n+\n test_done\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170033","messageId":"20110614204950.GB12776@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7vaadkkvew.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-14T20:49:50Z","receivedAt":"2011-06-14T20:49:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2011 at 01:30:47PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> I didn't know it was that easy (primarily because I didn't know zlib had a\n> ready-to-eat interface to do this).\n\nYes, though I think it may be worth doing the more flexible,\nexternal-filters approach. See elsewhere in the thread.\n\n> > +\tif (!strcasecmp(ext, \"tgz\"))\n> > +\t\treturn \"--format=tgz\";\n> > +\tif (!strcasecmp(ext, \"gz\") &&\n> > +\t ext - 4 >= filename &&\n> > +\t !strcasecmp(ext - 4, \"tar.gz\"))\n> \n> Shouldn't this be\n> \n> \tif (!strcasecmp(ext, \"gz\") && filename < ext - 5 &&\n> !strcasecmp(ext - 5, \".tar.gz\"))\n> \n> to exclude \"hellotar.gz\" and possibly \".tar.gz\" (\"<=\" vs \"<\")?\n\nYeah, definitely. I think the right way forward is to make this less\nhard-coded, to allow people to configure new filters, so this bit of\ncode will get rewritten in my next version. But I'll make sure it is a\nlittle more strict on extension matching.\n\n-Peff\n"},{"id":"170037","messageId":"87vcw8f0d5.fsf@catnip.gol.com","threadId":"27625","inReplyTo":"20110614204950.GB12776@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"Miles Bader","fromEmail":"miles@gnu.org","sentAt":"2011-06-14T23:40:22Z","receivedAt":"2011-06-14T23:40:22Z","isPatch":true,"sender":{"key":"miles@gnu.org","avatar":"https://gravatar.com/avatar/01069b69593af7bff28e2f97afeb3644ae6fe2f5f56cb3a8cf34c5fb8c36efe5?d=mp&s=160"},"body":"Jeff King writes:\n>> I didn't know it was that easy (primarily because I didn't know zlib had a\n>> ready-to-eat interface to do this).\n>\n> Yes, though I think it may be worth doing the more flexible,\n> external-filters approach. See elsewhere in the thread.\n\nGiven the relatively trivial code, isn't it worth doing both...?\n\nOne method for flexibility/multi-threaded-speed, the other for\nportability/robustness (doesn't depend on configuration / setup\ndetails)...\n\n-Miles\n\n-- \n\"Though they may have different meanings, the cries of 'Yeeeee-haw!' and\n 'Allahu akbar!' are, in spirit, not actually all that different.\"\n"},{"id":"170089","messageId":"20110615223030.GA16110@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110614204521.GA12776@sigill.intra.peff.net","subject":"[RFC/PATCH 0/7] user-configurable git-archive output formats","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-15T22:30:30Z","receivedAt":"2011-06-15T22:30:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 14, 2011 at 04:45:21PM -0400, Jeff King wrote:\n\n> The gzip path is not configurable at all. Probably it should read the\n> path and arguments from the config file. In fact, we could even allow\n> arbitrary config like:\n> \n> [tarfilter \"tgz\"]\n> command = gzip -c\n> extension = tgz\n> extension = tar.gz\n\nHere's a series implementing that. You can configure whatever you want,\nand it includes builtin gzip configuration by default. You can override\nto turn it off, or even switch it to run something like pigz instead.\n\nMy biggest reservation with the patches as-is is that they are very\ntar-centric and not orthogonal. Specifically, they won't handle:\n\n 1. Other streamable archive formats you would want to pipe through\n compressors. Do any of these actually exist? I guess we could offer\n \"pax\" as a format eventually, and it might be like tar with\n different defaults? I dunno.\n\n Fixing this would not be too hard. Instead of these being\n \"tarfilters\", they would be \"archive filters\", and they would chain\n to some format, defaulting to \"tar\". Since there is no other\n format right now, we could even punt on writing most of the code\n until somebody adds one. But we would want to get the naming of the\n config options right, since those are user-facing. Maybe\n \"archivefilter\" (unfortunately the more readable archive.filter is\n a little awkward with the way we parse config files)?\n\n 2. In theory you might want to plug in external helpers that are not\n just stream filters, but actually their own container formats (like\n zip). I think people who want 7zip would want this.\n\n But how does git-archive interact with the helper? By definition\n the data it wants is the set of files, not a single stream. So\n either:\n\n a. We give the helper a temporary exported checkout, and it\n generates the stream from that.\n\n b. We use tar as the lingua franca of streaming file containers,\n and let the helper deal with converting to its preferred\n output format.\n\n Option (a) seems horribly inefficient on disk I/O. And if we did\n want to do that, I think it's largely unrelated to this patch\n series.\n\n You can actually do option (b) with this series. In its worst\n case, you can do the same as (a): just untar into a temporary\n directory and compress from there. But a well-written helper could\n convert tar into the output format on the fly.\n\nThe patches are:\n\n [1/7]: archive: reorder option parsing and config reading\n [2/7]: archive: add user-configurable tar-filter infrastructure\n [3/7]: archive: support user tar-filters via --format\n [4/7]: archive: advertise user tar-filters in --list\n [5/7]: archive: refactor format-guessing from filename\n [6/7]: archive: match extensions from user-configured formats\n [7/7]: archive: provide builtin .tar.gz filter\n\n-Peff\n"},{"id":"170090","messageId":"20110615223128.GA16807@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"[PATCH 1/7] archive: reorder option parsing and config reading","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-15T22:31:28Z","receivedAt":"2011-06-15T22:31:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The archive command does three things during its\ninitialization phase:\n\n 1. parse command-line options\n\n 2. setup the git directory\n\n 3. read config\n\nDuring phase (1), if we see any options that do not require\na git directory (like \"--list\"), we handle them immediately\nand exit, making it safe to abort step (2) if we are not in\na git directory.\n\nStep (3) must come after step (2), since the git directory\nmay influence configuration. However, this leaves no\npossibility of configuration from step (3) impacting the\ncommand-line options in step (1) (which is useful, for\nexample, for supporting user-configurable output formats).\n\nInstead, let's reorder this to:\n\n 1. setup the git directory, if it exists\n\n 2. read config\n\n 3. parse command-line options\n\n 4. if we are not in a git repository, die\n\nThis should have the same external behavior, but puts\nconfiguration before command-line parsing.\n\nSigned-off-by: Jeff King \n---\n archive.c | 18 ++++++++++++++----\n 1 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 42f2d2f..2616676 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -387,17 +387,27 @@ static int parse_archive_args(int argc, const char **argv,\n int write_archive(int argc, const char **argv, const char *prefix,\n \t\tint setup_prefix)\n {\n+\tint nongit = 0;\n \tconst struct archiver *ar = NULL;\n \tstruct archiver_args args;\n \n-\targc = parse_archive_args(argc, argv, &ar, &args);\n \tif (setup_prefix && prefix == NULL)\n-\t\tprefix = setup_git_directory();\n+\t\tprefix = setup_git_directory_gently(&nongit);\n+\n+\tgit_config(git_default_config, NULL);\n+\n+\targc = parse_archive_args(argc, argv, &ar, &args);\n+\tif (nongit) {\n+\t\t/*\n+\t\t * We know this will die() with an error, so we could just\n+\t\t * die ourselves; but its error message will be more specific\n+\t\t * than what we could write here.\n+\t\t */\n+\t\tsetup_git_directory();\n+\t}\n \n \tparse_treeish_arg(argv, &args, prefix);\n \tparse_pathspec_arg(argv + 1, &args);\n \n-\tgit_config(git_default_config, NULL);\n-\n \treturn ar->write_archive(&args);\n }\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170091","messageId":"20110615223301.GB16807@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"[PATCH 2/7] archive: add user-configurable tar-filter infrastructure","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-15T22:33:01Z","receivedAt":"2011-06-15T22:33:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Archive supports two output formats: tar and zip. The tar\nfiles are totally uncompressed, and it is up to the user to\npipe them through gzip or similar. This is no more than\na minor inconvenience when running archive locally. However,\nfor remote calls to upload-archive, having the server do the\ncompression can save a lot of bandwidth.\n\nThis patch lays the foundation for tar filters; it parses\nuser configuration into an internal representation, but\ndoesn't yet do anything with the result.\n\nIt also introduces some tests to document the intended\nusage.\n\nSigned-off-by: Jeff King \n---\n Makefile | 1 +\n archive-tar-filter.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++++++\n archive.c | 1 +\n archive.h | 15 +++++++\n t/t5000-tar-tree.sh | 54 ++++++++++++++++++++++++\n 5 files changed, 183 insertions(+), 0 deletions(-)\n create mode 100644 archive-tar-filter.c\n\ndiff --git a/Makefile b/Makefile\nindex e40ac0c..bd3002f 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -573,6 +573,7 @@ LIB_OBJS += advice.o\n LIB_OBJS += alias.o\n LIB_OBJS += alloc.o\n LIB_OBJS += archive.o\n+LIB_OBJS += archive-tar-filter.o\n LIB_OBJS += archive-tar.o\n LIB_OBJS += archive-zip.o\n LIB_OBJS += attr.o\ndiff --git a/archive-tar-filter.c b/archive-tar-filter.c\nnew file mode 100644\nindex 0000000..211f1df\n--- /dev/null\n+++ b/archive-tar-filter.c\n@@ -0,0 +1,112 @@\n+#include \"cache.h\"\n+#include \"archive.h\"\n+\n+struct tar_filter *tar_filters;\n+static struct tar_filter **tar_filters_tail = &tar_filters;\n+\n+static struct tar_filter *tar_filter_new(const char *name, int namelen)\n+{\n+\tstruct tar_filter *tf;\n+\ttf = xcalloc(1, sizeof(*tf));\n+\ttf->name = xmemdupz(name, namelen);\n+\ttf->extensions.strdup_strings = 1;\n+\t*tar_filters_tail = tf;\n+\ttar_filters_tail = &tf->next;\n+\treturn tf;\n+}\n+\n+static void tar_filter_free(struct tar_filter *tf)\n+{\n+\tstring_list_clear(&tf->extensions, 0);\n+\tfree(tf->name);\n+\tfree(tf->command);\n+\tfree(tf);\n+}\n+\n+static struct tar_filter *tar_filter_by_namelen(const char *name,\n+\t\t\t\t\t\tint len)\n+{\n+\tstruct tar_filter *p;\n+\tfor (p = tar_filters; p; p = p->next)\n+\t\tif (!strncmp(p->name, name, len) && !p->name[len])\n+\t\t\treturn p;\n+\treturn NULL;\n+}\n+\n+struct tar_filter *tar_filter_by_name(const char *name)\n+{\n+\treturn tar_filter_by_namelen(name, strlen(name));\n+}\n+\n+static int tar_filter_config(const char *var, const char *value, void *data)\n+{\n+\tstruct tar_filter *tf;\n+\tconst char *dot;\n+\tconst char *name;\n+\tconst char *type;\n+\tint namelen;\n+\n+\tif (prefixcmp(var, \"tarfilter.\"))\n+\t\treturn 0;\n+\tdot = strrchr(var, '.');\n+\tif (dot == var + 9)\n+\t\treturn 0;\n+\n+\tname = var + 10;\n+\tnamelen = dot - name;\n+\ttype = dot + 1;\n+\n+\ttf = tar_filter_by_namelen(name, namelen);\n+\tif (!tf)\n+\t\ttf = tar_filter_new(name, namelen);\n+\n+\tif (!strcmp(type, \"command\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\ttf->command = xstrdup(value);\n+\t\treturn 0;\n+\t}\n+\telse if (!strcmp(type, \"extension\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tstring_list_append(&tf->extensions, value);\n+\t\treturn 0;\n+\t}\n+\telse if (!strcmp(type, \"compressionlevels\")) {\n+\t\ttf->use_compression = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static void remove_filters_without_command(void)\n+{\n+\tstruct tar_filter *p = tar_filters;\n+\tstruct tar_filter **last = &tar_filters;\n+\n+\twhile (p) {\n+\t\tif (p->command && *p->command)\n+\t\t\tlast = &p->next;\n+\t\telse {\n+\t\t\t*last = p->next;\n+\t\t\ttar_filter_free(p);\n+\t\t}\n+\t\tp = *last;\n+\t}\n+}\n+\n+/*\n+ * We don't want to load twice, since some of our\n+ * values actually append rather than overwrite.\n+ */\n+static int tar_filter_config_loaded;\n+extern void tar_filter_load_config(void)\n+{\n+\tif (tar_filter_config_loaded)\n+\t\treturn;\n+\ttar_filter_config_loaded = 1;\n+\n+\tgit_config(tar_filter_config, NULL);\n+\tremove_filters_without_command();\n+}\ndiff --git a/archive.c b/archive.c\nindex 2616676..2ed9259 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -395,6 +395,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \t\tprefix = setup_git_directory_gently(&nongit);\n \n \tgit_config(git_default_config, NULL);\n+\ttar_filter_load_config();\n \n \targc = parse_archive_args(argc, argv, &ar, &args);\n \tif (nongit) {\ndiff --git a/archive.h b/archive.h\nindex 038ac35..8386c46 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -1,6 +1,8 @@\n #ifndef ARCHIVE_H\n #define ARCHIVE_H\n \n+#include \"string-list.h\"\n+\n struct archiver_args {\n \tconst char *base;\n \tsize_t baselen;\n@@ -27,4 +29,17 @@ extern int write_zip_archive(struct archiver_args *);\n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\n \n+struct tar_filter {\n+\tchar *name;\n+\tchar *command;\n+\tstruct string_list extensions;\n+\tunsigned use_compression:1;\n+\tstruct tar_filter *next;\n+};\n+\n+extern struct tar_filter *tar_filters;\n+extern struct tar_filter *tar_filter_by_name(const char *name);\n+\n+extern void tar_filter_load_config(void);\n+\n #endif\t/* ARCHIVE_H */\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex cff1b3e..c3e1a4e 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -252,4 +252,58 @@ test_expect_success 'git-archive --prefix=olde-' '\n \ttest -f h/olde-a/bin/sh\n '\n \n+test_expect_success 'setup fake tar filter' '\n+\tgit config tarfilter.fake.command \"cat >/dev/null; echo args: \"\n+'\n+\n+test_expect_failure 'filter does not allow compression levels by default' '\n+\ttest_must_fail git archive --format=fake -9 HEAD >output\n+'\n+\n+test_expect_failure 'filters can allow compression levels' '\n+\tgit config tarfilter.fake.compressionlevels true &&\n+\techo \"args: -9\" >expect &&\n+\tgit archive --format=fake -9 HEAD >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_failure 'archive --list mentions user filter' '\n+\tgit archive --list >output &&\n+\tgrep \"^fake\\$\" output\n+'\n+\n+test_expect_failure 'archive --list shows remote user filters' '\n+\tgit archive --list --remote=. >output &&\n+\tgrep \"^fake\\$\" output\n+'\n+\n+test_expect_success 'setup slightly more useful tar filter' '\n+\tgit config tarfilter.foo.command \"tr ab ba\" &&\n+\tgit config --add tarfilter.foo.extension tar.foo &&\n+\tgit config --add tarfilter.foo.extension bar\n+'\n+\n+test_expect_failure 'archive outputs in configurable format' '\n+\tgit archive --format=foo HEAD >config.tar.foo &&\n+\ttr ab ba config.tar &&\n+\ttest_cmp b.tar config.tar\n+'\n+\n+test_expect_failure 'archive selects implicit format by configured extension' '\n+\tgit archive -o config-implicit.tar.foo HEAD &&\n+\ttest_cmp config.tar.foo config-implicit.tar.foo &&\n+\tgit archive -o config-implicit.bar HEAD &&\n+\ttest_cmp config.tar.foo config-implicit.bar\n+'\n+\n+test_expect_success 'default output format remains tar' '\n+\tgit archive -o config-implicit.baz HEAD &&\n+\ttest_cmp b.tar config-implicit.baz\n+'\n+\n+test_expect_failure 'extension matching requires dot' '\n+\tgit archive -o config-implicittar.foo HEAD &&\n+\ttest_cmp b.tar config-implicittar.foo\n+'\n+\n test_done\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170092","messageId":"20110615223312.GC16807@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"[PATCH 3/7] archive: support user tar-filters via --format","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-15T22:33:12Z","receivedAt":"2011-06-15T22:33:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The previous commit set up the infrastructure to read\ntar-filter configuration. This commit actually uses it to\npipe the tar output through the specified filter.\n\nSigned-off-by: Jeff King \n---\n archive-tar-filter.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n archive.c | 16 +++++++++++++---\n archive.h | 2 ++\n t/t5000-tar-tree.sh | 6 +++---\n 4 files changed, 66 insertions(+), 6 deletions(-)\n\ndiff --git a/archive-tar-filter.c b/archive-tar-filter.c\nindex 211f1df..ffe510e 100644\n--- a/archive-tar-filter.c\n+++ b/archive-tar-filter.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"archive.h\"\n+#include \"run-command.h\"\n \n struct tar_filter *tar_filters;\n static struct tar_filter **tar_filters_tail = &tar_filters;\n@@ -110,3 +111,50 @@ extern void tar_filter_load_config(void)\n \tgit_config(tar_filter_config, NULL);\n \tremove_filters_without_command();\n }\n+\n+static int write_tar_to_filter(struct archiver_args *args, const char *cmd)\n+{\n+\tstruct child_process filter;\n+\tconst char *argv[2];\n+\tint r;\n+\n+\tmemset(&filter, 0, sizeof(filter));\n+\targv[0] = cmd;\n+\targv[1] = NULL;\n+\tfilter.argv = argv;\n+\tfilter.use_shell = 1;\n+\tfilter.in = -1;\n+\n+\tif (start_command(&filter) < 0)\n+\t\tdie_errno(\"unable to start '%s' filter\", argv[0]);\n+\tclose(1);\n+\tif (dup2(filter.in, 1) < 0)\n+\t\tdie_errno(\"unable to redirect descriptor\");\n+\tclose(filter.in);\n+\n+\tr = write_tar_archive(args);\n+\n+\tclose(1);\n+\tif (finish_command(&filter) != 0)\n+\t\tdie(\"'%s' filter reported error\", argv[0]);\n+\n+\treturn r;\n+}\n+\n+int write_tar_filter_archive(struct archiver_args *args)\n+{\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tint r;\n+\n+\tif (!args->tar_filter)\n+\t\tdie(\"BUG: tar-filter archiver called with no filter defined\");\n+\n+\tstrbuf_addstr(&cmd, args->tar_filter->command);\n+\tif (args->tar_filter->use_compression && args->compression_level >= 0)\n+\t\tstrbuf_addf(&cmd, \" -%d\", args->compression_level);\n+\n+\tr = write_tar_to_filter(args, cmd.buf);\n+\n+\tstrbuf_release(&cmd);\n+\treturn r;\n+}\ndiff --git a/archive.c b/archive.c\nindex 2ed9259..cf58faa 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -24,6 +24,9 @@ static const struct archiver {\n \t{ \"tar\", write_tar_archive },\n \t{ \"zip\", write_zip_archive, USES_ZLIB_COMPRESSION },\n };\n+static const struct archiver tar_filter_archiver = {\n+\t\"tar-filter\", write_tar_filter_archive\n+};\n \n static void format_subst(const struct commit *commit,\n const char *src, size_t len,\n@@ -364,12 +367,19 @@ static int parse_archive_args(int argc, const char **argv,\n \tif (argc < 1)\n \t\tusage_with_options(archive_usage, opts);\n \t*ar = lookup_archiver(format);\n-\tif (!*ar)\n-\t\tdie(\"Unknown archive format '%s'\", format);\n+\n+\t/* Fallback to user-configured tar filters */\n+\tif (!*ar) {\n+\t\targs->tar_filter = tar_filter_by_name(format);\n+\t\tif (!args->tar_filter)\n+\t\t\tdie(\"Unknown archive format '%s'\", format);\n+\t\t*ar = &tar_filter_archiver;\n+\t}\n \n \targs->compression_level = Z_DEFAULT_COMPRESSION;\n \tif (compression_level != -1) {\n-\t\tif ((*ar)->flags & USES_ZLIB_COMPRESSION)\n+\t\tif ((*ar)->flags & USES_ZLIB_COMPRESSION ||\n+\t\t (args->tar_filter && args->tar_filter->use_compression))\n \t\t\targs->compression_level = compression_level;\n \t\telse {\n \t\t\tdie(\"Argument not supported for format '%s': -%d\",\ndiff --git a/archive.h b/archive.h\nindex 8386c46..fb2bb9e 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -14,6 +14,7 @@ struct archiver_args {\n \tunsigned int verbose : 1;\n \tunsigned int worktree_attributes : 1;\n \tint compression_level;\n+\tstruct tar_filter *tar_filter;\n };\n \n typedef int (*write_archive_fn_t)(struct archiver_args *);\n@@ -25,6 +26,7 @@ typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsign\n */\n extern int write_tar_archive(struct archiver_args *);\n extern int write_zip_archive(struct archiver_args *);\n+extern int write_tar_filter_archive(struct archiver_args *);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex c3e1a4e..2b2b128 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -256,11 +256,11 @@ test_expect_success 'setup fake tar filter' '\n \tgit config tarfilter.fake.command \"cat >/dev/null; echo args: \"\n '\n \n-test_expect_failure 'filter does not allow compression levels by default' '\n+test_expect_success 'filter does not allow compression levels by default' '\n \ttest_must_fail git archive --format=fake -9 HEAD >output\n '\n \n-test_expect_failure 'filters can allow compression levels' '\n+test_expect_success 'filters can allow compression levels' '\n \tgit config tarfilter.fake.compressionlevels true &&\n \techo \"args: -9\" >expect &&\n \tgit archive --format=fake -9 HEAD >output &&\n@@ -283,7 +283,7 @@ test_expect_success 'setup slightly more useful tar filter' '\n \tgit config --add tarfilter.foo.extension bar\n '\n \n-test_expect_failure 'archive outputs in configurable format' '\n+test_expect_success 'archive outputs in configurable format' '\n \tgit archive --format=foo HEAD >config.tar.foo &&\n \ttr ab ba config.tar &&\n \ttest_cmp b.tar config.tar\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170093","messageId":"20110615223332.GD16807@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"[PATCH 4/7] archive: advertise user tar-filters in --list","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-15T22:33:32Z","receivedAt":"2011-06-15T22:33:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"These can be selected by --format, so we need to let users\nknow about them. It is especially important for the remote\ncase, since this is the only method by which users can find\nout which formats the remote has configured.\n\nSigned-off-by: Jeff King \n---\n archive.c | 3 +++\n t/t5000-tar-tree.sh | 4 ++--\n 2 files changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex cf58faa..a987936 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -358,8 +358,11 @@ static int parse_archive_args(int argc, const char **argv,\n \t\tbase = \"\";\n \n \tif (list) {\n+\t\tstruct tar_filter *p;\n \t\tfor (i = 0; i < ARRAY_SIZE(archivers); i++)\n \t\t\tprintf(\"%s\\n\", archivers[i].name);\n+\t\tfor (p = tar_filters; p; p = p->next)\n+\t\t\tprintf(\"%s\\n\", p->name);\n \t\texit(0);\n \t}\n \ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 2b2b128..9f959b1 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -267,12 +267,12 @@ test_expect_success 'filters can allow compression levels' '\n \ttest_cmp expect output\n '\n \n-test_expect_failure 'archive --list mentions user filter' '\n+test_expect_success 'archive --list mentions user filter' '\n \tgit archive --list >output &&\n \tgrep \"^fake\\$\" output\n '\n \n-test_expect_failure 'archive --list shows remote user filters' '\n+test_expect_success 'archive --list shows remote user filters' '\n \tgit archive --list --remote=. >output &&\n \tgrep \"^fake\\$\" output\n '\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170094","messageId":"20110615223407.GE16807@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"[PATCH 5/7] archive: refactor format-guessing from filename","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-15T22:34:07Z","receivedAt":"2011-06-15T22:34:07Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The process for guessing an archive output format based on\nthe filename is something like this:\n\n a. parse --output in cmd_archive; check the filename\n against a static set of mapping heuristics (right now\n it just matches \".zip\" for zip files).\n\n b. if found, stick a fake \"--format=zip\" at the beginning\n of the arguments list (if the user did specify a\n --format manually, the later option will override our\n fake one)\n\n c. if it's a remote call, ship the arguments to the remote\n (including the fake), which will call write_archive on\n their end\n\n d. if it's local, ship the arguments to write_archive\n locally\n\nThere are two problems:\n\n 1. The set of mappings is static and at too high a level.\n The write_archive level is going to check config for\n user-defined formats, some of which will specify\n extensions. We need to delay lookup until those are\n parsed, so we can match against them.\n\n 2. For a remote archive call, our set of mappings (or\n formats) may not match the remote side's. This is OK in\n practice right now, because all versions of git\n understand \"zip\" and \"tar\". But as new formats are\n added, there is going to be a mismatch between what the\n client can do and what the remote server can do.\n\nTo fix (1), this patch refactors the location guessing to\nhappen at the write_archive level, instead of the\ncmd_archive level. So instead of sticking a fake --format\nfield in the argv list, we actually pass a \"name hint\" down\nthe callchain; this hint is used at the appropriate time to\nguess the format (if one hasn't been given already).\n\nThis patch leaves (2) unfixed. The name_hint is converted to\na \"--format\" option as before, and passed to the remote. We\ncould in theory pass the name hint to the remote side and\nlet it decide which format to use. But that introduces a\ncompatibility problem, as we have no place to put that\ninformation during the remote call without adding a new\n\"--name-hint=\" argument. An older version of git would choke\non that, and the client has no way of knowing if the server\nis new enough or not (i.e., there is no capabilities\nadvertisement, as there is with the git protocol itself).\n\nOn top of this, it's unclear whether the remote side should\nbe in charge of format selection, anyway. There is a minor\ninformation leak; the server will learn about the filename\nyou are using to save. If we sent just the basename, though,\nthat would lessen the leak and still give the remote side\nenough information to make a decision.\n\nBut more important is that the name hint is only a hint, and\nwe default to the tar format. Which means that\ninconsistencies between the client's and server's set of\nformats will have confusing results. For example, imagine\nthe client learns about \"tar.gz\" as an extension for gzip'd\ntar (\"tgz\") files, but the server does not. Locally,\nrunning:\n\n git archive -o file.tar.gz HEAD\n\nwill produce a gzip'd file. If we make the mapping decision\nlocally, then running:\n\n git archive --remote=origin -o file.tar.gz HEAD\n\nwill send \"--format=tgz\" to the remote side. The server will\ncomplain, saying that it doesn't know about the tgz format.\n\nIf we instead send the name hint to the remote side and let\nit make the decision, it will not know what \".tar.gz\" is,\nand will silently default to a plain tar, without the user\neven realizing it.\n\nThe flip side of this is an old client talking to a new\nserver (i.e., only the servers knows about \".tar.gz\"). If we\nmap the filename remotely, then the user is happy. If we map\nit locally, though, we will send the server no --format and\nit will silently default to tar.\n\nSo the question is: should the mapping of filenames to\nformats be consistent for a single client (i.e., doing it\nlocally or against a remote will either produce the same\nformat, or report an error if the remote does not support\nthe format), or should it be consistent for multiple clients\nhitting the same server (i.e., no matter what machine I am\non, if I use git-archive to hit kernel.org, I will always\nsee the same format for the same filename)?\n\nI chose consistency on a single client (i.e., we do the\nmapping locally), because:\n\n 1. Using git on one machine against multiple remotes is\n more common than using git on many machines against the\n same remote. So it's less likely for the user to be\n surprised.\n\n 2. Even if we wanted to do the reverse, it is not as\n simple as making the decision and writing some code.\n Because the server literally says nothing before we\n send it arguments, it's difficult to get\n interoperability between versions. We'd probably end up\n having to write the server side, wait sufficiently long\n for everybody to have deployed it, and then write the\n client side.\n\nSigned-off-by: Jeff King \n---\n archive.c | 25 +++++++++++++++++++---\n archive.h | 4 ++-\n builtin/archive.c | 51 ++++++++++++++-------------------------------\n builtin/upload-archive.c | 2 +-\n 4 files changed, 41 insertions(+), 41 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex a987936..e04f689 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -302,9 +302,10 @@ static void parse_treeish_arg(const char **argv,\n \t PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }\n \n static int parse_archive_args(int argc, const char **argv,\n-\t\tconst struct archiver **ar, struct archiver_args *args)\n+\t\tconst struct archiver **ar, struct archiver_args *args,\n+\t\tconst char *name_hint)\n {\n-\tconst char *format = \"tar\";\n+\tconst char *format = NULL;\n \tconst char *base = NULL;\n \tconst char *remote = NULL;\n \tconst char *exec = NULL;\n@@ -366,6 +367,11 @@ static int parse_archive_args(int argc, const char **argv,\n \t\texit(0);\n \t}\n \n+\tif (!format && name_hint)\n+\t\tformat = archive_format_from_filename(name_hint);\n+\tif (!format)\n+\t\tformat = \"tar\";\n+\n \t/* We need at least one parameter -- tree-ish */\n \tif (argc < 1)\n \t\tusage_with_options(archive_usage, opts);\n@@ -398,7 +404,7 @@ static int parse_archive_args(int argc, const char **argv,\n }\n \n int write_archive(int argc, const char **argv, const char *prefix,\n-\t\tint setup_prefix)\n+\t\tint setup_prefix, const char *name_hint)\n {\n \tint nongit = 0;\n \tconst struct archiver *ar = NULL;\n@@ -410,7 +416,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tgit_config(git_default_config, NULL);\n \ttar_filter_load_config();\n \n-\targc = parse_archive_args(argc, argv, &ar, &args);\n+\targc = parse_archive_args(argc, argv, &ar, &args, name_hint);\n \tif (nongit) {\n \t\t/*\n \t\t * We know this will die() with an error, so we could just\n@@ -425,3 +431,14 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \n \treturn ar->write_archive(&args);\n }\n+\n+const char *archive_format_from_filename(const char *filename)\n+{\n+\tconst char *ext = strrchr(filename, '.');\n+\tif (!ext)\n+\t\treturn NULL;\n+\text++;\n+\tif (!strcasecmp(ext, \"zip\"))\n+\t\treturn \"zip\";\n+\treturn NULL;\n+}\ndiff --git a/archive.h b/archive.h\nindex fb2bb9e..894d4c4 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -29,7 +29,7 @@ extern int write_zip_archive(struct archiver_args *);\n extern int write_tar_filter_archive(struct archiver_args *);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n-extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\n+extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint);\n \n struct tar_filter {\n \tchar *name;\n@@ -44,4 +44,6 @@ extern struct tar_filter *tar_filter_by_name(const char *name);\n \n extern void tar_filter_load_config(void);\n \n+const char *archive_format_from_filename(const char *filename);\n+\n #endif\t/* ARCHIVE_H */\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex b14eaba..2578cf5 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -24,7 +24,8 @@ static void create_output_file(const char *output_file)\n }\n \n static int run_remote_archiver(int argc, const char **argv,\n-\t\t\t const char *remote, const char *exec)\n+\t\t\t const char *remote, const char *exec,\n+\t\t\t const char *name_hint)\n {\n \tchar buf[LARGE_PACKET_MAX];\n \tint fd[2], i, len, rv;\n@@ -37,6 +38,17 @@ static int run_remote_archiver(int argc, const char **argv,\n \ttransport = transport_get(_remote, _remote->url[0]);\n \ttransport_connect(transport, \"git-upload-archive\", exec, fd);\n \n+\t/*\n+\t * Inject a fake --format field at the beginning of the\n+\t * arguments, with the format inferred from our output\n+\t * filename. This way explicit --format options can override\n+\t * it.\n+\t */\n+\tif (name_hint) {\n+\t\tconst char *format = archive_format_from_filename(name_hint);\n+\t\tif (format)\n+\t\t\tpacket_write(fd[1], \"argument --format=%s\\n\", format);\n+\t}\n \tfor (i = 1; i < argc; i++)\n \t\tpacket_write(fd[1], \"argument %s\\n\", argv[i]);\n \tpacket_flush(fd[1]);\n@@ -63,17 +75,6 @@ static int run_remote_archiver(int argc, const char **argv,\n \treturn !!rv;\n }\n \n-static const char *format_from_name(const char *filename)\n-{\n-\tconst char *ext = strrchr(filename, '.');\n-\tif (!ext)\n-\t\treturn NULL;\n-\text++;\n-\tif (!strcasecmp(ext, \"zip\"))\n-\t\treturn \"--format=zip\";\n-\treturn NULL;\n-}\n-\n #define PARSE_OPT_KEEP_ALL ( PARSE_OPT_KEEP_DASHDASH | \t\\\n \t\t\t PARSE_OPT_KEEP_ARGV0 | \t\\\n \t\t\t PARSE_OPT_KEEP_UNKNOWN |\t\\\n@@ -84,7 +85,6 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \tconst char *exec = \"git-upload-archive\";\n \tconst char *output = NULL;\n \tconst char *remote = NULL;\n-\tconst char *format_option = NULL;\n \tstruct option local_opts[] = {\n \t\tOPT_STRING('o', \"output\", &output, \"file\",\n \t\t\t\"write the archive to this file\"),\n@@ -98,32 +98,13 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, local_opts, NULL,\n \t\t\t PARSE_OPT_KEEP_ALL);\n \n-\tif (output) {\n+\tif (output)\n \t\tcreate_output_file(output);\n-\t\tformat_option = format_from_name(output);\n-\t}\n-\n-\t/*\n-\t * We have enough room in argv[] to muck it in place, because\n-\t * --output must have been given on the original command line\n-\t * if we get to this point, and parse_options() must have eaten\n-\t * it, i.e. we can add back one element to the array.\n-\t *\n-\t * We add a fake --format option at the beginning, with the\n-\t * format inferred from our output filename. This way explicit\n-\t * --format options can override it, and the fake option is\n-\t * inserted before any \"--\" that might have been given.\n-\t */\n-\tif (format_option) {\n-\t\tmemmove(argv + 2, argv + 1, sizeof(*argv) * argc);\n-\t\targv[1] = format_option;\n-\t\targv[++argc] = NULL;\n-\t}\n \n \tif (remote)\n-\t\treturn run_remote_archiver(argc, argv, remote, exec);\n+\t\treturn run_remote_archiver(argc, argv, remote, exec, output);\n \n \tsetvbuf(stderr, NULL, _IOLBF, BUFSIZ);\n \n-\treturn write_archive(argc, argv, prefix, 1);\n+\treturn write_archive(argc, argv, prefix, 1, output);\n }\ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex 73f788e..e6bb97d 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -64,7 +64,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)\n \tsent_argv[sent_argc] = NULL;\n \n \t/* parse all options sent by the client */\n-\treturn write_archive(sent_argc, sent_argv, prefix, 0);\n+\treturn write_archive(sent_argc, sent_argv, prefix, 0, NULL);\n }\n \n __attribute__((format (printf, 1, 2)))\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170095","messageId":"20110615223432.GF16807@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"[PATCH 6/7] archive: match extensions from user-configured formats","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-15T22:34:32Z","receivedAt":"2011-06-15T22:34:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This lets you configure a format like:\n\n [tarfilter \"tgz\"]\n command = gzip\n extension = tgz\n extension = tar.gz\n\nand have it automatically used for \"foo.tgz\" or \"foo.tar.gz\".\n\nSigned-off-by: Jeff King \n---\n archive-tar-filter.c | 29 +++++++++++++++++++++++++++++\n archive.c | 12 ++++++++++++\n archive.h | 1 +\n t/t5000-tar-tree.sh | 4 ++--\n 4 files changed, 44 insertions(+), 2 deletions(-)\n\ndiff --git a/archive-tar-filter.c b/archive-tar-filter.c\nindex ffe510e..e749133 100644\n--- a/archive-tar-filter.c\n+++ b/archive-tar-filter.c\n@@ -39,6 +39,35 @@ struct tar_filter *tar_filter_by_name(const char *name)\n \treturn tar_filter_by_namelen(name, strlen(name));\n }\n \n+static int match_extension(const char *filename, const char *ext)\n+{\n+\tint prefixlen = strlen(filename) - strlen(ext);\n+\n+\t/*\n+\t * We need 1 character for the '.', and 1 character to ensure that the\n+\t * prefix is non-empty (i.e., we don't match \".tar.gz\" with no actual\n+\t * filename).\n+\t */\n+\tif (prefixlen < 2 || filename[prefixlen-1] != '.')\n+\t\treturn 0;\n+\treturn !strcmp(filename + prefixlen, ext);\n+}\n+\n+struct tar_filter *tar_filter_by_extension(const char *filename)\n+{\n+\tstruct tar_filter *p;\n+\n+\tfor (p = tar_filters; p; p = p->next) {\n+\t\tint i;\n+\t\tfor (i = 0; i < p->extensions.nr; i++) {\n+\t\t\tconst char *ext = p->extensions.items[i].string;\n+\t\t\tif (match_extension(filename, ext))\n+\t\t\t\treturn p;\n+\t\t}\n+\t}\n+\treturn NULL;\n+}\n+\n static int tar_filter_config(const char *var, const char *value, void *data)\n {\n \tstruct tar_filter *tf;\ndiff --git a/archive.c b/archive.c\nindex e04f689..e509b6c 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -434,11 +434,23 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \n const char *archive_format_from_filename(const char *filename)\n {\n+\tstruct tar_filter *tf;\n \tconst char *ext = strrchr(filename, '.');\n \tif (!ext)\n \t\treturn NULL;\n \text++;\n \tif (!strcasecmp(ext, \"zip\"))\n \t\treturn \"zip\";\n+\n+\t/*\n+\t * Fallback to user-configured tar filters; but note\n+\t * that we might have to load config ourselves, first,\n+\t * if we are not being called via write_archive.\n+\t */\n+\ttar_filter_load_config();\n+\ttf = tar_filter_by_extension(filename);\n+\tif (tf)\n+\t\treturn tf->name;\n+\n \treturn NULL;\n }\ndiff --git a/archive.h b/archive.h\nindex 894d4c4..80c89dc 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -41,6 +41,7 @@ struct tar_filter {\n \n extern struct tar_filter *tar_filters;\n extern struct tar_filter *tar_filter_by_name(const char *name);\n+extern struct tar_filter *tar_filter_by_extension(const char *filename);\n \n extern void tar_filter_load_config(void);\n \ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 9f959b1..fe661f3 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -289,7 +289,7 @@ test_expect_success 'archive outputs in configurable format' '\n \ttest_cmp b.tar config.tar\n '\n \n-test_expect_failure 'archive selects implicit format by configured extension' '\n+test_expect_success 'archive selects implicit format by configured extension' '\n \tgit archive -o config-implicit.tar.foo HEAD &&\n \ttest_cmp config.tar.foo config-implicit.tar.foo &&\n \tgit archive -o config-implicit.bar HEAD &&\n@@ -301,7 +301,7 @@ test_expect_success 'default output format remains tar' '\n \ttest_cmp b.tar config-implicit.baz\n '\n \n-test_expect_failure 'extension matching requires dot' '\n+test_expect_success 'extension matching requires dot' '\n \tgit archive -o config-implicittar.foo HEAD &&\n \ttest_cmp b.tar config-implicittar.foo\n '\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170096","messageId":"20110615223501.GG16807@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"[PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-15T22:35:01Z","receivedAt":"2011-06-15T22:35:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This works exactly as if the user had configured it via:\n\n [tarfilter \"tgz\"]\n\tcommand = gzip\n\textension = tgz\n\textension = tar.gz\n\tcompressionlevels = true\n\nbut since it is so common, it's convenient to have it\nbuiltin without the user needing to do anything.\n\nSigned-off-by: Jeff King \n---\n archive-tar-filter.c | 12 ++++++++++++\n t/t5000-tar-tree.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 60 insertions(+), 0 deletions(-)\n\ndiff --git a/archive-tar-filter.c b/archive-tar-filter.c\nindex e749133..de8719a 100644\n--- a/archive-tar-filter.c\n+++ b/archive-tar-filter.c\n@@ -126,6 +126,17 @@ static void remove_filters_without_command(void)\n \t}\n }\n \n+static void load_builtin_filters(void)\n+{\n+\tstruct tar_filter *tf;\n+\n+\ttf = tar_filter_new(\"tgz\", strlen(\"tgz\"));\n+\ttf->command = xstrdup(\"gzip\");\n+\tstring_list_append(&tf->extensions, \"tgz\");\n+\tstring_list_append(&tf->extensions, \"tar.gz\");\n+\ttf->use_compression = 1;\n+}\n+\n /*\n * We don't want to load twice, since some of our\n * values actually append rather than overwrite.\n@@ -137,6 +148,7 @@ extern void tar_filter_load_config(void)\n \t\treturn;\n \ttar_filter_config_loaded = 1;\n \n+\tload_builtin_filters();\n \tgit_config(tar_filter_config, NULL);\n \tremove_filters_without_command();\n }\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex fe661f3..ebad295 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -26,6 +26,7 @@ commit id embedding:\n \n . ./test-lib.sh\n UNZIP=${UNZIP:-unzip}\n+GUNZIP=${GUNZIP:-gzip -d}\n \n SUBSTFORMAT=%H%n\n \n@@ -306,4 +307,51 @@ test_expect_success 'extension matching requires dot' '\n \ttest_cmp b.tar config-implicittar.foo\n '\n \n+test_expect_success 'git archive --format=tgz' '\n+\tgit archive --format=tgz HEAD >j.tgz\n+'\n+\n+test_expect_success 'infer tgz from .tgz filename' '\n+\tgit archive --output=j1.tgz HEAD &&\n+\ttest_cmp j.tgz j1.tgz\n+'\n+\n+test_expect_success 'infer tgz from .tar.gz filename' '\n+\tgit archive --output=j2.tar.gz HEAD &&\n+\ttest_cmp j.tgz j2.tar.gz\n+'\n+\n+if $GUNZIP --version >/dev/null 2>&1; then\n+\ttest_set_prereq GUNZIP\n+else\n+\tsay \"Skipping some tgz tests because gunzip was not found\"\n+fi\n+\n+test_expect_success GUNZIP 'extract tgz file' '\n+\t$GUNZIP -c j.tar &&\n+\ttest_cmp b.tar j.tar\n+'\n+\n+test_expect_success GUNZIP 'tgz allows compression levels' '\n+\tgit archive -1 --output=j3.tgz HEAD\n+'\n+\n+test_expect_success 'disable builtin tgz via config' '\n+\tgit config tarfilter.tgz.command \"\"\n+'\n+\n+test_expect_success 'disabled filter does not appear in --list' '\n+\tgit archive --list >output &&\n+\t! grep tgz output\n+'\n+\n+test_expect_success 'disabled filter cannot be used' '\n+\ttest_must_fail git archive --format=tgz HEAD >output\n+'\n+\n+test_expect_success 'disabled filter does not match extensions' '\n+\tgit archive -o disabled.tar.gz HEAD &&\n+\ttest_cmp b.tar disabled.tar.gz\n+'\n+\n test_done\n-- \n1.7.6.rc1.4.g49204\n"},{"id":"170099","messageId":"20110615224640.GA19803@sigill.intra.peff.net","threadId":"27625","inReplyTo":"87vcw8f0d5.fsf@catnip.gol.com","subject":"Re: [PATCH 2/2] archive: support gzipped tar files","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-15T22:46:40Z","receivedAt":"2011-06-15T22:46:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2011 at 08:40:22AM +0900, Miles Bader wrote:\n\n> Jeff King writes:\n> >> I didn't know it was that easy (primarily because I didn't know zlib had a\n> >> ready-to-eat interface to do this).\n> >\n> > Yes, though I think it may be worth doing the more flexible,\n> > external-filters approach. See elsewhere in the thread.\n> \n> Given the relatively trivial code, isn't it worth doing both...?\n>\n> One method for flexibility/multi-threaded-speed, the other for\n> portability/robustness (doesn't depend on configuration / setup\n> details)...\n\nMaybe, although the code is a little less trivial than I hoped (see\nRené's response for some bugs in my original series).\n\nMy series allowing external filters via configuration also comes with\nbuiltin config for gzip. So there's no extra config or setup details for\nthe user, assuming they can run \"gzip\" from their PATH.\n\nSo I think the only disadvantage now is for people who don't have gzip\nat all. I suspect people on such platforms are going to want another\nformat anyway, but we could still help them out. However, I think\ninstead of building it specially into the archive code, it would be\ncleaner to simply ship a bare-bones version of gzip that only stdio\n(i.e., a \"git-gzip\"). It would be no more code than what the internal\nsolution would be, it could be easier to read (since the program is\nself-contained), and it benefits from the SMP speedup.\n\nAlthough at the point we are shipping \"git-gzip\", I really have to\nwonder if people wouldn't prefer to just install gzip themselves. So I'm\ninclined to wait until somebody complains that git+zlib are easy to get\non their system, but gzip isn't.\n\n-Peff\n"},{"id":"170102","messageId":"7vr56uisaa.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"20110615223301.GB16807@sigill.intra.peff.net","subject":"Re: [PATCH 2/7] archive: add user-configurable tar-filter infrastructure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-15T23:33:33Z","receivedAt":"2011-06-15T23:33:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Archive supports two output formats: tar and zip. The tar\n> ...\n> +static struct tar_filter *tar_filter_by_namelen(const char *name,\n> +\t\t\t\t\t\tint len)\n> +{\n> +\tstruct tar_filter *p;\n> +\tfor (p = tar_filters; p; p = p->next)\n> +\t\tif (!strncmp(p->name, name, len) && !p->name[len])\n> +\t\t\treturn p;\n> +\treturn NULL;\n> +}\n\nMakes me wonder if we want to have a generic table that is keyed by name\nwhose contents can be looked up by counted string. string_list is the\nclosest thing we already have, but I do not think it has counted string\ninterface (shouldn't be a rocket surgery to add it, though).\n\n> +static int tar_filter_config(const char *var, const char *value, void *data)\n> +{\n> ...\n> +\tif (!strcmp(type, \"command\")) {\n> +\t\tif (!value)\n> +\t\t\treturn config_error_nonbool(var);\n> +\t\ttf->command = xstrdup(value);\n\nDoes this result in small leak if the same filter is multiply defined, say\nin /etc/gitconfig and then in ~/.gitconfig?\n\n> diff --git a/archive.h b/archive.h\n> index 038ac35..8386c46 100644\n> --- a/archive.h\n> +++ b/archive.h\n> @@ -1,6 +1,8 @@\n> #ifndef ARCHIVE_H\n> #define ARCHIVE_H\n> \n> +#include \"string-list.h\"\n> +\n> struct archiver_args {\n> \tconst char *base;\n> \tsize_t baselen;\n> @@ -27,4 +29,17 @@ extern int write_zip_archive(struct archiver_args *);\n> extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n> extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\n> \n> +struct tar_filter {\n> +\tchar *name;\n> +\tchar *command;\n> +\tstruct string_list extensions;\n> +\tunsigned use_compression:1;\n\nI suspect that you plan to pass sprintf(\"-%d\", level) for the ones marked\nwith this bit, but I wonder if we want to give a bit more control on how a\ncompression level option is shaped for the particular command, and where\non the command line the option comes. As long as we are targetting gzip\nand nothing else it is fine, and I suspect newer compression commands\nwould try to mimic the -[0-9] command line interface gzip has (e.g. xz),\nso this probably is not an issue in practice.\n"},{"id":"170103","messageId":"7vmxhiirlb.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"20110615223407.GE16807@sigill.intra.peff.net","subject":"Re: [PATCH 5/7] archive: refactor format-guessing from filename","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-15T23:48:32Z","receivedAt":"2011-06-15T23:48:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> But more important is that the name hint is only a hint, and\n> we default to the tar format. Which means that\n> inconsistencies between the client's and server's set of\n> formats will have confusing results. For example, imagine\n> the client learns about \"tar.gz\" as an extension for gzip'd\n> tar (\"tgz\") files, but the server does not. Locally,\n> running:\n>\n> git archive -o file.tar.gz HEAD\n>\n> will produce a gzip'd file. If we make the mapping decision\n> locally, then running:\n>\n> git archive --remote=origin -o file.tar.gz HEAD\n>\n> will send \"--format=tgz\" to the remote side. The server will\n> complain, saying that it doesn't know about the tgz format.\n\nAs long as that complaint is clearly marked as coming from the remote\nside, the user now knows that tgz is not supported, and can fall back to a\nplain tar.\n\nAm I being naïve thinking that barfing (and assuming that the user\nunderstands why the remote end barfed) actually is a good thing?\n"},{"id":"170105","messageId":"7vboxyir8y.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"20110615223501.GG16807@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-15T23:55:57Z","receivedAt":"2011-06-15T23:55:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> +test_expect_success 'git archive --format=tgz' '\n> +\tgit archive --format=tgz HEAD >j.tgz\n> +'\n> +\n> +test_expect_success 'infer tgz from .tgz filename' '\n> +\tgit archive --output=j1.tgz HEAD &&\n> +\ttest_cmp j.tgz j1.tgz\n> +'\n\nI suspect this would get intermittent failures for the same reason as\n0c8c385 (gitweb: supply '-n' to gzip for identical output, 2011-04-26)\n"},{"id":"170106","messageId":"7v7h8mir5u.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"7vboxyir8y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-15T23:57:49Z","receivedAt":"2011-06-15T23:57:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jeff King writes:\n>\n>> +test_expect_success 'git archive --format=tgz' '\n>> +\tgit archive --format=tgz HEAD >j.tgz\n>> +'\n>> +\n>> +test_expect_success 'infer tgz from .tgz filename' '\n>> +\tgit archive --output=j1.tgz HEAD &&\n>> +\ttest_cmp j.tgz j1.tgz\n>> +'\n>\n> I suspect this would get intermittent failures for the same reason as\n> 0c8c385 (gitweb: supply '-n' to gzip for identical output, 2011-04-26)\n\nTo be squashed into 7/7, I guess...\n\n archive-tar-filter.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/archive-tar-filter.c b/archive-tar-filter.c\nindex de8719a..d6e4e32 100644\n--- a/archive-tar-filter.c\n+++ b/archive-tar-filter.c\n@@ -131,7 +131,7 @@ static void load_builtin_filters(void)\n \tstruct tar_filter *tf;\n \n \ttf = tar_filter_new(\"tgz\", strlen(\"tgz\"));\n-\ttf->command = xstrdup(\"gzip\");\n+\ttf->command = xstrdup(\"gzip -n\");\n \tstring_list_append(&tf->extensions, \"tgz\");\n \tstring_list_append(&tf->extensions, \"tar.gz\");\n \ttf->use_compression = 1;\n"},{"id":"170107","messageId":"20110616002959.GA20355@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7vr56uisaa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 2/7] archive: add user-configurable tar-filter infrastructure","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-16T00:29:59Z","receivedAt":"2011-06-16T00:29:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2011 at 04:33:33PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Archive supports two output formats: tar and zip. The tar\n> > ...\n> > +static struct tar_filter *tar_filter_by_namelen(const char *name,\n> > +\t\t\t\t\t\tint len)\n> > +{\n> > +\tstruct tar_filter *p;\n> > +\tfor (p = tar_filters; p; p = p->next)\n> > +\t\tif (!strncmp(p->name, name, len) && !p->name[len])\n> > +\t\t\treturn p;\n> > +\treturn NULL;\n> > +}\n> \n> Makes me wonder if we want to have a generic table that is keyed by name\n> whose contents can be looked up by counted string. string_list is the\n> closest thing we already have, but I do not think it has counted string\n> interface (shouldn't be a rocket surgery to add it, though).\n\nI don't know that it would actually make this code significantly clearer\nor more efficient. If it were a sorted array, one could do a binary\nsearch, but we are really talking about a handful of elements (if you\ndid want to refactor, this is almost identical to the matching code in\nuserdiff, too).\n\n> > +static int tar_filter_config(const char *var, const char *value, void *data)\n> > +{\n> > ...\n> > +\tif (!strcmp(type, \"command\")) {\n> > +\t\tif (!value)\n> > +\t\t\treturn config_error_nonbool(var);\n> > +\t\ttf->command = xstrdup(value);\n> \n> Does this result in small leak if the same filter is multiply defined, say\n> in /etc/gitconfig and then in ~/.gitconfig?\n\nYeah, it does. My original version had the builtin gzip statically\nallocated, and it wasn't safe to free() anything. But I ended up having\nto allocate it dynamically anyway because of the variable-sized list of\nextensions, so it would be safe to free(tf->command) here. I'll do that\nin my re-roll.\n\n> > +struct tar_filter {\n> > +\tchar *name;\n> > +\tchar *command;\n> > +\tstruct string_list extensions;\n> > +\tunsigned use_compression:1;\n> \n> I suspect that you plan to pass sprintf(\"-%d\", level) for the ones marked\n> with this bit, but I wonder if we want to give a bit more control on how a\n> compression level option is shaped for the particular command, and where\n> on the command line the option comes. As long as we are targetting gzip\n> and nothing else it is fine, and I suspect newer compression commands\n> would try to mimic the -[0-9] command line interface gzip has (e.g. xz),\n> so this probably is not an issue in practice.\n\nYeah, I assumed everyone who would want this would support -[0-9]. After\nall, all the flag is doing is passing -[0-9] that was supplied to\ngit-archive. We could allow something like:\n\n [tarfilter \"gzip\"]\n command = gzip %(compression)\n\nbut I don't see much point. Either you want it or you don't. If there is\na complex mapping of those numbers to some other options in your\ncommand, then point git to a helper script which does the conversion\nand then execs your command.\n\nIf somebody has a counterexample, I'd be curious to hear it.\n\n-Peff\n"},{"id":"170108","messageId":"20110616003400.GB20355@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7vmxhiirlb.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/7] archive: refactor format-guessing from filename","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-16T00:34:00Z","receivedAt":"2011-06-16T00:34:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2011 at 04:48:32PM -0700, Junio C Hamano wrote:\n\n> > will produce a gzip'd file. If we make the mapping decision\n> > locally, then running:\n> >\n> > git archive --remote=origin -o file.tar.gz HEAD\n> >\n> > will send \"--format=tgz\" to the remote side. The server will\n> > complain, saying that it doesn't know about the tgz format.\n> \n> As long as that complaint is clearly marked as coming from the remote\n> side, the user now knows that tgz is not supported, and can fall back to a\n> plain tar.\n\nIt is. You can simulate with:\n\n $ git archive --format=foobar --remote=. HEAD\n remote: fatal: Unknown archive format 'foobar'\n remote: git upload-archive: archiver died with error\n fatal: sent error to the client: git upload-archive: archiver died with error\n\nOr if you want to be really thorough and you have an old git lying\naround, you can see the format auto-selection triggers the same code:\n\n $ git archive -o foo.tar.gz --remote=. --exec='git.v1.7.5 upload-archive' HEAD\n remote: fatal: Unknown archive format 'tgz'\n fatal: sent error to the client: git upload-archive: archiver died with error\n remote: git upload-archive: archiver died with error\n\n> Am I being naïve thinking that barfing (and assuming that the user\n> understands why the remote end barfed) actually is a good thing?\n\nNo, I think barfing is totally fine there. I worry more about the cases\nwhere we silently produce a format the user was not expecting (the way\nmine is coded, it is \"the server knows about .tar.gz, but the client\ndoes not; I expect gzip, but I get regular tar\").\n\n-Peff\n"},{"id":"170109","messageId":"20110616003800.GC20355@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7v7h8mir5u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-16T00:38:00Z","receivedAt":"2011-06-16T00:38:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2011 at 04:55:57PM -0700, Junio C Hamano wrote:\n\n> > +test_expect_success 'infer tgz from .tgz filename' '\n> > +\tgit archive --output=j1.tgz HEAD &&\n> > +\ttest_cmp j.tgz j1.tgz\n> > +'\n> \n> I suspect this would get intermittent failures for the same reason as\n> 0c8c385 (gitweb: supply '-n' to gzip for identical output, 2011-04-26)\n\nIck, yeah. I pulled these tests from my original internal\nimplementation, which I suspect may have been more stable.\n\nThe filename will always be stdin, which is OK, but the timestamp will\nprobably get us.\n\n> diff --git a/archive-tar-filter.c b/archive-tar-filter.c\n> index de8719a..d6e4e32 100644\n> --- a/archive-tar-filter.c\n> +++ b/archive-tar-filter.c\n> @@ -131,7 +131,7 @@ static void load_builtin_filters(void)\n> \tstruct tar_filter *tf;\n> \n> \ttf = tar_filter_new(\"tgz\", strlen(\"tgz\"));\n> -\ttf->command = xstrdup(\"gzip\");\n> +\ttf->command = xstrdup(\"gzip -n\");\n> \tstring_list_append(&tf->extensions, \"tgz\");\n> \tstring_list_append(&tf->extensions, \"tar.gz\");\n> \ttf->use_compression = 1;\n\nThis feels a little wrong, as we are changing what the tool outputs all\nthe time just to appease a poorly-written test. Maybe nobody cares about\nthe timestamp field (I certainly don't), but it seems like it might\nsurprise some users.\n\n-Peff\n"},{"id":"170113","messageId":"7v39jai94h.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"20110616003800.GC20355@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-16T06:27:26Z","receivedAt":"2011-06-16T06:27:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> This feels a little wrong, as we are changing what the tool outputs all\n> the time just to appease a poorly-written test.\n\nNow you confused me. Isn't '-n' to tell the tool *not* to write timestamp\nout, so that we can avoid changing what the tool outputs all the time?\n"},{"id":"170114","messageId":"20110616065146.GA30672@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7v39jai94h.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-16T06:51:46Z","receivedAt":"2011-06-16T06:51:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 15, 2011 at 11:27:26PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > This feels a little wrong, as we are changing what the tool outputs all\n> > the time just to appease a poorly-written test.\n> \n> Now you confused me. Isn't '-n' to tell the tool *not* to write timestamp\n> out, so that we can avoid changing what the tool outputs all the time?\n\nNo, I mean that people may _want_ the timestamp in day to day use. Using\n\"-n\" all the time suppresses it. And there is no reason to suppress it,\nexcept that our test does not account for it properly. So your patch is\nhurting people who don't want \"-n\" (i.e., want the timestamp) just to\nmake our test happy.\n\n-Peff\n"},{"id":"170117","messageId":"20110616075621.GA12413@arachsys.com","threadId":"27625","inReplyTo":"20110616065146.GA30672@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Chris Webb","fromEmail":"chris@arachsys.com","sentAt":"2011-06-16T07:56:21Z","receivedAt":"2011-06-16T07:56:21Z","isPatch":true,"sender":{"key":"chris@arachsys.com","avatar":"https://avatars.githubusercontent.com/u/299056?v=4"},"body":"Jeff King writes:\n\n> No, I mean that people may _want_ the timestamp in day to day use. Using\n> \"-n\" all the time suppresses it. And there is no reason to suppress it,\n> except that our test does not account for it properly. So your patch is\n> hurting people who don't want \"-n\" (i.e., want the timestamp) just to\n> make our test happy.\n\nIt's useful to omit the timestamp outside of git too. Source-based package\nmanagement systems generally store a URL from which to fetch a source\ntarball, and a hash of that source tarball to ensure it hasn't been tampered\nwith. It's nice to be able to use a gitweb URL like\n\n http://git.kernel.org/?p=git/git.git;a=snapshot;h=e5af0de202e885b793482d416b8ce9d50dd2b8bc;sf=tgz\n\nas the tarball source, and still be able to verify its integrity against a\nprestored hash.\n\nCheers,\n\nChris.\n"},{"id":"170138","messageId":"20110616174653.GD6584@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110616075621.GA12413@arachsys.com","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-16T17:46:53Z","receivedAt":"2011-06-16T17:46:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 16, 2011 at 08:56:21AM +0100, Chris Webb wrote:\n\n> Jeff King writes:\n> \n> > No, I mean that people may _want_ the timestamp in day to day use. Using\n> > \"-n\" all the time suppresses it. And there is no reason to suppress it,\n> > except that our test does not account for it properly. So your patch is\n> > hurting people who don't want \"-n\" (i.e., want the timestamp) just to\n> > make our test happy.\n> \n> It's useful to omit the timestamp outside of git too. Source-based package\n> management systems generally store a URL from which to fetch a source\n> tarball, and a hash of that source tarball to ensure it hasn't been tampered\n> with. It's nice to be able to use a gitweb URL like\n> \n> http://git.kernel.org/?p=git/git.git;a=snapshot;h=e5af0de202e885b793482d416b8ce9d50dd2b8bc;sf=tgz\n> \n> as the tarball source, and still be able to verify its integrity against a\n> prestored hash.\n\nOK. I'm totally willing to accept that people actually prefer the \"-n\"\nbehavior. I don't care either way myself. I just don't want the reason\nto default to \"-n\" to be \"because our test scripts need it\" and not\n\"because this is what people actually want\".\n\n-Peff\n"},{"id":"170140","messageId":"7vtybphcym.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"20110616174653.GD6584@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-16T18:02:09Z","receivedAt":"2011-06-16T18:02:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> OK. I'm totally willing to accept that people actually prefer the \"-n\"\n> behavior. I don't care either way myself. I just don't want the reason\n> to default to \"-n\" to be \"because our test scripts need it\" and not\n> \"because this is what people actually want\".\n\nSurely I share the exact feeling, and that is why I quoted the other \"-n\"\nadded to gitweb because that was what people actually wanted.\n"},{"id":"170144","messageId":"20110616182149.GB12689@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7vtybphcym.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-16T18:21:50Z","receivedAt":"2011-06-16T18:21:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 16, 2011 at 11:02:09AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > OK. I'm totally willing to accept that people actually prefer the \"-n\"\n> > behavior. I don't care either way myself. I just don't want the reason\n> > to default to \"-n\" to be \"because our test scripts need it\" and not\n> > \"because this is what people actually want\".\n> \n> Surely I share the exact feeling, and that is why I quoted the other \"-n\"\n> added to gitweb because that was what people actually wanted.\n\nFair enough. I'll use \"gzip -n\" in my re-roll.\n\nAny comment on the \"tarfilter\" versus \"generic archive filter\" issue, or\non the general interface? I think getting that right is my biggest issue\nin moving forward.\n\nAlso, since it's easy via the external helper route, should there be any\nother builtin formats? Bzip2? It's not that big a deal for a big hosting\nsite like kernel.org to stick it in their configuration, but I wonder if\nnormal users would find it useful.\n\n-Peff\n"},{"id":"170145","messageId":"BANLkTikv+3G5isAGECTm=YJjzvQkmZZvKw@mail.gmail.com","threadId":"27625","inReplyTo":"20110616182149.GB12689@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"John Szakmeister","fromEmail":"john@szakmeister.net","sentAt":"2011-06-16T18:27:27Z","receivedAt":"2011-06-16T18:27:27Z","isPatch":true,"sender":{"key":"john@szakmeister.net","avatar":"https://avatars.githubusercontent.com/u/448087?v=4"},"body":"On Thu, Jun 16, 2011 at 2:21 PM, Jeff King wrote:\n[snip]\n> Also, since it's easy via the external helper route, should there be any\n> other builtin formats? Bzip2? It's not that big a deal for a big hosting\n> site like kernel.org to stick it in their configuration, but I wonder if\n> normal users would find it useful.\n\nI'd certainly find it useful.\n\n-John\n"},{"id":"170147","messageId":"7vpqmdhb3q.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"20110616182149.GB12689@sigill.intra.peff.net","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-16T18:42:17Z","receivedAt":"2011-06-16T18:42:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Also, since it's easy via the external helper route, should there be any\n> other builtin formats? Bzip2? It's not that big a deal for a big hosting\n> site like kernel.org to stick it in their configuration, but I wonder if\n> normal users would find it useful.\n\nI know k.org statically prepares *.bz2 for any *.gz in /pub/software/\nhierarchy, but isn't bzip2 significantly more expensive (like taking ten\ntimes as much memory or four times as much CPI time to squeeze the last\nextra 15% out), making it unsuitable for one-shot online use?\n"},{"id":"170148","messageId":"20110616185739.GA13616@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7vpqmdhb3q.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 7/7] archive: provide builtin .tar.gz filter","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-16T18:57:39Z","receivedAt":"2011-06-16T18:57:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 16, 2011 at 11:42:17AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Also, since it's easy via the external helper route, should there be any\n> > other builtin formats? Bzip2? It's not that big a deal for a big hosting\n> > site like kernel.org to stick it in their configuration, but I wonder if\n> > normal users would find it useful.\n> \n> I know k.org statically prepares *.bz2 for any *.gz in /pub/software/\n> hierarchy, but isn't bzip2 significantly more expensive (like taking ten\n> times as much memory or four times as much CPI time to squeeze the last\n> extra 15% out), making it unsuitable for one-shot online use?\n\nI will let J.H. comment on how appropriate that is for k.org; he is the\none who mentioned bzip2 in the first place earlier in the thread.\n\nEven if kernel.org wants it, it is a departure from what we now, though.\nPeople who set up remote upload-archive access long ago and upgrade git\nwould now suddenly let anyone convince their machines to chew up a lot\nof CPU. I don't mind too much doing it for gzip, which takes roughly the\nsame amount of time as zip, which already exists. Silently adding other\nformats may be worse, though.\n\nAt the same time, it would be nice for people running git-archive\nlocally to avoid having to configure a bzip2 filter manually. Maybe\nupload-archive should ignore these filters by default, and require a\nspecial config variable to enable them?\n\n-Peff\n"},{"id":"170202","messageId":"4DFCBB92.5040308@lsrfire.ath.cx","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"Re: [RFC/PATCH 0/7] user-configurable git-archive output formats","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-06-18T14:52:02Z","receivedAt":"2011-06-18T14:52:02Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 16.06.2011 00:30, schrieb Jeff King:\n> On Tue, Jun 14, 2011 at 04:45:21PM -0400, Jeff King wrote:\n> \n>> The gzip path is not configurable at all. Probably it should read the\n>> path and arguments from the config file. In fact, we could even allow\n>> arbitrary config like:\n>>\n>> [tarfilter \"tgz\"]\n>> command = gzip -c\n>> extension = tgz\n>> extension = tar.gz\n\nConfiguration options whose values are appended instead of overwritten\nby duplicate definitions are a new concept for git, I think. Perhaps\nit's not a big thing, but I think it's better avoided.\n\nThe only (stupid) practical shortcoming I can think if is this, though:\nYou can't remove anything from the list of supported extensions in a\nuser config if the system config already contains e.g. tgz and tar.gz.\n\n> Here's a series implementing that. You can configure whatever you want,\n> and it includes builtin gzip configuration by default. You can override\n> to turn it off, or even switch it to run something like pigz instead.\n> \n> My biggest reservation with the patches as-is is that they are very\n> tar-centric and not orthogonal. Specifically, they won't handle:\n> \n> 1. Other streamable archive formats you would want to pipe through\n> compressors. Do any of these actually exist? I guess we could offer\n> \"pax\" as a format eventually, and it might be like tar with\n> different defaults? I dunno.\n> \n> Fixing this would not be too hard. Instead of these being\n> \"tarfilters\", they would be \"archive filters\", and they would chain\n> to some format, defaulting to \"tar\". Since there is no other\n> format right now, we could even punt on writing most of the code\n> until somebody adds one. But we would want to get the naming of the\n> config options right, since those are user-facing. Maybe\n> \"archivefilter\" (unfortunately the more readable archive.filter is\n> a little awkward with the way we parse config files)?\n\nThe pax format is identical to the ustar format, which --format=tar\nproduces. The other major format that comes to mind is cpio. The\n(never merged) predecessor of tar-tree actually used that format.\n\nSince then I have been waiting for users to request being able to export\nusing cpio format (which is simpler and slightly smaller than tar), but\nthat never happened. It seems the existence of the pax format really\nhas pacified the tar vs. cpio war of old.\n\nI'm not sure \"filter\" is a good name, though. We have core.pager, which\nis technically a filter as well, but for a specific purpose. And we\nhave the tar.umask setting as a precedence for format specfic config\noptions. So how about tar..compressor?\n\n\t[tar \"tgz\"]\n\t\tcompressor = gzip -cn\n\t[tar \"tar.gz\"]\n\t\tcompressor = gzip -cn\n\t[tar \"tar.bz2\"]\n\t\tcompressor = bzip2 -c\n\nWe don't need a compressionlevels option here because we can simply\nassume that the compressor commands do support them. (Side note: this\nis not fully true for bzip2, as it doesn't support -0, but I don't think\nthis is worth special consideration in our code, as long as errors of\nthe filter are displayed properly.)\n\nAnd we can also add a config option to restrict the formats creatable by\nupload-archive, to address concerns over DoS attacks with expensive\ncompressors:\n\n\t[archive]\n\t\tremoteFormats = tar zip tgz tar.gz\n"},{"id":"170204","messageId":"m3wrgjtb1c.fsf@localhost.localdomain","threadId":"27625","inReplyTo":"4DFCBB92.5040308@lsrfire.ath.cx","subject":"Re: [RFC/PATCH 0/7] user-configurable git-archive output formats","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-06-18T15:28:48Z","receivedAt":"2011-06-18T15:28:48Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"René Scharfe writes:\n> Am 16.06.2011 00:30, schrieb Jeff King:\n>> On Tue, Jun 14, 2011 at 04:45:21PM -0400, Jeff King wrote:\n>> \n>>> The gzip path is not configurable at all. Probably it should read the\n>>> path and arguments from the config file. In fact, we could even allow\n>>> arbitrary config like:\n>>>\n>>> [tarfilter \"tgz\"]\n>>> command = gzip -c\n>>> extension = tgz\n>>> extension = tar.gz\n> \n> Configuration options whose values are appended instead of overwritten\n> by duplicate definitions are a new concept for git, I think. Perhaps\n> it's not a big thing, but I think it's better avoided.\n\nActually they are not a new concept, but they are quite rare. `git config`\nhas even special options to deal with them ('--replace-all', '--add', \n'--get-all', '--unset-all'.\n\nFor example \"core.gitProxy\" can be set multiple times, and of course\n\"remote..fetch\", 'pull' and 'url'.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"170205","messageId":"4DFCC6FF.8020801@lsrfire.ath.cx","threadId":"27625","inReplyTo":"20110615223030.GA16110@sigill.intra.peff.net","subject":"Re: [RFC/PATCH 0/7] user-configurable git-archive output formats","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-06-18T15:40:47Z","receivedAt":"2011-06-18T15:40:47Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 16.06.2011 00:30, schrieb Jeff King:\n> 2. In theory you might want to plug in external helpers that are not\n> just stream filters, but actually their own container formats (like\n> zip). I think people who want 7zip would want this.\n> \n> But how does git-archive interact with the helper? By definition\n> the data it wants is the set of files, not a single stream. So\n> either:\n> \n> a. We give the helper a temporary exported checkout, and it\n> generates the stream from that.\n> \n> b. We use tar as the lingua franca of streaming file containers,\n> and let the helper deal with converting to its preferred\n> output format.\n> \n> Option (a) seems horribly inefficient on disk I/O. And if we did\n> want to do that, I think it's largely unrelated to this patch\n> series.\n> \n> You can actually do option (b) with this series. In its worst\n> case, you can do the same as (a): just untar into a temporary\n> directory and compress from there. But a well-written helper could\n> convert tar into the output format on the fly.\n\nBoth can be done today, locally. One just needs to add a tar-to-7z/any\nprogram for 2.b). :)\n\nCurrently the easiest way to apply LZMA compression would be to pipe\n--format=tar through the xz utils. This method is supported by your\npatches and the result can be read by 7-Zip. It should be good enough\nfor most uses; I think we can disregard the whole point 2 until users\nstart to ask for these other formats or use cases.\n"},{"id":"170306","messageId":"7vsjr4cx5a.fsf@alter.siamese.dyndns.org","threadId":"27625","inReplyTo":"4DFCBB92.5040308@lsrfire.ath.cx","subject":"Re: [RFC/PATCH 0/7] user-configurable git-archive output formats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-06-20T15:58:41Z","receivedAt":"2011-06-20T15:58:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> I'm not sure \"filter\" is a good name, though. We have core.pager, which\n> is technically a filter as well, but for a specific purpose. And we\n> have the tar.umask setting as a precedence for format specfic config\n> options. So how about tar..compressor?\n>\n> \t[tar \"tgz\"]\n> \t\tcompressor = gzip -cn\n> \t[tar \"tar.gz\"]\n> \t\tcompressor = gzip -cn\n> \t[tar \"tar.bz2\"]\n> \t\tcompressor = bzip2 -c\n>\n> We don't need a compressionlevels option here because we can simply\n> assume that the compressor commands do support them. (Side note: this\n> is not fully true for bzip2, as it doesn't support -0, but I don't think\n> this is worth special consideration in our code, as long as errors of\n> the filter are displayed properly.)\n>\n> And we can also add a config option to restrict the formats creatable by\n> upload-archive, to address concerns over DoS attacks with expensive\n> compressors:\n>\n> \t[archive]\n> \t\tremoteFormats = tar zip tgz tar.gz\n\nBoth sounds sensible, I think.\n"},{"id":"170393","messageId":"20110621160159.GA17334@sigill.intra.peff.net","threadId":"27625","inReplyTo":"4DFCBB92.5040308@lsrfire.ath.cx","subject":"Re: [RFC/PATCH 0/7] user-configurable git-archive output formats","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-21T16:01:59Z","receivedAt":"2011-06-21T16:01:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Jun 18, 2011 at 04:52:02PM +0200, René Scharfe wrote:\n\n> >> The gzip path is not configurable at all. Probably it should read the\n> >> path and arguments from the config file. In fact, we could even allow\n> >> arbitrary config like:\n> >>\n> >> [tarfilter \"tgz\"]\n> >> command = gzip -c\n> >> extension = tgz\n> >> extension = tar.gz\n> \n> Configuration options whose values are appended instead of overwritten\n> by duplicate definitions are a new concept for git, I think. Perhaps\n> it's not a big thing, but I think it's better avoided.\n> \n> The only (stupid) practical shortcoming I can think if is this, though:\n> You can't remove anything from the list of supported extensions in a\n> user config if the system config already contains e.g. tgz and tar.gz.\n\nYeah, I have mixed feelings on that.\n\nAs Jakub pointed out, we already have them in several places. I don't\nknow that removal is that big a deal in this instance. If we did want to\nsupport it, I think it would make more sense to have a generic solution\nat the config level, like:\n\n [some-section]\n multivalue = foo\n multivalue = bar\n !multivalue\n multivalue = baz\n multivalue = whee\n\nat which point the value is (\"baz\", \"whee\"). That matches what we do on\nthe command line, where:\n\n git foo --multivalue=foo --multivalue=bar --no-multivalue \\\n --multivalue=baz --multivalue=whee\n\nhandles the same issue in a similar way.\n\nThe other option, of course, is having a single value with list\nsemantics. But then you have to invent separator syntax. In this\ninstance whitespace would probably be fine, but I'd rather that each new\nmulti-valued option did not invent its own syntax, and in the general\ncase you may need to handle quoting. Plus you may need some kind of\nappend syntax. For example, if we support \"tgz\" and \"tar.gz\" internally,\nhow do you say 'add \"pax.gz\"' to that list without reiterating the whole\nlist?\n\n> The pax format is identical to the ustar format, which --format=tar\n> produces. The other major format that comes to mind is cpio. The\n> (never merged) predecessor of tar-tree actually used that format.\n\nThanks, cpio is probably the most likely example.\n\n> Since then I have been waiting for users to request being able to export\n> using cpio format (which is simpler and slightly smaller than tar), but\n> that never happened. It seems the existence of the pax format really\n> has pacified the tar vs. cpio war of old.\n\nFair enough. I haven't heard anybody clamoring for it either. I just\ndidn't want to paint us into a corner. Since it seems like the most\nlikely format and nobody really wants it, it's perhaps not worth\nworrying about.\n\n> I'm not sure \"filter\" is a good name, though. We have core.pager, which\n> is technically a filter as well, but for a specific purpose.\n\nYeah, any name would have to be \"archive filter\" or similar. But I would\nthink being under the \"tar\" section would be enough to disambiguate it.\n\n> And we have the tar.umask setting as a precedence for format specfic\n> config options. So how about tar..compressor?\n> \n> \t[tar \"tgz\"]\n> \t\tcompressor = gzip -cn\n> \t[tar \"tar.gz\"]\n> \t\tcompressor = gzip -cn\n> \t[tar \"tar.bz2\"]\n> \t\tcompressor = bzip2 -c\n\nMy two complaints are:\n\n 1. The user has to repeat themselves in describing the command for\n multiple extensions. In practice, that's probably not a big deal,\n though.\n\n 2. The namespace for user-defined extensions is the same as the\n namespace for tar options. I guess we can disambiguate based on the\n number of dots (so, e.g., I know that \"tar.umask\" is not the umask\n extension, because it doesn't have a third component). It does\n limit us a little bit for adding future options.\n\n I don't know if it's worth caring about. We have the same problem\n with the diff.* namespace (e.g., diff.color.* exists, but is not a\n userdiff driver). In that case, besides the code being a little\n careful to be tolerant of the clash, I don't think it has been a\n problem.\n\n> We don't need a compressionlevels option here because we can simply\n> assume that the compressor commands do support them.\n\nBut we discussed elsewhere the concept of a tar-to-7z filter. I'm not\nsure I'd call that a \"compressor\" as much as a filter. And it wouldn't\nwant the compression-level options (or maybe you would; I don't use it,\nbut skimming the manpage, it looks like you would want to convert -5\ninto \"-mx=5\"; so maybe you would want a wrapper script anyway).\n\n> (Side note: this is not fully true for bzip2, as it doesn't support\n> -0, but I don't think this is worth special consideration in our code,\n> as long as errors of the filter are displayed properly.)\n\nYeah, I think that can be ignored. bzip can take care of complaining\nitself.\n\n> And we can also add a config option to restrict the formats creatable by\n> upload-archive, to address concerns over DoS attacks with expensive\n> compressors:\n> \n> \t[archive]\n> \t\tremoteFormats = tar zip tgz tar.gz\n\nRight. It does have the ad-hoc list syntax I complained about above,\nthough.\n\n-Peff\n"},{"id":"170431","messageId":"20110622011923.GA30370@sigill.intra.peff.net","threadId":"27625","inReplyTo":"7vsjr4cx5a.fsf@alter.siamese.dyndns.org","subject":"[PATCHv2 0/9] configurable tar compressors","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-22T01:19:23Z","receivedAt":"2011-06-22T01:19:23Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Here's my re-roll. In addition to integrating comments on the list, I\nmade the integration with the archive code a little smoother. Filters\nnow appear as their own \"struct archiver\" in the list, and don't have to\nbe special-cased everywhere.\n\n [1/9]: archive: reorder option parsing and config reading\n [2/9]: archive-tar: don't reload default config options\n [3/9]: archive: refactor list of archive formats\n [4/9]: archive: pass archiver struct to write_archive callback\n [5/9]: archive: move file extension format-guessing lower\n [6/9]: archive: refactor file extension format-guessing\n [7/9]: archive: implement configurable tar filters\n [8/9]: archive: provide builtin .tar.gz filter\n [9/9]: upload-archive: allow user to turn off filters\n\n-Peff\n"},{"id":"170432","messageId":"20110622012044.GA30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 1/9] archive: reorder option parsing and config reading","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:20:44Z","receivedAt":"2011-06-22T01:20:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The archive command does three things during its\ninitialization phase:\n\n 1. parse command-line options\n\n 2. setup the git directory\n\n 3. read config\n\nDuring phase (1), if we see any options that do not require\na git directory (like \"--list\"), we handle them immediately\nand exit, making it safe to abort step (2) if we are not in\na git directory.\n\nStep (3) must come after step (2), since the git directory\nmay influence configuration. However, this leaves no\npossibility of configuration from step (3) impacting the\ncommand-line options in step (1) (which is useful, for\nexample, for supporting user-configurable output formats).\n\nInstead, let's reorder this to:\n\n 1. setup the git directory, if it exists\n\n 2. read config\n\n 3. parse command-line options\n\n 4. if we are not in a git repository, die\n\nThis should have the same external behavior, but puts\nconfiguration before command-line parsing.\n\nSigned-off-by: Jeff King \n---\nSame as v1.\n\n archive.c | 18 ++++++++++++++----\n 1 files changed, 14 insertions(+), 4 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 42f2d2f..2616676 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -387,17 +387,27 @@ static int parse_archive_args(int argc, const char **argv,\n int write_archive(int argc, const char **argv, const char *prefix,\n \t\tint setup_prefix)\n {\n+\tint nongit = 0;\n \tconst struct archiver *ar = NULL;\n \tstruct archiver_args args;\n \n-\targc = parse_archive_args(argc, argv, &ar, &args);\n \tif (setup_prefix && prefix == NULL)\n-\t\tprefix = setup_git_directory();\n+\t\tprefix = setup_git_directory_gently(&nongit);\n+\n+\tgit_config(git_default_config, NULL);\n+\n+\targc = parse_archive_args(argc, argv, &ar, &args);\n+\tif (nongit) {\n+\t\t/*\n+\t\t * We know this will die() with an error, so we could just\n+\t\t * die ourselves; but its error message will be more specific\n+\t\t * than what we could write here.\n+\t\t */\n+\t\tsetup_git_directory();\n+\t}\n \n \tparse_treeish_arg(argv, &args, prefix);\n \tparse_pathspec_arg(argv + 1, &args);\n \n-\tgit_config(git_default_config, NULL);\n-\n \treturn ar->write_archive(&args);\n }\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170433","messageId":"20110622012220.GB30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 2/9] archive-tar: don't reload default config options","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:22:20Z","receivedAt":"2011-06-22T01:22:20Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We load our own tar-specific config, and then chain to\ngit_default_config. This is pointless, as our caller should\nalready have loaded the default config. It also introduces a\nneedless inconsistency with the zip archiver, which does not\nlook at the config files at all (and therefore relies on the\ncaller to have loaded config).\n\nSigned-off-by: Jeff King \n---\nLast time the tar-filter code was not integrated with the tar code.\nSince we are now under tar.*, and since the tar code reads the config\nanyway, it makes sense for us to use that config callback. This was just\na little nit I noticed while changing it.\n\n archive-tar.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex cee06ce..1ab1a2c 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -231,7 +231,7 @@ static int git_tar_config(const char *var, const char *value, void *cb)\n \t\t}\n \t\treturn 0;\n \t}\n-\treturn git_default_config(var, value, cb);\n+\treturn 0;\n }\n \n int write_tar_archive(struct archiver_args *args)\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170434","messageId":"20110622012333.GC30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 3/9] archive: refactor list of archive formats","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:23:33Z","receivedAt":"2011-06-22T01:23:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Most of the tar and zip code was nicely split out into two\nabstracted files which knew only about their specific\nformats. The entry point to this code was a single \"write\narchive\" function.\n\nHowever, as these basic formats grow more complex (e.g., by\nhandling multiple file extensions and format names), a\nstatic list of the entry point functions won't be enough.\nInstead, let's provide a way for the tar and zip code to\ntell the main archive code what they support by registering\narchiver names and functions.\n\nSigned-off-by: Jeff King \n---\nNew in v2. This turns archivers more into proper objects, rather than a\nhard-coded list of functions, and makes the rest of the series much\ncleaner.\n\n archive-tar.c | 16 +++++++++++++---\n archive-zip.c | 13 ++++++++++++-\n archive.c | 33 +++++++++++++++++----------------\n archive.h | 17 ++++++++++-------\n 4 files changed, 52 insertions(+), 27 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 1ab1a2c..930375b 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -234,12 +234,10 @@ static int git_tar_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n-int write_tar_archive(struct archiver_args *args)\n+static int write_tar_archive(struct archiver_args *args)\n {\n \tint err = 0;\n \n-\tgit_config(git_tar_config, NULL);\n-\n \tif (args->commit_sha1)\n \t\terr = write_global_extended_header(args);\n \tif (!err)\n@@ -248,3 +246,15 @@ int write_tar_archive(struct archiver_args *args)\n \t\twrite_trailer();\n \treturn err;\n }\n+\n+static struct archiver tar_archiver = {\n+\t\"tar\",\n+\twrite_tar_archive,\n+\t0\n+};\n+\n+void init_tar_archiver(void)\n+{\n+\tregister_archiver(&tar_archiver);\n+\tgit_config(git_tar_config, NULL);\n+}\ndiff --git a/archive-zip.c b/archive-zip.c\nindex cf28504..a776d83 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -261,7 +261,7 @@ static void dos_time(time_t *time, int *dos_date, int *dos_time)\n \t*dos_time = t->tm_sec / 2 + t->tm_min * 32 + t->tm_hour * 2048;\n }\n \n-int write_zip_archive(struct archiver_args *args)\n+static int write_zip_archive(struct archiver_args *args)\n {\n \tint err;\n \n@@ -278,3 +278,14 @@ int write_zip_archive(struct archiver_args *args)\n \n \treturn err;\n }\n+\n+static struct archiver zip_archiver = {\n+\t\"zip\",\n+\twrite_zip_archive,\n+\tARCHIVER_WANT_COMPRESSION_LEVELS\n+};\n+\n+void init_zip_archiver(void)\n+{\n+\tregister_archiver(&zip_archiver);\n+}\ndiff --git a/archive.c b/archive.c\nindex 2616676..f0b4e85 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -14,16 +14,15 @@ static char const * const archive_usage[] = {\n \tNULL\n };\n \n-#define USES_ZLIB_COMPRESSION 1\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{ \"zip\", write_zip_archive, USES_ZLIB_COMPRESSION },\n-};\n+static const struct archiver **archivers;\n+static int nr_archivers;\n+static int alloc_archivers;\n+\n+void register_archiver(struct archiver *ar)\n+{\n+\tALLOC_GROW(archivers, nr_archivers + 1, alloc_archivers);\n+\tarchivers[nr_archivers++] = ar;\n+}\n \n static void format_subst(const struct commit *commit,\n const char *src, size_t len,\n@@ -208,9 +207,9 @@ static const struct archiver *lookup_archiver(const char *name)\n \tif (!name)\n \t\treturn NULL;\n \n-\tfor (i = 0; i < ARRAY_SIZE(archivers); i++) {\n-\t\tif (!strcmp(name, archivers[i].name))\n-\t\t\treturn &archivers[i];\n+\tfor (i = 0; i < nr_archivers; i++) {\n+\t\tif (!strcmp(name, archivers[i]->name))\n+\t\t\treturn archivers[i];\n \t}\n \treturn NULL;\n }\n@@ -355,8 +354,8 @@ static int parse_archive_args(int argc, const char **argv,\n \t\tbase = \"\";\n \n \tif (list) {\n-\t\tfor (i = 0; i < ARRAY_SIZE(archivers); i++)\n-\t\t\tprintf(\"%s\\n\", archivers[i].name);\n+\t\tfor (i = 0; i < nr_archivers; i++)\n+\t\t\tprintf(\"%s\\n\", archivers[i]->name);\n \t\texit(0);\n \t}\n \n@@ -369,7 +368,7 @@ static int parse_archive_args(int argc, const char **argv,\n \n \targs->compression_level = Z_DEFAULT_COMPRESSION;\n \tif (compression_level != -1) {\n-\t\tif ((*ar)->flags & USES_ZLIB_COMPRESSION)\n+\t\tif ((*ar)->flags & ARCHIVER_WANT_COMPRESSION_LEVELS)\n \t\t\targs->compression_level = compression_level;\n \t\telse {\n \t\t\tdie(\"Argument not supported for format '%s': -%d\",\n@@ -395,6 +394,8 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \t\tprefix = setup_git_directory_gently(&nongit);\n \n \tgit_config(git_default_config, NULL);\n+\tinit_tar_archiver();\n+\tinit_zip_archiver();\n \n \targc = parse_archive_args(argc, argv, &ar, &args);\n \tif (nongit) {\ndiff --git a/archive.h b/archive.h\nindex 038ac35..f39cede 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -14,15 +14,18 @@ struct archiver_args {\n \tint compression_level;\n };\n \n-typedef int (*write_archive_fn_t)(struct archiver_args *);\n+#define ARCHIVER_WANT_COMPRESSION_LEVELS 1\n+struct archiver {\n+\tconst char *name;\n+\tint (*write_archive)(struct archiver_args *);\n+\tunsigned flags;\n+};\n+extern void register_archiver(struct archiver *);\n \n-typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);\n+extern void init_tar_archiver(void);\n+extern void init_zip_archiver(void);\n \n-/*\n- * Archive-format specific backends.\n- */\n-extern int write_tar_archive(struct archiver_args *);\n-extern int write_zip_archive(struct archiver_args *);\n+typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170435","messageId":"20110622012407.GD30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 4/9] archive: pass archiver struct to write_archive callback","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:24:07Z","receivedAt":"2011-06-22T01:24:07Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The current archivers are very static; when you are in the\nwrite_tar_archive function, you know you are writing a tar.\nHowever, to facilitate runtime-configurable archivers\nthat will share a common write function we need to tell the\nfunction which archiver was used.\n\nAs a convenience, we also provide an opaque data pointer in\nthe archiver struct so that individual archivers can put\nsomething useful there when they register themselves.\nTechnically they could just use the \"name\" field to look in\nan internal map of names to data, but this is much simpler.\n\nSigned-off-by: Jeff King \n---\nNew in v2; before there was magic special-casing of the tar_filter code.\n\n archive-tar.c | 3 ++-\n archive-zip.c | 3 ++-\n archive.c | 2 +-\n archive.h | 3 ++-\n 4 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 930375b..bed9a9b 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -234,7 +234,8 @@ static int git_tar_config(const char *var, const char *value, void *cb)\n \treturn 0;\n }\n \n-static int write_tar_archive(struct archiver_args *args)\n+static int write_tar_archive(const struct archiver *ar,\n+\t\t\t struct archiver_args *args)\n {\n \tint err = 0;\n \ndiff --git a/archive-zip.c b/archive-zip.c\nindex a776d83..42df660 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -261,7 +261,8 @@ static void dos_time(time_t *time, int *dos_date, int *dos_time)\n \t*dos_time = t->tm_sec / 2 + t->tm_min * 32 + t->tm_hour * 2048;\n }\n \n-static int write_zip_archive(struct archiver_args *args)\n+static int write_zip_archive(const struct archiver *ar,\n+\t\t\t struct archiver_args *args)\n {\n \tint err;\n \ndiff --git a/archive.c b/archive.c\nindex f0b4e85..a0a5beb 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -410,5 +410,5 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tparse_treeish_arg(argv, &args, prefix);\n \tparse_pathspec_arg(argv + 1, &args);\n \n-\treturn ar->write_archive(&args);\n+\treturn ar->write_archive(ar, &args);\n }\ndiff --git a/archive.h b/archive.h\nindex f39cede..b3cf219 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -17,8 +17,9 @@ struct archiver_args {\n #define ARCHIVER_WANT_COMPRESSION_LEVELS 1\n struct archiver {\n \tconst char *name;\n-\tint (*write_archive)(struct archiver_args *);\n+\tint (*write_archive)(const struct archiver *, struct archiver_args *);\n \tunsigned flags;\n+\tvoid *data;\n };\n extern void register_archiver(struct archiver *);\n \n-- \n1.7.5.4.44.g4b107\n"},{"id":"170436","messageId":"20110622012448.GE30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 5/9] archive: move file extension format-guessing lower","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:24:48Z","receivedAt":"2011-06-22T01:24:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The process for guessing an archive output format based on\nthe filename is something like this:\n\n a. parse --output in cmd_archive; check the filename\n against a static set of mapping heuristics (right now\n it just matches \".zip\" for zip files).\n\n b. if found, stick a fake \"--format=zip\" at the beginning\n of the arguments list (if the user did specify a\n --format manually, the later option will override our\n fake one)\n\n c. if it's a remote call, ship the arguments to the remote\n (including the fake), which will call write_archive on\n their end\n\n d. if it's local, ship the arguments to write_archive\n locally\n\nThere are two problems:\n\n 1. The set of mappings is static and at too high a level.\n The write_archive level is going to check config for\n user-defined formats, some of which will specify\n extensions. We need to delay lookup until those are\n parsed, so we can match against them.\n\n 2. For a remote archive call, our set of mappings (or\n formats) may not match the remote side's. This is OK in\n practice right now, because all versions of git\n understand \"zip\" and \"tar\". But as new formats are\n added, there is going to be a mismatch between what the\n client can do and what the remote server can do.\n\nTo fix (1), this patch refactors the location guessing to\nhappen at the write_archive level, instead of the\ncmd_archive level. So instead of sticking a fake --format\nfield in the argv list, we actually pass a \"name hint\" down\nthe callchain; this hint is used at the appropriate time to\nguess the format (if one hasn't been given already).\n\nThis patch leaves (2) unfixed. The name_hint is converted to\na \"--format\" option as before, and passed to the remote.\nThis means the local side's idea of how extensions map to\nformats will take precedence.\n\nAnother option would be to pass the name hint to the remote\nside and let the remote choose. This isn't a good idea for\ntwo reasons:\n\n 1. There's no room in the protocol for passing that\n information. We can pass a new argument, but older\n versions of git on the server will choke on it.\n\n 2. Letting the remote side decide creates a silent\n inconsistency in user experience. Consider the case\n that the locally installed git knows about the \"tar.gz\"\n format, but a remote server doesn't.\n\n Running \"git archive -o foo.tar.gz\" will use the tar.gz\n format. If we use --remote, and the local side chooses\n the format, then we send \"--format=tar.gz\" to the\n remote, which will complain about the unknown format.\n But if we let the remote side choose the format, then\n it will realize that it doesn't know about \"tar.gz\" and\n output uncompressed tar without even issuing a warning.\n\nSigned-off-by: Jeff King \n---\nCode same as v1, but I cleaned up the commit message to be a little less\nrambling.\n\n archive.c | 25 +++++++++++++++++++---\n archive.h | 4 ++-\n builtin/archive.c | 51 ++++++++++++++-------------------------------\n builtin/upload-archive.c | 2 +-\n 4 files changed, 41 insertions(+), 41 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex a0a5beb..7d0ca32 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -298,9 +298,10 @@ static void parse_treeish_arg(const char **argv,\n \t PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_HIDDEN, NULL, (p) }\n \n static int parse_archive_args(int argc, const char **argv,\n-\t\tconst struct archiver **ar, struct archiver_args *args)\n+\t\tconst struct archiver **ar, struct archiver_args *args,\n+\t\tconst char *name_hint)\n {\n-\tconst char *format = \"tar\";\n+\tconst char *format = NULL;\n \tconst char *base = NULL;\n \tconst char *remote = NULL;\n \tconst char *exec = NULL;\n@@ -359,6 +360,11 @@ static int parse_archive_args(int argc, const char **argv,\n \t\texit(0);\n \t}\n \n+\tif (!format && name_hint)\n+\t\tformat = archive_format_from_filename(name_hint);\n+\tif (!format)\n+\t\tformat = \"tar\";\n+\n \t/* We need at least one parameter -- tree-ish */\n \tif (argc < 1)\n \t\tusage_with_options(archive_usage, opts);\n@@ -384,7 +390,7 @@ static int parse_archive_args(int argc, const char **argv,\n }\n \n int write_archive(int argc, const char **argv, const char *prefix,\n-\t\tint setup_prefix)\n+\t\t int setup_prefix, const char *name_hint)\n {\n \tint nongit = 0;\n \tconst struct archiver *ar = NULL;\n@@ -397,7 +403,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tinit_tar_archiver();\n \tinit_zip_archiver();\n \n-\targc = parse_archive_args(argc, argv, &ar, &args);\n+\targc = parse_archive_args(argc, argv, &ar, &args, name_hint);\n \tif (nongit) {\n \t\t/*\n \t\t * We know this will die() with an error, so we could just\n@@ -412,3 +418,14 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \n \treturn ar->write_archive(ar, &args);\n }\n+\n+const char *archive_format_from_filename(const char *filename)\n+{\n+\tconst char *ext = strrchr(filename, '.');\n+\tif (!ext)\n+\t\treturn NULL;\n+\text++;\n+\tif (!strcasecmp(ext, \"zip\"))\n+\t\treturn \"zip\";\n+\treturn NULL;\n+}\ndiff --git a/archive.h b/archive.h\nindex b3cf219..202d528 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -29,6 +29,8 @@ extern void init_zip_archiver(void);\n typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n-extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix);\n+extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint);\n+\n+const char *archive_format_from_filename(const char *filename);\n \n #endif\t/* ARCHIVE_H */\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex b14eaba..2578cf5 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -24,7 +24,8 @@ static void create_output_file(const char *output_file)\n }\n \n static int run_remote_archiver(int argc, const char **argv,\n-\t\t\t const char *remote, const char *exec)\n+\t\t\t const char *remote, const char *exec,\n+\t\t\t const char *name_hint)\n {\n \tchar buf[LARGE_PACKET_MAX];\n \tint fd[2], i, len, rv;\n@@ -37,6 +38,17 @@ static int run_remote_archiver(int argc, const char **argv,\n \ttransport = transport_get(_remote, _remote->url[0]);\n \ttransport_connect(transport, \"git-upload-archive\", exec, fd);\n \n+\t/*\n+\t * Inject a fake --format field at the beginning of the\n+\t * arguments, with the format inferred from our output\n+\t * filename. This way explicit --format options can override\n+\t * it.\n+\t */\n+\tif (name_hint) {\n+\t\tconst char *format = archive_format_from_filename(name_hint);\n+\t\tif (format)\n+\t\t\tpacket_write(fd[1], \"argument --format=%s\\n\", format);\n+\t}\n \tfor (i = 1; i < argc; i++)\n \t\tpacket_write(fd[1], \"argument %s\\n\", argv[i]);\n \tpacket_flush(fd[1]);\n@@ -63,17 +75,6 @@ static int run_remote_archiver(int argc, const char **argv,\n \treturn !!rv;\n }\n \n-static const char *format_from_name(const char *filename)\n-{\n-\tconst char *ext = strrchr(filename, '.');\n-\tif (!ext)\n-\t\treturn NULL;\n-\text++;\n-\tif (!strcasecmp(ext, \"zip\"))\n-\t\treturn \"--format=zip\";\n-\treturn NULL;\n-}\n-\n #define PARSE_OPT_KEEP_ALL ( PARSE_OPT_KEEP_DASHDASH | \t\\\n \t\t\t PARSE_OPT_KEEP_ARGV0 | \t\\\n \t\t\t PARSE_OPT_KEEP_UNKNOWN |\t\\\n@@ -84,7 +85,6 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \tconst char *exec = \"git-upload-archive\";\n \tconst char *output = NULL;\n \tconst char *remote = NULL;\n-\tconst char *format_option = NULL;\n \tstruct option local_opts[] = {\n \t\tOPT_STRING('o', \"output\", &output, \"file\",\n \t\t\t\"write the archive to this file\"),\n@@ -98,32 +98,13 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, prefix, local_opts, NULL,\n \t\t\t PARSE_OPT_KEEP_ALL);\n \n-\tif (output) {\n+\tif (output)\n \t\tcreate_output_file(output);\n-\t\tformat_option = format_from_name(output);\n-\t}\n-\n-\t/*\n-\t * We have enough room in argv[] to muck it in place, because\n-\t * --output must have been given on the original command line\n-\t * if we get to this point, and parse_options() must have eaten\n-\t * it, i.e. we can add back one element to the array.\n-\t *\n-\t * We add a fake --format option at the beginning, with the\n-\t * format inferred from our output filename. This way explicit\n-\t * --format options can override it, and the fake option is\n-\t * inserted before any \"--\" that might have been given.\n-\t */\n-\tif (format_option) {\n-\t\tmemmove(argv + 2, argv + 1, sizeof(*argv) * argc);\n-\t\targv[1] = format_option;\n-\t\targv[++argc] = NULL;\n-\t}\n \n \tif (remote)\n-\t\treturn run_remote_archiver(argc, argv, remote, exec);\n+\t\treturn run_remote_archiver(argc, argv, remote, exec, output);\n \n \tsetvbuf(stderr, NULL, _IOLBF, BUFSIZ);\n \n-\treturn write_archive(argc, argv, prefix, 1);\n+\treturn write_archive(argc, argv, prefix, 1, output);\n }\ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex 73f788e..e6bb97d 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -64,7 +64,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)\n \tsent_argv[sent_argc] = NULL;\n \n \t/* parse all options sent by the client */\n-\treturn write_archive(sent_argc, sent_argv, prefix, 0);\n+\treturn write_archive(sent_argc, sent_argv, prefix, 0, NULL);\n }\n \n __attribute__((format (printf, 1, 2)))\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170437","messageId":"20110622012525.GF30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 6/9] archive: refactor file extension format-guessing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:25:25Z","receivedAt":"2011-06-22T01:25:25Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Git-archive will guess a format from the output filename if\nno format is explicitly given. The current function just\nhardcodes \"zip\" to the zip format, and leaves everything\nelse NULL (which will default to tar). Since we are about\nto add user-specified formats, we need to be more flexible.\nThe new rule is \"if a filename ends with a dot and the name\nof a format, it matches that format\". For the existing \"tar\"\nand \"zip\" formats, this is identical to the current\nbehavior. For new user-specified formats, this will do what\nthe user expects if they name their formats appropriately.\n\nBecause we will eventually start matching arbitrary\nuser-specified extensions that may include dots, the strrchr\nsearch for the final dot is not sufficient. We need to do an\nactual suffix match with each extension.\n\nSigned-off-by: Jeff King \n---\nSimilar to v1, except we don't need to special case tar-filter code\nanymore.\n\n archive.c | 25 +++++++++++++++++++------\n 1 files changed, 19 insertions(+), 6 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 7d0ca32..41065a8 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -419,13 +419,26 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \treturn ar->write_archive(ar, &args);\n }\n \n+static int match_extension(const char *filename, const char *ext)\n+{\n+\tint prefixlen = strlen(filename) - strlen(ext);\n+\n+\t/*\n+\t * We need 1 character for the '.', and 1 character to ensure that the\n+\t * prefix is non-empty (k.e., we don't match .tar.gz with no actual\n+\t * filename).\n+\t */\n+\tif (prefixlen < 2 || filename[prefixlen-1] != '.')\n+\t\treturn 0;\n+\treturn !strcmp(filename + prefixlen, ext);\n+}\n+\n const char *archive_format_from_filename(const char *filename)\n {\n-\tconst char *ext = strrchr(filename, '.');\n-\tif (!ext)\n-\t\treturn NULL;\n-\text++;\n-\tif (!strcasecmp(ext, \"zip\"))\n-\t\treturn \"zip\";\n+\tint i;\n+\n+\tfor (i = 0; i < nr_archivers; i++)\n+\t\tif (match_extension(filename, archivers[i]->name))\n+\t\t\treturn archivers[i]->name;\n \treturn NULL;\n }\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170438","messageId":"20110622012631.GG30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 7/9] archive: implement configurable tar filters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:26:31Z","receivedAt":"2011-06-22T01:26:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"It's common to pipe the tar output produce by \"git archive\"\nthrough gzip or some other compressor. Locally, this can\neasily be done by using a shell pipe. When requesting a\nremote archive, though, it cannot be done through the\nupload-archive interface.\n\nThis patch allows configurable tar filters, so that one\ncould define a \"tar.gz\" format that automatically pipes tar\noutput through gzip.\n\nSigned-off-by: Jeff King \n---\nThis was split across several commits in the previous version of the\nseries, but due to the cleanups it fits nicely into a single commit.\n\n Documentation/git-archive.txt | 16 ++++++\n archive-tar.c | 107 ++++++++++++++++++++++++++++++++++++++++-\n t/t5000-tar-tree.sh | 43 ++++++++++++++++\n 3 files changed, 165 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 9c750e2..726bf63 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -101,6 +101,16 @@ tar.umask::\n \tdetails. If `--remote` is used then only the configuration of\n \tthe remote repository takes effect.\n \n+tar..command::\n+\tThis variable specifies a shell command through which the tar\n+\toutput generated by `git archive` should be piped. The command\n+\tis executed using the shell with the generated tar file on its\n+\tstandard input, and should produce the final output on its\n+\tstandard output. Any compression-level options will be passed\n+\tto the command (e.g., \"-9\"). An output file with the same\n+\textension as `` will be use this format if no other\n+\tformat is given.\n+\n ATTRIBUTES\n ----------\n \n@@ -149,6 +159,12 @@ git archive -o latest.zip HEAD::\n \tcommit on the current branch. Note that the output format is\n \tinferred by the extension of the output file.\n \n+git config tar.tar.xz.command \"xz -c\"::\n+\n+\tConfigure a \"tar.xz\" format for making LZMA-compressed tarfiles.\n+\tYou can use it specifying `--format=tar.xz`, or by creating an\n+\toutput file like `-o foo.tar.xz`.\n+\n \n SEE ALSO\n --------\ndiff --git a/archive-tar.c b/archive-tar.c\nindex bed9a9b..5c30747 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -4,6 +4,7 @@\n #include \"cache.h\"\n #include \"tar.h\"\n #include \"archive.h\"\n+#include \"run-command.h\"\n \n #define RECORDSIZE\t(512)\n #define BLOCKSIZE\t(RECORDSIZE * 20)\n@@ -13,6 +14,9 @@ static unsigned long offset;\n \n static int tar_umask = 002;\n \n+static int write_tar_filter_archive(const struct archiver *ar,\n+\t\t\t\t struct archiver_args *args);\n+\n /* writes out the whole block, but only if it is full */\n static void write_if_needed(void)\n {\n@@ -220,6 +224,60 @@ static int write_global_extended_header(struct archiver_args *args)\n \treturn err;\n }\n \n+static struct archiver **tar_filters;\n+static int nr_tar_filters;\n+static int alloc_tar_filters;\n+\n+static struct archiver *find_tar_filter(const char *name, int len)\n+{\n+\tint i;\n+\tfor (i = 0; i < nr_tar_filters; i++) {\n+\t\tstruct archiver *ar = tar_filters[i];\n+\t\tif (!strncmp(ar->name, name, len) && !ar->name[len])\n+\t\t\treturn ar;\n+\t}\n+\treturn NULL;\n+}\n+\n+static int tar_filter_config(const char *var, const char *value, void *data)\n+{\n+\tstruct archiver *ar;\n+\tconst char *dot;\n+\tconst char *name;\n+\tconst char *type;\n+\tint namelen;\n+\n+\tif (prefixcmp(var, \"tar.\"))\n+\t\treturn 0;\n+\tdot = strrchr(var, '.');\n+\tif (dot == var + 9)\n+\t\treturn 0;\n+\n+\tname = var + 4;\n+\tnamelen = dot - name;\n+\ttype = dot + 1;\n+\n+\tar = find_tar_filter(name, namelen);\n+\tif (!ar) {\n+\t\tar = xcalloc(1, sizeof(*ar));\n+\t\tar->name = xmemdupz(name, namelen);\n+\t\tar->write_archive = write_tar_filter_archive;\n+\t\tar->flags = ARCHIVER_WANT_COMPRESSION_LEVELS;\n+\t\tALLOC_GROW(tar_filters, nr_tar_filters + 1, alloc_tar_filters);\n+\t\ttar_filters[nr_tar_filters++] = ar;\n+\t}\n+\n+\tif (!strcmp(type, \"command\")) {\n+\t\tif (!value)\n+\t\t\treturn config_error_nonbool(var);\n+\t\tfree(ar->data);\n+\t\tar->data = xstrdup(value);\n+\t\treturn 0;\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int git_tar_config(const char *var, const char *value, void *cb)\n {\n \tif (!strcmp(var, \"tar.umask\")) {\n@@ -231,7 +289,8 @@ static int git_tar_config(const char *var, const char *value, void *cb)\n \t\t}\n \t\treturn 0;\n \t}\n-\treturn 0;\n+\n+\treturn tar_filter_config(var, value, cb);\n }\n \n static int write_tar_archive(const struct archiver *ar,\n@@ -248,6 +307,45 @@ static int write_tar_archive(const struct archiver *ar,\n \treturn err;\n }\n \n+static int write_tar_filter_archive(const struct archiver *ar,\n+\t\t\t\t struct archiver_args *args)\n+{\n+\tstruct strbuf cmd = STRBUF_INIT;\n+\tstruct child_process filter;\n+\tconst char *argv[2];\n+\tint r;\n+\n+\tif (!ar->data)\n+\t\tdie(\"BUG: tar-filter archiver called with no filter defined\");\n+\n+\tstrbuf_addstr(&cmd, ar->data);\n+\tif (args->compression_level >= 0)\n+\t\tstrbuf_addf(&cmd, \" -%d\", args->compression_level);\n+\n+\tmemset(&filter, 0, sizeof(filter));\n+\targv[0] = cmd.buf;\n+\targv[1] = NULL;\n+\tfilter.argv = argv;\n+\tfilter.use_shell = 1;\n+\tfilter.in = -1;\n+\n+\tif (start_command(&filter) < 0)\n+\t\tdie_errno(\"unable to start '%s' filter\", argv[0]);\n+\tclose(1);\n+\tif (dup2(filter.in, 1) < 0)\n+\t\tdie_errno(\"unable to redirect descriptor\");\n+\tclose(filter.in);\n+\n+\tr = write_tar_archive(ar, args);\n+\n+\tclose(1);\n+\tif (finish_command(&filter) != 0)\n+\t\tdie(\"'%s' filter reported error\", argv[0]);\n+\n+\tstrbuf_release(&cmd);\n+\treturn r;\n+}\n+\n static struct archiver tar_archiver = {\n \t\"tar\",\n \twrite_tar_archive,\n@@ -256,6 +354,13 @@ static struct archiver tar_archiver = {\n \n void init_tar_archiver(void)\n {\n+\tint i;\n \tregister_archiver(&tar_archiver);\n+\n \tgit_config(git_tar_config, NULL);\n+\tfor (i = 0; i < nr_tar_filters; i++) {\n+\t\t/* omit any filters that never had a command configured */\n+\t\tif (tar_filters[i]->data)\n+\t\t\tregister_archiver(tar_filters[i]);\n+\t}\n }\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex cff1b3e..1f90692 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -252,4 +252,47 @@ test_expect_success 'git-archive --prefix=olde-' '\n \ttest -f h/olde-a/bin/sh\n '\n \n+test_expect_success 'setup tar filters' '\n+\tgit config tar.tar.foo.command \"tr ab ba\" &&\n+\tgit config tar.bar.command \"tr ab ba\"\n+'\n+\n+test_expect_success 'archive --list mentions user filter' '\n+\tgit archive --list >output &&\n+\tgrep \"^tar\\.foo\\$\" output &&\n+\tgrep \"^bar\\$\" output\n+'\n+\n+test_expect_success 'archive --list shows remote user filters' '\n+\tgit archive --list --remote=. >output &&\n+\tgrep \"^tar\\.foo\\$\" output &&\n+\tgrep \"^bar\\$\" output\n+'\n+\n+test_expect_success 'invoke tar filter by format' '\n+\tgit archive --format=tar.foo HEAD >config.tar.foo &&\n+\ttr ab ba config.tar &&\n+\ttest_cmp b.tar config.tar &&\n+\tgit archive --format=bar HEAD >config.bar &&\n+\ttr ab ba config.tar &&\n+\ttest_cmp b.tar config.tar\n+'\n+\n+test_expect_success 'invoke tar filter by extension' '\n+\tgit archive -o config-implicit.tar.foo HEAD &&\n+\ttest_cmp config.tar.foo config-implicit.tar.foo &&\n+\tgit archive -o config-implicit.bar HEAD &&\n+\ttest_cmp config.tar.foo config-implicit.bar\n+'\n+\n+test_expect_success 'default output format remains tar' '\n+\tgit archive -o config-implicit.baz HEAD &&\n+\ttest_cmp b.tar config-implicit.baz\n+'\n+\n+test_expect_success 'extension matching requires dot' '\n+\tgit archive -o config-implicittar.foo HEAD &&\n+\ttest_cmp b.tar config-implicittar.foo\n+'\n+\n test_done\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170439","messageId":"20110622012735.GH30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 8/9] archive: provide builtin .tar.gz filter","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:27:35Z","receivedAt":"2011-06-22T01:27:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This works exactly as if the user had configured it via:\n\n [tar \"tgz\"]\n\tcommand = gzip -cn\n [tar \"tar.gz\"]\n\tcommand = gzip -cn\n\nbut since it is so common, it's convenient to have it\nbuiltin without the user needing to do anything.\n\nSigned-off-by: Jeff King \n---\nSimilar to v1, but rebased. And with docs.\n\n Documentation/git-archive.txt | 11 +++++++++++\n archive-tar.c | 2 ++\n t/t5000-tar-tree.sh | 38 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 51 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 726bf63..8b0080a 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -110,6 +110,9 @@ tar..command::\n \tto the command (e.g., \"-9\"). An output file with the same\n \textension as `` will be use this format if no other\n \tformat is given.\n++\n+The \"tar.gz\" and \"tgz\" formats are defined automatically and default to\n+`gzip -cn`. You may override them with custom commands.\n \n ATTRIBUTES\n ----------\n@@ -143,6 +146,14 @@ git archive --format=tar --prefix=git-1.4.0/ v1.4.0 | gzip >git-1.4.0.tar.gz::\n \n \tCreate a compressed tarball for v1.4.0 release.\n \n+git archive --format=tar.gz --prefix=git-1.4.0/ v1.4.0 >git-1.4.0.tar.gz::\n+\n+\tSame as above, but using the builtin tar.gz handling.\n+\n+git archive --prefix=git-1.4.0/ -o git-1.4.0.tar.gz v1.4.0::\n+\n+\tSame as above, but the format is inferred from the output file.\n+\n git archive --format=tar --prefix=git-1.4.0/ v1.4.0{caret}\\{tree\\} | gzip >git-1.4.0.tar.gz::\n \n \tCreate a compressed tarball for v1.4.0 release, but without a\ndiff --git a/archive-tar.c b/archive-tar.c\nindex 5c30747..f470ebe 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -357,6 +357,8 @@ void init_tar_archiver(void)\n \tint i;\n \tregister_archiver(&tar_archiver);\n \n+\ttar_filter_config(\"tar.tgz.command\", \"gzip -cn\", NULL);\n+\ttar_filter_config(\"tar.tar.gz.command\", \"gzip -cn\", NULL);\n \tgit_config(git_tar_config, NULL);\n \tfor (i = 0; i < nr_tar_filters; i++) {\n \t\t/* omit any filters that never had a command configured */\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 1f90692..070250e 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -26,6 +26,8 @@ commit id embedding:\n \n . ./test-lib.sh\n UNZIP=${UNZIP:-unzip}\n+GZIP=${GZIP:-gzip}\n+GUNZIP=${GUNZIP:-gzip -d}\n \n SUBSTFORMAT=%H%n\n \n@@ -295,4 +297,40 @@ test_expect_success 'extension matching requires dot' '\n \ttest_cmp b.tar config-implicittar.foo\n '\n \n+if $GZIP --version >/dev/null 2>&1; then\n+\ttest_set_prereq GZIP\n+else\n+\tsay \"Skipping some tar.gz tests because gzip not found\"\n+fi\n+\n+test_expect_success GZIP 'git archive --format=tgz' '\n+\tgit archive --format=tgz HEAD >j.tgz\n+'\n+\n+test_expect_success GZIP 'git archive --format=tar.gz' '\n+\tgit archive --format=tar.gz HEAD >j1.tar.gz &&\n+\ttest_cmp j.tgz j1.tar.gz\n+'\n+\n+test_expect_success GZIP 'infer tgz from .tgz filename' '\n+\tgit archive --output=j2.tgz HEAD &&\n+\ttest_cmp j.tgz j2.tgz\n+'\n+\n+test_expect_success GZIP 'infer tgz from .tar.gz filename' '\n+\tgit archive --output=j3.tar.gz HEAD &&\n+\ttest_cmp j.tgz j3.tar.gz\n+'\n+\n+if $GUNZIP --version >/dev/null 2>&1; then\n+\ttest_set_prereq GUNZIP\n+else\n+\tsay \"Skipping some tar.gz tests because gunzip was not found\"\n+fi\n+\n+test_expect_success GZIP,GUNZIP 'extract tgz file' '\n+\t$GUNZIP -c j.tar &&\n+\ttest_cmp b.tar j.tar\n+'\n+\n test_done\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170440","messageId":"20110622013545.GI30604@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622011923.GA30370@sigill.intra.peff.net","subject":"[PATCHv2 9/9] upload-archive: allow user to turn off filters","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-06-22T01:35:45Z","receivedAt":"2011-06-22T01:35:45Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some tar filters may be very expensive to run, so sites do\nnot want to expose them via upload-archive. This patch lets\nusers configure tar..remote to turn them off.\n\nBy default, gzip filters are left on, as they are about as\nexpensive as creating zip archives.\n\nSigned-off-by: Jeff King \n---\nThis is my response to René's:\n\n [archive]\n remoteFormats = tar zip tgz tar.gz\n\nIt's a little more verbose, but it lets you turn individual formats off\nand on without having to enumerate the whole list.\n\nBy itself, this may not be that useful. It seems unlikely that somebody\nwould both to configure a filter globally on a machine serving\nupload-pack, and then not want it remotely available. You can also\ndisable the builtin gzip filter, but it's not really much more expensive\nthan zip.\n\nBut two possible follow-on patches would be:\n\n 1. tar.remote and zip.remote, to turn those formats off. I can see\n wanting to turn off _all_ compression, including zip and gzip, if\n your CPU is terrible (but git servers are almost always I/O bound,\n so I don't know how common that will be). I can also see wanting to\n turn off uncompressed tar, because bandwidth is expensive, CPU is\n cheap, and source code repositories compress well.\n\n 2. adding default bzip2 config; this should be off by default for\n upload-archive\n\n Documentation/git-archive.txt | 6 ++++++\n archive-tar.c | 11 ++++++++++-\n archive-zip.c | 2 +-\n archive.c | 11 ++++++-----\n archive.h | 3 ++-\n builtin/archive.c | 2 +-\n builtin/upload-archive.c | 2 +-\n t/t5000-tar-tree.sh | 25 ++++++++++++++++++++++---\n 8 files changed, 49 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 8b0080a..1320c87 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -114,6 +114,12 @@ tar..command::\n The \"tar.gz\" and \"tgz\" formats are defined automatically and default to\n `gzip -cn`. You may override them with custom commands.\n \n+tar..remote::\n+\tIf true, enable `` for use by remote clients via\n+\tlinkgit:git-upload-archive[1]. Defaults to false for\n+\tuser-defined formats, but true for the \"tar.gz\" and \"tgz\"\n+\tformats.\n+\n ATTRIBUTES\n ----------\n \ndiff --git a/archive-tar.c b/archive-tar.c\nindex f470ebe..01ab43f 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -274,6 +274,13 @@ static int tar_filter_config(const char *var, const char *value, void *data)\n \t\tar->data = xstrdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(type, \"remote\")) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\ttf->flags |= ARCHIVER_REMOTE;\n+\t\telse\n+\t\t\ttf->flags &= ~ARCHIVER_REMOTE;\n+\t\treturn 0;\n+\t}\n \n \treturn 0;\n }\n@@ -349,7 +356,7 @@ static int write_tar_filter_archive(const struct archiver *ar,\n static struct archiver tar_archiver = {\n \t\"tar\",\n \twrite_tar_archive,\n-\t0\n+\tARCHIVER_REMOTE\n };\n \n void init_tar_archiver(void)\n@@ -358,7 +365,9 @@ void init_tar_archiver(void)\n \tregister_archiver(&tar_archiver);\n \n \ttar_filter_config(\"tar.tgz.command\", \"gzip -cn\", NULL);\n+\ttar_filter_config(\"tar.tgz.remote\", \"true\", NULL);\n \ttar_filter_config(\"tar.tar.gz.command\", \"gzip -cn\", NULL);\n+\ttar_filter_config(\"tar.tar.gz.remote\", \"true\", NULL);\n \tgit_config(git_tar_config, NULL);\n \tfor (i = 0; i < nr_tar_filters; i++) {\n \t\t/* omit any filters that never had a command configured */\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 42df660..3c102a1 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -283,7 +283,7 @@ static int write_zip_archive(const struct archiver *ar,\n static struct archiver zip_archiver = {\n \t\"zip\",\n \twrite_zip_archive,\n-\tARCHIVER_WANT_COMPRESSION_LEVELS\n+\tARCHIVER_WANT_COMPRESSION_LEVELS|ARCHIVER_REMOTE\n };\n \n void init_zip_archiver(void)\ndiff --git a/archive.c b/archive.c\nindex 41065a8..2a7a28e 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -299,7 +299,7 @@ static void parse_treeish_arg(const char **argv,\n \n static int parse_archive_args(int argc, const char **argv,\n \t\tconst struct archiver **ar, struct archiver_args *args,\n-\t\tconst char *name_hint)\n+\t\tconst char *name_hint, int is_remote)\n {\n \tconst char *format = NULL;\n \tconst char *base = NULL;\n@@ -356,7 +356,8 @@ static int parse_archive_args(int argc, const char **argv,\n \n \tif (list) {\n \t\tfor (i = 0; i < nr_archivers; i++)\n-\t\t\tprintf(\"%s\\n\", archivers[i]->name);\n+\t\t\tif (!is_remote || archivers[i]->flags & ARCHIVER_REMOTE)\n+\t\t\t\tprintf(\"%s\\n\", archivers[i]->name);\n \t\texit(0);\n \t}\n \n@@ -369,7 +370,7 @@ static int parse_archive_args(int argc, const char **argv,\n \tif (argc < 1)\n \t\tusage_with_options(archive_usage, opts);\n \t*ar = lookup_archiver(format);\n-\tif (!*ar)\n+\tif (!*ar || (is_remote && !((*ar)->flags & ARCHIVER_REMOTE)))\n \t\tdie(\"Unknown archive format '%s'\", format);\n \n \targs->compression_level = Z_DEFAULT_COMPRESSION;\n@@ -390,7 +391,7 @@ static int parse_archive_args(int argc, const char **argv,\n }\n \n int write_archive(int argc, const char **argv, const char *prefix,\n-\t\t int setup_prefix, const char *name_hint)\n+\t\t int setup_prefix, const char *name_hint, int remote)\n {\n \tint nongit = 0;\n \tconst struct archiver *ar = NULL;\n@@ -403,7 +404,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tinit_tar_archiver();\n \tinit_zip_archiver();\n \n-\targc = parse_archive_args(argc, argv, &ar, &args, name_hint);\n+\targc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n \tif (nongit) {\n \t\t/*\n \t\t * We know this will die() with an error, so we could just\ndiff --git a/archive.h b/archive.h\nindex 202d528..2b0884f 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -15,6 +15,7 @@ struct archiver_args {\n };\n \n #define ARCHIVER_WANT_COMPRESSION_LEVELS 1\n+#define ARCHIVER_REMOTE 2\n struct archiver {\n \tconst char *name;\n \tint (*write_archive)(const struct archiver *, struct archiver_args *);\n@@ -29,7 +30,7 @@ extern void init_zip_archiver(void);\n typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n-extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint);\n+extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint, int remote);\n \n const char *archive_format_from_filename(const char *filename);\n \ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex 2578cf5..883c009 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -106,5 +106,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \n \tsetvbuf(stderr, NULL, _IOLBF, BUFSIZ);\n \n-\treturn write_archive(argc, argv, prefix, 1, output);\n+\treturn write_archive(argc, argv, prefix, 1, output, 0);\n }\ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex e6bb97d..2d0b383 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -64,7 +64,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)\n \tsent_argv[sent_argc] = NULL;\n \n \t/* parse all options sent by the client */\n-\treturn write_archive(sent_argc, sent_argv, prefix, 0, NULL);\n+\treturn write_archive(sent_argc, sent_argv, prefix, 0, NULL, 1);\n }\n \n __attribute__((format (printf, 1, 2)))\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 070250e..9e3ba98 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -256,7 +256,8 @@ test_expect_success 'git-archive --prefix=olde-' '\n \n test_expect_success 'setup tar filters' '\n \tgit config tar.tar.foo.command \"tr ab ba\" &&\n-\tgit config tar.bar.command \"tr ab ba\"\n+\tgit config tar.bar.command \"tr ab ba\" &&\n+\tgit config tar.bar.remote true\n '\n \n test_expect_success 'archive --list mentions user filter' '\n@@ -265,9 +266,9 @@ test_expect_success 'archive --list mentions user filter' '\n \tgrep \"^bar\\$\" output\n '\n \n-test_expect_success 'archive --list shows remote user filters' '\n+test_expect_success 'archive --list shows only enabled remote filters' '\n \tgit archive --list --remote=. >output &&\n-\tgrep \"^tar\\.foo\\$\" output &&\n+\t! grep \"^tar\\.foo\\$\" output &&\n \tgrep \"^bar\\$\" output\n '\n \n@@ -297,6 +298,13 @@ test_expect_success 'extension matching requires dot' '\n \ttest_cmp b.tar config-implicittar.foo\n '\n \n+test_expect_success 'only enabled filters are available remotely' '\n+\ttest_must_fail git archive --remote=. --format=tar.foo HEAD \\\n+\t\t>remote.tar.foo &&\n+\tgit archive --remote=. --format=bar >remote.bar HEAD &&\n+\ttest_cmp remote.bar config.bar\n+'\n+\n if $GZIP --version >/dev/null 2>&1; then\n \ttest_set_prereq GZIP\n else\n@@ -333,4 +341,15 @@ test_expect_success GZIP,GUNZIP 'extract tgz file' '\n \ttest_cmp b.tar j.tar\n '\n \n+test_expect_success GZIP 'remote tar.gz is allowed by default' '\n+\tgit archive --remote=. --format=tar.gz HEAD >remote.tar.gz &&\n+\ttest_cmp j.tgz remote.tar.gz\n+'\n+\n+test_expect_success GZIP 'remote tar.gz can be disabled' '\n+\tgit config tar.tar.gz.remote false &&\n+\ttest_must_fail git archive --remote=. --format=tar.gz HEAD \\\n+\t\t>remote.tar.gz\n+'\n+\n test_done\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170441","messageId":"20110622014521.GA30733@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622012631.GG30604@sigill.intra.peff.net","subject":"Re: [PATCHv2 7/9] archive: implement configurable tar filters","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-22T01:45:21Z","receivedAt":"2011-06-22T01:45:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 21, 2011 at 09:26:31PM -0400, Jeff King wrote:\n\n> It's common to pipe the tar output produce by \"git archive\"\n> through gzip or some other compressor. Locally, this can\n> easily be done by using a shell pipe. When requesting a\n> remote archive, though, it cannot be done through the\n> upload-archive interface.\n> \n> This patch allows configurable tar filters, so that one\n> could define a \"tar.gz\" format that automatically pipes tar\n> output through gzip.\n> \n> Signed-off-by: Jeff King \n> ---\n> This was split across several commits in the previous version of the\n> series, but due to the cleanups it fits nicely into a single commit.\n\nA few comments on what I took and what I didn't:\n\n 1. config is now in tar..*; this avoids having yet another\n config section. However, the flat names we give to \"git config\"\n look a little silly. E.g., \"tar.tar.gz.command\".\n\n 2. René suggested compressor as the config name. I wanted to stay away\n from that name, as this really is about generic filtering. I expect\n most uses will be compressors, but this is also our method for\n supporting other container formats via external helpers (e.g., you\n could convert to cpio on the fly). The word \"filter\" is generic,\n but it's also a bit redundant. The whole tar. subsection is\n about the filter. I chose \"command\", as that is what is used for\n external diff in userdiff drivers, and it makes it clear that we\n are running an external helper. I'm lukewarm on it if somebody\n wants to argue for something else.\n\n 3. There's no config to say you want or don't want - compression\n levels. We always allow them, and it is up to the tool to complain\n if it doesn't want them. My reasoning is that most everything\n either takes them already (e.g., gzip, bzip2, xz), or would require\n a helper script that can either map them (7z) or reject them\n (whatever helper somebody might write to convert tar2cpio on the\n fly).\n\n-Peff\n"},{"id":"170445","messageId":"20110622031735.GA13879@sigill.intra.peff.net","threadId":"27625","inReplyTo":"20110622013545.GI30604@sigill.intra.peff.net","subject":"Re: [PATCHv2 9/9] upload-archive: allow user to turn off filters","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-22T03:17:35Z","receivedAt":"2011-06-22T03:17:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 21, 2011 at 09:35:45PM -0400, Jeff King wrote:\n\n> Some tar filters may be very expensive to run, so sites do\n> not want to expose them via upload-archive. This patch lets\n> users configure tar..remote to turn them off.\n> \n> By default, gzip filters are left on, as they are about as\n> expensive as creating zip archives.\n\nArgh, sorry, wrong version of the patch. This one has a slight\nrefactoring typo that makes it not compile. Correct patch is below.\n\n-- >8 --\nSubject: upload-archive: allow user to turn off filters\n\nSome tar filters may be very expensive to run, so sites do\nnot want to expose them via upload-archive. This patch lets\nusers configure tar..remote to turn them off.\n\nBy default, gzip filters are left on, as they are about as\nexpensive as creating zip archives.\n\nSigned-off-by: Jeff King \n---\n Documentation/git-archive.txt | 6 ++++++\n archive-tar.c | 11 ++++++++++-\n archive-zip.c | 2 +-\n archive.c | 11 ++++++-----\n archive.h | 3 ++-\n builtin/archive.c | 2 +-\n builtin/upload-archive.c | 2 +-\n t/t5000-tar-tree.sh | 25 ++++++++++++++++++++++---\n 8 files changed, 49 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 8b0080a..1320c87 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -114,6 +114,12 @@ tar..command::\n The \"tar.gz\" and \"tgz\" formats are defined automatically and default to\n `gzip -cn`. You may override them with custom commands.\n \n+tar..remote::\n+\tIf true, enable `` for use by remote clients via\n+\tlinkgit:git-upload-archive[1]. Defaults to false for\n+\tuser-defined formats, but true for the \"tar.gz\" and \"tgz\"\n+\tformats.\n+\n ATTRIBUTES\n ----------\n \ndiff --git a/archive-tar.c b/archive-tar.c\nindex f470ebe..20af005 100644\n--- a/archive-tar.c\n+++ b/archive-tar.c\n@@ -274,6 +274,13 @@ static int tar_filter_config(const char *var, const char *value, void *data)\n \t\tar->data = xstrdup(value);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(type, \"remote\")) {\n+\t\tif (git_config_bool(var, value))\n+\t\t\tar->flags |= ARCHIVER_REMOTE;\n+\t\telse\n+\t\t\tar->flags &= ~ARCHIVER_REMOTE;\n+\t\treturn 0;\n+\t}\n \n \treturn 0;\n }\n@@ -349,7 +356,7 @@ static int write_tar_filter_archive(const struct archiver *ar,\n static struct archiver tar_archiver = {\n \t\"tar\",\n \twrite_tar_archive,\n-\t0\n+\tARCHIVER_REMOTE\n };\n \n void init_tar_archiver(void)\n@@ -358,7 +365,9 @@ void init_tar_archiver(void)\n \tregister_archiver(&tar_archiver);\n \n \ttar_filter_config(\"tar.tgz.command\", \"gzip -cn\", NULL);\n+\ttar_filter_config(\"tar.tgz.remote\", \"true\", NULL);\n \ttar_filter_config(\"tar.tar.gz.command\", \"gzip -cn\", NULL);\n+\ttar_filter_config(\"tar.tar.gz.remote\", \"true\", NULL);\n \tgit_config(git_tar_config, NULL);\n \tfor (i = 0; i < nr_tar_filters; i++) {\n \t\t/* omit any filters that never had a command configured */\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 42df660..3c102a1 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -283,7 +283,7 @@ static int write_zip_archive(const struct archiver *ar,\n static struct archiver zip_archiver = {\n \t\"zip\",\n \twrite_zip_archive,\n-\tARCHIVER_WANT_COMPRESSION_LEVELS\n+\tARCHIVER_WANT_COMPRESSION_LEVELS|ARCHIVER_REMOTE\n };\n \n void init_zip_archiver(void)\ndiff --git a/archive.c b/archive.c\nindex 41065a8..2a7a28e 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -299,7 +299,7 @@ static void parse_treeish_arg(const char **argv,\n \n static int parse_archive_args(int argc, const char **argv,\n \t\tconst struct archiver **ar, struct archiver_args *args,\n-\t\tconst char *name_hint)\n+\t\tconst char *name_hint, int is_remote)\n {\n \tconst char *format = NULL;\n \tconst char *base = NULL;\n@@ -356,7 +356,8 @@ static int parse_archive_args(int argc, const char **argv,\n \n \tif (list) {\n \t\tfor (i = 0; i < nr_archivers; i++)\n-\t\t\tprintf(\"%s\\n\", archivers[i]->name);\n+\t\t\tif (!is_remote || archivers[i]->flags & ARCHIVER_REMOTE)\n+\t\t\t\tprintf(\"%s\\n\", archivers[i]->name);\n \t\texit(0);\n \t}\n \n@@ -369,7 +370,7 @@ static int parse_archive_args(int argc, const char **argv,\n \tif (argc < 1)\n \t\tusage_with_options(archive_usage, opts);\n \t*ar = lookup_archiver(format);\n-\tif (!*ar)\n+\tif (!*ar || (is_remote && !((*ar)->flags & ARCHIVER_REMOTE)))\n \t\tdie(\"Unknown archive format '%s'\", format);\n \n \targs->compression_level = Z_DEFAULT_COMPRESSION;\n@@ -390,7 +391,7 @@ static int parse_archive_args(int argc, const char **argv,\n }\n \n int write_archive(int argc, const char **argv, const char *prefix,\n-\t\t int setup_prefix, const char *name_hint)\n+\t\t int setup_prefix, const char *name_hint, int remote)\n {\n \tint nongit = 0;\n \tconst struct archiver *ar = NULL;\n@@ -403,7 +404,7 @@ int write_archive(int argc, const char **argv, const char *prefix,\n \tinit_tar_archiver();\n \tinit_zip_archiver();\n \n-\targc = parse_archive_args(argc, argv, &ar, &args, name_hint);\n+\targc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);\n \tif (nongit) {\n \t\t/*\n \t\t * We know this will die() with an error, so we could just\ndiff --git a/archive.h b/archive.h\nindex 202d528..2b0884f 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -15,6 +15,7 @@ struct archiver_args {\n };\n \n #define ARCHIVER_WANT_COMPRESSION_LEVELS 1\n+#define ARCHIVER_REMOTE 2\n struct archiver {\n \tconst char *name;\n \tint (*write_archive)(const struct archiver *, struct archiver_args *);\n@@ -29,7 +30,7 @@ extern void init_zip_archiver(void);\n typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);\n \n extern int write_archive_entries(struct archiver_args *args, write_archive_entry_fn_t write_entry);\n-extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint);\n+extern int write_archive(int argc, const char **argv, const char *prefix, int setup_prefix, const char *name_hint, int remote);\n \n const char *archive_format_from_filename(const char *filename);\n \ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex 2578cf5..883c009 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -106,5 +106,5 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \n \tsetvbuf(stderr, NULL, _IOLBF, BUFSIZ);\n \n-\treturn write_archive(argc, argv, prefix, 1, output);\n+\treturn write_archive(argc, argv, prefix, 1, output, 0);\n }\ndiff --git a/builtin/upload-archive.c b/builtin/upload-archive.c\nindex e6bb97d..2d0b383 100644\n--- a/builtin/upload-archive.c\n+++ b/builtin/upload-archive.c\n@@ -64,7 +64,7 @@ static int run_upload_archive(int argc, const char **argv, const char *prefix)\n \tsent_argv[sent_argc] = NULL;\n \n \t/* parse all options sent by the client */\n-\treturn write_archive(sent_argc, sent_argv, prefix, 0, NULL);\n+\treturn write_archive(sent_argc, sent_argv, prefix, 0, NULL, 1);\n }\n \n __attribute__((format (printf, 1, 2)))\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex 070250e..9e3ba98 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -256,7 +256,8 @@ test_expect_success 'git-archive --prefix=olde-' '\n \n test_expect_success 'setup tar filters' '\n \tgit config tar.tar.foo.command \"tr ab ba\" &&\n-\tgit config tar.bar.command \"tr ab ba\"\n+\tgit config tar.bar.command \"tr ab ba\" &&\n+\tgit config tar.bar.remote true\n '\n \n test_expect_success 'archive --list mentions user filter' '\n@@ -265,9 +266,9 @@ test_expect_success 'archive --list mentions user filter' '\n \tgrep \"^bar\\$\" output\n '\n \n-test_expect_success 'archive --list shows remote user filters' '\n+test_expect_success 'archive --list shows only enabled remote filters' '\n \tgit archive --list --remote=. >output &&\n-\tgrep \"^tar\\.foo\\$\" output &&\n+\t! grep \"^tar\\.foo\\$\" output &&\n \tgrep \"^bar\\$\" output\n '\n \n@@ -297,6 +298,13 @@ test_expect_success 'extension matching requires dot' '\n \ttest_cmp b.tar config-implicittar.foo\n '\n \n+test_expect_success 'only enabled filters are available remotely' '\n+\ttest_must_fail git archive --remote=. --format=tar.foo HEAD \\\n+\t\t>remote.tar.foo &&\n+\tgit archive --remote=. --format=bar >remote.bar HEAD &&\n+\ttest_cmp remote.bar config.bar\n+'\n+\n if $GZIP --version >/dev/null 2>&1; then\n \ttest_set_prereq GZIP\n else\n@@ -333,4 +341,15 @@ test_expect_success GZIP,GUNZIP 'extract tgz file' '\n \ttest_cmp b.tar j.tar\n '\n \n+test_expect_success GZIP 'remote tar.gz is allowed by default' '\n+\tgit archive --remote=. --format=tar.gz HEAD >remote.tar.gz &&\n+\ttest_cmp j.tgz remote.tar.gz\n+'\n+\n+test_expect_success GZIP 'remote tar.gz can be disabled' '\n+\tgit config tar.tar.gz.remote false &&\n+\ttest_must_fail git archive --remote=. --format=tar.gz HEAD \\\n+\t\t>remote.tar.gz\n+'\n+\n test_done\n-- \n1.7.5.4.44.g4b107\n"},{"id":"170447","messageId":"4E01872F.8070503@lsrfire.ath.cx","threadId":"27625","inReplyTo":"20110622012631.GG30604@sigill.intra.peff.net","subject":"Re: [PATCHv2 7/9] archive: implement configurable tar filters","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2011-06-22T06:09:51Z","receivedAt":"2011-06-22T06:09:51Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Hello,\n\njust a quick comment before I drop off the net for a few days. I like\nthe series a lot, especially the refactorings in patches 1 to 6.\n\nAm 22.06.2011 03:26, schrieb Jeff King:\n> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\n> index 9c750e2..726bf63 100644\n> --- a/Documentation/git-archive.txt\n> +++ b/Documentation/git-archive.txt\n> @@ -101,6 +101,16 @@ tar.umask::\n> \tdetails. If `--remote` is used then only the configuration of\n> \tthe remote repository takes effect.\n> \n> +tar..command::\n\nWould switching around format and \"command\" be better?\n\n\t[tar \"command\"]\n\t\ttar.gz = gzip -cn\n\t\ttar.xz = xz -c\n\n> diff --git a/archive-tar.c b/archive-tar.c\n> index bed9a9b..5c30747 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -4,6 +4,7 @@\n> #include \"cache.h\"\n> #include \"tar.h\"\n> #include \"archive.h\"\n> +#include \"run-command.h\"\n> \n> #define RECORDSIZE\t(512)\n> #define BLOCKSIZE\t(RECORDSIZE * 20)\n> @@ -13,6 +14,9 @@ static unsigned long offset;\n> \n> static int tar_umask = 002;\n> \n> +static int write_tar_filter_archive(const struct archiver *ar,\n> +\t\t\t\t struct archiver_args *args);\n> +\n> /* writes out the whole block, but only if it is full */\n> static void write_if_needed(void)\n> {\n> @@ -220,6 +224,60 @@ static int write_global_extended_header(struct archiver_args *args)\n> \treturn err;\n> }\n> \n> +static struct archiver **tar_filters;\n> +static int nr_tar_filters;\n> +static int alloc_tar_filters;\n> +\n> +static struct archiver *find_tar_filter(const char *name, int len)\n> +{\n> +\tint i;\n> +\tfor (i = 0; i < nr_tar_filters; i++) {\n> +\t\tstruct archiver *ar = tar_filters[i];\n> +\t\tif (!strncmp(ar->name, name, len) && !ar->name[len])\n> +\t\t\treturn ar;\n> +\t}\n> +\treturn NULL;\n> +}\n> +\n> +static int tar_filter_config(const char *var, const char *value, void *data)\n> +{\n> +\tstruct archiver *ar;\n> +\tconst char *dot;\n> +\tconst char *name;\n> +\tconst char *type;\n> +\tint namelen;\n> +\n> +\tif (prefixcmp(var, \"tar.\"))\n> +\t\treturn 0;\n> +\tdot = strrchr(var, '.');\n> +\tif (dot == var + 9)\n> +\t\treturn 0;\n> +\n> +\tname = var + 4;\n> +\tnamelen = dot - name;\n> +\ttype = dot + 1;\n> +\n> +\tar = find_tar_filter(name, namelen);\n> +\tif (!ar) {\n> +\t\tar = xcalloc(1, sizeof(*ar));\n> +\t\tar->name = xmemdupz(name, namelen);\n> +\t\tar->write_archive = write_tar_filter_archive;\n> +\t\tar->flags = ARCHIVER_WANT_COMPRESSION_LEVELS;\n> +\t\tALLOC_GROW(tar_filters, nr_tar_filters + 1, alloc_tar_filters);\n> +\t\ttar_filters[nr_tar_filters++] = ar;\n> +\t}\n> +\n> +\tif (!strcmp(type, \"command\")) {\n> +\t\tif (!value)\n> +\t\t\treturn config_error_nonbool(var);\n> +\t\tfree(ar->data);\n> +\t\tar->data = xstrdup(value);\n> +\t\treturn 0;\n> +\t}\n\nWhy not register it right here instead of adding it to the intermediate\nlist? And are duplicates handled properly, e.g. system has \"gzip -cn\"\nand local wants \"gzip -c\"?\n"},{"id":"170463","messageId":"20110622145916.GA9266@sigill.intra.peff.net","threadId":"27625","inReplyTo":"4E01872F.8070503@lsrfire.ath.cx","subject":"Re: [PATCHv2 7/9] archive: implement configurable tar filters","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-22T14:59:17Z","receivedAt":"2011-06-22T14:59:17Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jun 22, 2011 at 08:09:51AM +0200, René Scharfe wrote:\n\n> just a quick comment before I drop off the net for a few days. I like\n> the series a lot, especially the refactorings in patches 1 to 6.\n\nThanks. I didn't know if I was going overboard, but the result looked\ncleaner to me, so it is good to have a second opinion.\n\n> > +tar..command::\n> \n> Would switching around format and \"command\" be better?\n> \n> \t[tar \"command\"]\n> \t\ttar.gz = gzip -cn\n> \t\ttar.xz = xz -c\n\nThat violates our usual convention that the first and final components\nare static, and the middle part can contain everything. So doing:\n\n git config tar.command.tar.gz \"gzip -cn\"\n\nis going to end up as:\n\n [tar \"command.tar\"]\n gz = gzip -cn\n\nPlus it doesn't leave room for any additional per-command config keys if\nwe want to add them in the future.\n\n> > +\tar = find_tar_filter(name, namelen);\n> > +\tif (!ar) {\n> > +\t\tar = xcalloc(1, sizeof(*ar));\n> > +\t\tar->name = xmemdupz(name, namelen);\n> > +\t\tar->write_archive = write_tar_filter_archive;\n> > +\t\tar->flags = ARCHIVER_WANT_COMPRESSION_LEVELS;\n> > +\t\tALLOC_GROW(tar_filters, nr_tar_filters + 1, alloc_tar_filters);\n> > +\t\ttar_filters[nr_tar_filters++] = ar;\n> > +\t}\n> > +\n> > +\tif (!strcmp(type, \"command\")) {\n> > +\t\tif (!value)\n> > +\t\t\treturn config_error_nonbool(var);\n> > +\t\tfree(ar->data);\n> > +\t\tar->data = xstrdup(value);\n> > +\t\treturn 0;\n> > +\t}\n> \n> Why not register it right here instead of adding it to the intermediate\n> list?\n\nIf it were just this patch, you could do that. But as soon as you add\nmore keys (e.g., a later patch adds tar.*.remote), then you run into the\nsituation of getting only part of the config at a time, and maybe not\ngetting the full config for a command at all. For example:\n\n [tar \"tar.gz\"]\n remote = true\n\nwould make an archiver with no \"command\" set. We would need to\nspecial-case it everywhere to ignore it when we looked at the list, or\nlater just remove it. This patch takes the approach of having a\nsecondary list of all of the configured bits, and then only registering\nthose that are actually valid.\n\nIt also keeps the configured and builtin lists separate. Otherwise I\nhave to special-case:\n\n [tar \"zip\"]\n command = ...\n\nto ignore the builtin zip archiver, which I think is not something we\nwant to be able to override in this way.\n\n> And are duplicates handled properly, e.g. system has \"gzip -cn\"\n> and local wants \"gzip -c\"?\n\nYes. We look up the archiver in the list of configured ones and\noverwrite its command field (that's why the .tar.gz patch actually calls\nthe config parser as if you had those lines in your config file, instead\nof registering static archiver structs).\n\nI should probably include a test for that, though.\n\n-Peff\n"},{"id":"170505","messageId":"BANLkTim7O3pcJAy4U1d6QiS6cvv2-Og21A@mail.gmail.com","threadId":"27625","inReplyTo":"20110622012333.GC30604@sigill.intra.peff.net","subject":"Re: [PATCHv2 3/9] archive: refactor list of archive formats","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2011-06-23T17:05:35Z","receivedAt":"2011-06-23T17:05:35Z","isPatch":false,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Tue, Jun 21, 2011 at 10:23 PM, Jeff King wrote:\n> Most of the tar and zip code was nicely split out into two\n> abstracted files which knew only about their specific\n> formats. The entry point to this code was a single \"write\n> archive\" function.\n>\n> However, as these basic formats grow more complex (e.g., by\n> handling multiple file extensions and format names), a\n> static list of the entry point functions won't be enough.\n> Instead, let's provide a way for the tar and zip code to\n> tell the main archive code what they support by registering\n> archiver names and functions.\n>\n> Signed-off-by: Jeff King \n> ---\n> New in v2. This turns archivers more into proper objects, rather than a\n> hard-coded list of functions, and makes the rest of the series much\n> cleaner.\n>\n>  archive-tar.c |   16 +++++++++++++---\n>  archive-zip.c |   13 ++++++++++++-\n>  archive.c     |   33 +++++++++++++++++----------------\n>  archive.h     |   17 ++++++++++-------\n>  4 files changed, 52 insertions(+), 27 deletions(-)\n>\n> diff --git a/archive-tar.c b/archive-tar.c\n> index 1ab1a2c..930375b 100644\n> --- a/archive-tar.c\n> +++ b/archive-tar.c\n> @@ -234,12 +234,10 @@ static int git_tar_config(const char *var, const char *value, void *cb)\n>        return 0;\n>  }\n>\n> -int write_tar_archive(struct archiver_args *args)\n> +static int write_tar_archive(struct archiver_args *args)\n>  {\n>        int err = 0;\n>\n> -       git_config(git_tar_config, NULL);\n> -\n>        if (args->commit_sha1)\n>                err = write_global_extended_header(args);\n>        if (!err)\n> @@ -248,3 +246,15 @@ int write_tar_archive(struct archiver_args *args)\n>                write_trailer();\n>        return err;\n>  }\n> +\n> +static struct archiver tar_archiver = {\n> +       \"tar\",\n> +       write_tar_archive,\n> +       0\nA named constant instead of 0, like you did with\nARCHIVER_WANT_COMPRESSION_LEVELS, would be better? 0 here means the\narchiver does not want compression?\n"},{"id":"170509","messageId":"20110623173013.GA7364@sigill.intra.peff.net","threadId":"27625","inReplyTo":"BANLkTim7O3pcJAy4U1d6QiS6cvv2-Og21A@mail.gmail.com","subject":"Re: [PATCHv2 3/9] archive: refactor list of archive formats","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-06-23T17:30:13Z","receivedAt":"2011-06-23T17:30:13Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 23, 2011 at 02:05:35PM -0300, Thiago Farina wrote:\n\n> > +static struct archiver tar_archiver = {\n> > +       \"tar\",\n> > +       write_tar_archive,\n> > +       0\n> A named constant instead of 0, like you did with\n> ARCHIVER_WANT_COMPRESSION_LEVELS, would be better? 0 here means the\n> archiver does not want compression?\n\nIt's actually a bit-wise flag, so it is not \"no compression\", but \"no\nflags\". So \"0\" is fairly idiomatic. Given that it's a static initializer\nthat will default to 0, probably a more readable version would be:\n\n static struct archiver tar_archiver = {\n \"tar\",\n write_tar_archive\n };\n\n-Peff\n"}]}