{"thread":{"id":"17849","subject":"[PATCH v3] git-archive: Add new option \"--output\" to write archive to a file instead of stdout.","startedAt":"2009-02-17T09:42:13Z","lastAt":"2009-02-17T22:59:50Z","messageCount":3,"participants":["carlos.duclos@nokia.com","Junio C Hamano","René Scharfe"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"105112","messageId":"599636D7828020419E3AB2DE5CCC8130036BF8B1D6@NOK-EUMSG-02.mgdnok.nokia.com","threadId":"17849","inReplyTo":null,"subject":"[PATCH v3] git-archive: Add new option \"--output\" to write archive to a file instead of stdout.","fromName":"","fromEmail":"carlos.duclos@nokia.com","sentAt":"2009-02-17T09:42:13Z","receivedAt":"2009-02-17T09:42:13Z","isPatch":true,"sender":{"key":"carlos.duclos@nokia.com","avatar":"https://gravatar.com/avatar/be05d03546ed19ca9104e62d4a330bc40b4115ebcf39d1a4c43884b663667951?d=mp&s=160"},"body":"Patch attached as MIME to avoid conversion problems.\n\nPatch highlights:\n1. Formatted to 80 columns.\n2. Change the language in the documentation to match the style.\n3. Modified create_output_file.\n3.1 Changed from creat(2) to open(2).\n3.2 Changed the logic so we only dup the file if the fd is different from 1.\n3.3 Didn't change the closing of the file, since dup2 will close the file for us.\nOnly close the file if there was an error.\n4. Didn't change the tests, it never hurts to have many tests :-)\n\nRegards\n\n\n\nFrom b68d40dca34d45e2535c50879cce62e3b24a2f30 Mon Sep 17 00:00:00 2001\nFrom: Carlos Manuel Duclos Vergara \nDate: Mon, 16 Feb 2009 18:20:25 +0100\nSubject: [PATCH] git-archive: Add new option \"--output\" to write archive to a file instead of stdout.\n\nWhen archiving a repository there is no way to specify a file as output. This patch adds a new option \"--output\" that redirects the output to a file instead of stdout.\n---\n Documentation/git-archive.txt | 3 +\n archive.c | 23 ++++++\n t/t0024-crlf-archive.sh | 19 +++++\n t/t5000-tar-tree.sh | 148 +++++++++++++++++++++++++++++++++++++++++\n 4 files changed, 193 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 41cbf9c..5bde197 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -47,6 +47,9 @@ OPTIONS\n --prefix=/::\n \tPrepend / to each filename in the archive.\n \n+--output=::\n+\tWrite the archive to instead of stdout.\n+\n ::\n \tThis can be any options that the archiver backend understand.\n \tSee next section.\ndiff --git a/archive.c b/archive.c\nindex e6de039..e6af4ec 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -239,6 +239,23 @@ static void parse_treeish_arg(const char **argv,\n \tar_args->time = archive_time;\n }\n \n+static void create_output_file(const char *output_file)\n+{\n+\tint output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n+\tif (output_fd < 0)\n+\t\tdie(\"could not create archive file: %s \", output_file);\n+\tif (output_fd != 1)\n+\t\tif (dup2(output_fd, 1) < 0) {\n+\t\t\t/*\n+\t\t\t * dup2 closes output_fd on success, if something \n+\t\t\t * goes wrong we close output_fd here to avoid\n+\t\t\t * problems.\n+\t\t\t */\n+\t\t\tclose(output_fd);\n+\t\t\tdie(\"could not redirect output\");\n+\t\t}\n+}\n+\n #define OPT__COMPR(s, v, h, p) \\\n \t{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \\\n \t PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }\n@@ -253,6 +270,7 @@ static int parse_archive_args(int argc, const char **argv,\n \tconst char *base = NULL;\n \tconst char *remote = NULL;\n \tconst char *exec = NULL;\n+\tconst char *output = NULL;\n \tint compression_level = -1;\n \tint verbose = 0;\n \tint i;\n@@ -262,6 +280,8 @@ static int parse_archive_args(int argc, const char **argv,\n \t\tOPT_STRING(0, \"format\", &format, \"fmt\", \"archive format\"),\n \t\tOPT_STRING(0, \"prefix\", &base, \"prefix\",\n \t\t\t\"prepend prefix to each pathname in the archive\"),\n+\t\tOPT_STRING(0, \"output\", &output, \"file\", \n+\t\t\t\"write the archive to this file\"),\n \t\tOPT__VERBOSE(&verbose),\n \t\tOPT__COMPR('0', &compression_level, \"store only\", 0),\n \t\tOPT__COMPR('1', &compression_level, \"compress faster\", 1),\n@@ -294,6 +314,9 @@ static int parse_archive_args(int argc, const char **argv,\n \tif (!base)\n \t\tbase = \"\";\n \n+\tif (output)\n+\t\tcreate_output_file(output);\n+\n \tif (list) {\n \t\tfor (i = 0; i < ARRAY_SIZE(archivers); i++)\n \t\t\tprintf(\"%s\\n\", archivers[i].name);\ndiff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh\nindex e533039..237a8f6 100755\n--- a/t/t0024-crlf-archive.sh\n+++ b/t/t0024-crlf-archive.sh\n@@ -26,6 +26,15 @@ test_expect_success 'tar archive' '\n \n '\n \n+test_expect_success 'tar archive output redirected' '\n+\n+\tgit archive --format=tar --output=test.tar HEAD &&\n+\t( mkdir untarred2 && cd untarred2 && \"$TAR\" -xf ../test.tar )\n+\n+\ttest_cmp sample untarred2/sample\n+\n+'\n+\n \"$UNZIP\" -v >/dev/null 2>&1\n if [ $? -eq 127 ]; then\n \techo \"Skipping ZIP test, because unzip was not found\"\n@@ -43,4 +52,14 @@ test_expect_success 'zip archive' '\n \n '\n \n+test_expect_success 'zip archive output redirected' '\n+\n+\tgit archive --format=zip --output=test.zip HEAD &&\n+\n+\t( mkdir unzipped2 && cd unzipped2 && unzip ../test.zip ) &&\n+\n+\ttest_cmp sample unzipped2/sample\n+\n+'\n+\n test_done\ndiff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh\nindex c942c8b..b11e504 100755\n--- a/t/t5000-tar-tree.sh\n+++ b/t/t5000-tar-tree.sh\n@@ -66,6 +66,11 @@ test_expect_success \\\n 'remove ignored file' \\\n 'rm a/ignored'\n \n+\n+#\n+# Tar tests\n+#\n+\n test_expect_success \\\n 'git archive' \\\n 'git archive HEAD >b.tar'\n@@ -160,6 +165,91 @@ test_expect_success \\\n diff a/substfile2 g/prefix/a/substfile2\n '\n \n+#\n+# Same tests as above but now using redirection\n+#\n+\n+test_expect_success \\\n+ 'git archive' \\\n+ 'git archive --output=b20.tar HEAD'\n+\n+test_expect_success \\\n+ 'git tar-tree' \\\n+ 'git tar-tree HEAD >b21.tar'\n+\n+test_expect_success \\\n+ 'git archive vs. git tar-tree' \\\n+ 'diff b20.tar b21.tar'\n+\n+test_expect_success \\\n+ 'git archive in a bare repo' \\\n+ '(cd bare.git && git archive --output=../b22.tar HEAD)'\n+\n+test_expect_success \\\n+ 'git archive vs. the same in a bare repo' \\\n+ 'test_cmp b20.tar b22.tar'\n+\n+test_expect_success \\\n+ 'validate file modification time' \\\n+ 'mkdir extract2 &&\n+ \"$TAR\" xf b20.tar -C extract2 a/a &&\n+ test-chmtime -v +0 extract2/a/a |cut -f 1 >b20.mtime &&\n+ echo \"1117231200\" >expected.mtime &&\n+ diff expected.mtime b20.mtime'\n+\n+test_expect_success \\\n+ 'git get-tar-commit-id' \\\n+ 'git get-tar-commit-id b20.commitid &&\n+ diff .git/$(git symbolic-ref HEAD) b20.commitid'\n+\n+test_expect_success \\\n+ 'extract tar archive' \\\n+ '(mkdir b20 && cd b20 && \"$TAR\" xf -) b20.lst &&\n+ diff a.lst b20.lst'\n+\n+test_expect_success \\\n+ 'validate file contents' \\\n+ 'diff -r a b20/a'\n+\n+test_expect_success \\\n+ 'create archives with substfiles' \\\n+ 'echo \"substfile?\" export-subst >a/.gitattributes &&\n+ git archive --output=f20.tar HEAD &&\n+ git archive --prefix=prefix/ --output=g20.tar HEAD &&\n+ rm a/.gitattributes'\n+\n+test_expect_success \\\n+ 'extract substfiles' \\\n+ '(mkdir f20 && cd f20 && \"$TAR\" xf -) f20/a/substfile1.expected &&\n+ diff f20/a/substfile1.expected f20/a/substfile1 &&\n+ diff a/substfile2 f20/a/substfile2\n+'\n+\n+test_expect_success \\\n+ 'extract substfiles from archive with prefix' \\\n+ '(mkdir g20 && cd g20 && \"$TAR\" xf -) g20/prefix/a/substfile1.expected &&\n+ diff g20/prefix/a/substfile1.expected g20/prefix/a/substfile1 &&\n+ diff a/substfile2 g20/prefix/a/substfile2\n+'\n+\n+#\n+# Zip tests\n+#\n+\n test_expect_success \\\n 'git archive --format=zip' \\\n 'git archive --format=zip HEAD >d.zip'\n@@ -172,6 +262,26 @@ test_expect_success \\\n 'git archive --format=zip vs. the same in a bare repo' \\\n 'test_cmp d.zip d1.zip'\n \n+#\n+# Same tests as above but now using redirection\n+#\n+\n+test_expect_success \\\n+ 'git archive --format=zip --output=d10.zip' \\\n+ 'git archive --format=zip --output=d10.zip HEAD'\n+\n+test_expect_success \\\n+ 'git archive --format=zip --output=d11.zip in a bare repo' \\\n+ '(cd bare.git && git archive --format=zip --output=../d11.zip HEAD)'\n+\n+test_expect_success \\\n+ 'git archive --format=zip redirected output vs. the same in a bare repo' \\\n+ 'test_cmp d10.zip d11.zip'\n+\n+#\n+# Zip tests\n+#\n+\n $UNZIP -v >/dev/null 2>&1\n if [ $? -eq 127 ]; then\n \techo \"Skipping ZIP tests, because unzip was not found\"\n@@ -213,4 +323,42 @@ test_expect_success \\\n 'git archive --list outside of a git repo' \\\n 'GIT_DIR=some/non-existing/directory git archive --list'\n \n+#\n+# Same tests as above but now with redirected output\n+#\n+\n+test_expect_success \\\n+ 'extract ZIP archive from redirected output archive' \\\n+ '(mkdir d10 && cd d10 && $UNZIP ../d10.zip)'\n+\n+test_expect_success \\\n+ 'validate filenames from redirected output archive' \\\n+ '(cd d10/a && find .) | sort >d10.lst &&\n+ diff a.lst d10.lst'\n+\n+test_expect_success \\\n+ 'validate file contents from redirected output archive' \\\n+ 'diff -r a d10/a'\n+\n+test_expect_success \\\n+ 'git archive --format=zip with prefix from redirected output archive' \\\n+ 'git archive --format=zip --prefix=prefix/ --output=e10.zip HEAD'\n+\n+test_expect_success \\\n+ 'extract ZIP archive with prefix from redirected output archive' \\\n+ '(mkdir e10 && cd e10 && $UNZIP ../e10.zip)'\n+\n+test_expect_success \\\n+ 'validate filenames with prefix from redirected output archive' \\\n+ '(cd e10/prefix/a && find .) | sort >e10.lst &&\n+ diff a.lst e10.lst'\n+\n+test_expect_success \\\n+ 'validate file contents with prefix from redirected output archive' \\\n+ 'diff -r a e10/prefix/a'\n+\n+test_expect_success \\\n+ 'git archive --list outside of a git repo' \\\n+ 'GIT_DIR=some/non-existing/directory git archive --list'\n+\n test_done\n-- \n1.6.2.rc0.63.g7cbd0.dirty\n\n"},{"id":"105197","messageId":"7v7i3o6h8x.fsf@gitster.siamese.dyndns.org","threadId":"17849","inReplyTo":"599636D7828020419E3AB2DE5CCC8130036BF8B1D6@NOK-EUMSG-02.mgdnok.nokia.com","subject":"Re: [PATCH v3] git-archive: Add new option \"--output\" to write archive to a file instead of stdout.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-17T18:38:38Z","receivedAt":"2009-02-17T18:38:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":" writes:\n\n> 4. Didn't change the tests, it never hurts to have many tests :-)\n\nIt actually hurts me a lot. I run every tests before pushing out the\nintegration results, and duplicated tests costs me a lot without adding\nmuch value to the process.\n\n> From b68d40dca34d45e2535c50879cce62e3b24a2f30 Mon Sep 17 00:00:00 2001\n> From: Carlos Manuel Duclos Vergara \n> Date: Mon, 16 Feb 2009 18:20:25 +0100\n> Subject: [PATCH] git-archive: Add new option \"--output\" to write archive to a file instead of stdout.\n\nVery long line that you can be a bit creative to shorten. E.g.\n\n git-archive: add --output= to send output to a file instead of stdout\n\nBecause anything you are adding is new, people can tell that --word is an\noption from the context, and people who are reading \"git log\" output know\nthat archive command traditionally writes to standard output.\n\n> When archiving a repository there is no way to specify a file as output. This patch adds a new option \"--output\" that redirects the output to a file instead of stdout.\n\nVery long line I can wrap locally so this by itself is not a grave enough\noffence to ask you to fix and resubmit, but I'm asking you to redo the\ntests anyway, so please wrap this while at it.\n\nNeeds a sign-off.\n\n> diff --git a/archive.c b/archive.c\n> index e6de039..e6af4ec 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -239,6 +239,23 @@ static void parse_treeish_arg(const char **argv,\n> \tar_args->time = archive_time;\n> }\n> \n> +static void create_output_file(const char *output_file)\n> +{\n> +\tint output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n> +\tif (output_fd < 0)\n> +\t\tdie(\"could not create archive file: %s \", output_file);\n> +\tif (output_fd != 1)\n> +\t\tif (dup2(output_fd, 1) < 0) {\n> +\t\t\t/*\n> +\t\t\t * dup2 closes output_fd on success, if something \n> +\t\t\t * goes wrong we close output_fd here to avoid\n> +\t\t\t * problems.\n> +\t\t\t */\n> +\t\t\tclose(output_fd);\n> +\t\t\tdie(\"could not redirect output\");\n\nThe comment and close() are probably unnecessary, as you will die()\nimmediately.\n\n> +test_expect_success 'tar archive output redirected' '\n> +\n> +\tgit archive --format=tar --output=test.tar HEAD &&\n> +\t( mkdir untarred2 && cd untarred2 && \"$TAR\" -xf ../test.tar )\n> +\n> +\ttest_cmp sample untarred2/sample\n> +\n> +'\n\nI agree with René that the test should just create the same archive twice,\nwith or without --output=, and compare the output.\n\nTo protect your patch from future changes that may do something funny to\nonly one side of the codepath, which is not even very likely knowing the\nstructure of the code, I'd recommend doing two tests, one for --format=tar\nand another for --format=zip.\n\nAlso you do not have to introduce an inconsistent way to give option to\n\"tar\" like the above (note that \"tar xf\" is the preferred traditional\nnotation, not \"tar -xf\", in this script).\n\nOther than that, I think this is ready for 'next'.\n"},{"id":"105233","messageId":"499B4166.90609@lsrfire.ath.cx","threadId":"17849","inReplyTo":"7v7i3o6h8x.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v3] git-archive: Add new option \"--output\" to write archive to a file instead of stdout.","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-17T22:59:50Z","receivedAt":"2009-02-17T22:59:50Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Junio C Hamano schrieb:\n> writes:\n>> +static void create_output_file(const char *output_file)\n>> +{\n>> +\tint output_fd = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);\n>> +\tif (output_fd < 0)\n>> +\t\tdie(\"could not create archive file: %s \", output_file);\n>> +\tif (output_fd != 1)\n>> +\t\tif (dup2(output_fd, 1) < 0) {\n>> +\t\t\t/*\n>> +\t\t\t * dup2 closes output_fd on success, if something \n>> +\t\t\t * goes wrong we close output_fd here to avoid\n>> +\t\t\t * problems.\n>> +\t\t\t */\n>> +\t\t\tclose(output_fd);\n>> +\t\t\tdie(\"could not redirect output\");\n> \n> The comment and close() are probably unnecessary, as you will die()\n> immediately.\n\ndup2() closes the second file, not the first one (but not if it's the\nsame as the first one), so the comment is incorrect. And I agree it's\nOK to just die() without cleaning up, as this is a fatal error anyway\nand an unlikely one on top of that. How about something like this?\n\n\tif (dup2(output_fd, 1) < 0)\n\t\tdie(\"could not redirect output\");\n\tclose(output_fd);\n\nRené\n"}]}