* [PATCH v2] git-archive: Add new option "--output" to write archive to a file instead of stdout.
@ 2009-02-16 17:37 carlos.duclos
2009-02-16 17:59 ` Sverre Rabbelier
2009-02-16 20:11 ` René Scharfe
0 siblings, 2 replies; 4+ messages in thread
From: carlos.duclos @ 2009-02-16 17:37 UTC (permalink / raw)
To: git
NOTE: I can only use a webmail client, so some of the tabs might have overwritten by it. If that's the case I'll resend the patch as MIME attachment.
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.
Signed-off by: Carlos Manuel Duclos Vergara <carlos.duclos@nokia.com>
---
Documentation/git-archive.txt | 3 +
archive-zip.c | 1 -
archive.c | 17 +++++
t/t0024-crlf-archive.sh | 19 +++++
t/t5000-tar-tree.sh | 148 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 187 insertions(+), 1 deletions(-)
diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
index 41cbf9c..04b05f3 100644
--- a/Documentation/git-archive.txt
+++ b/Documentation/git-archive.txt
@@ -47,6 +47,9 @@ OPTIONS
--prefix=<prefix>/::
Prepend <prefix>/ to each filename in the archive.
+--output=<file>::
+ Writes the archive to <file> instead of stdout.
+
<extra>::
This can be any options that the archiver backend understand.
See next section.
diff --git a/archive-zip.c b/archive-zip.c
index cf28504..de5e540 100644
--- a/archive-zip.c
+++ b/archive-zip.c
@@ -266,7 +266,6 @@ int write_zip_archive(struct archiver_args *args)
int err;
dos_time(&args->time, &zip_date, &zip_time);
-
zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
diff --git a/archive.c b/archive.c
index e6de039..fde8602 100644
--- a/archive.c
+++ b/archive.c
@@ -239,6 +239,18 @@ static void parse_treeish_arg(const char **argv,
ar_args->time = archive_time;
}
+static void create_output_file(const char *output_file)
+{
+ int output_fd = creat(output_file, 0666);
+ if (output_fd < 0)
+ die("could not create archive file");
+ if (dup2(output_fd, 1) != 1)
+ {
+ close(output_fd);
+ die("could not redirect output");
+ }
+}
+
#define OPT__COMPR(s, v, h, p) \
{ OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
@@ -253,6 +265,7 @@ static int parse_archive_args(int argc, const char **argv,
const char *base = NULL;
const char *remote = NULL;
const char *exec = NULL;
+ const char *output = NULL;
int compression_level = -1;
int verbose = 0;
int i;
@@ -262,6 +275,7 @@ static int parse_archive_args(int argc, const char **argv,
OPT_STRING(0, "format", &format, "fmt", "archive format"),
OPT_STRING(0, "prefix", &base, "prefix",
"prepend prefix to each pathname in the archive"),
+ OPT_STRING(0, "output", &output, "file", "write the results to this file"),
OPT__VERBOSE(&verbose),
OPT__COMPR('0', &compression_level, "store only", 0),
OPT__COMPR('1', &compression_level, "compress faster", 1),
@@ -294,6 +308,9 @@ static int parse_archive_args(int argc, const char **argv,
if (!base)
base = "";
+ if(output)
+ create_output_file(output);
+
if (list) {
for (i = 0; i < ARRAY_SIZE(archivers); i++)
printf("%s\n", archivers[i].name);
diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
index e533039..237a8f6 100755
--- a/t/t0024-crlf-archive.sh
+++ b/t/t0024-crlf-archive.sh
@@ -26,6 +26,15 @@ test_expect_success 'tar archive' '
'
+test_expect_success 'tar archive output redirected' '
+
+ git archive --format=tar --output=test.tar HEAD &&
+ ( mkdir untarred2 && cd untarred2 && "$TAR" -xf ../test.tar )
+
+ test_cmp sample untarred2/sample
+
+'
+
"$UNZIP" -v >/dev/null 2>&1
if [ $? -eq 127 ]; then
echo "Skipping ZIP test, because unzip was not found"
@@ -43,4 +52,14 @@ test_expect_success 'zip archive' '
'
+test_expect_success 'zip archive output redirected' '
+
+ git archive --format=zip --output=test.zip HEAD &&
+
+ ( mkdir unzipped2 && cd unzipped2 && unzip ../test.zip ) &&
+
+ test_cmp sample unzipped2/sample
+
+'
+
test_done
diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
index c942c8b..b11e504 100755
--- a/t/t5000-tar-tree.sh
+++ b/t/t5000-tar-tree.sh
@@ -66,6 +66,11 @@ test_expect_success \
'remove ignored file' \
'rm a/ignored'
+
+#
+# Tar tests
+#
+
test_expect_success \
'git archive' \
'git archive HEAD >b.tar'
@@ -160,6 +165,91 @@ test_expect_success \
diff a/substfile2 g/prefix/a/substfile2
'
+#
+# Same tests as above but now using redirection
+#
+
+test_expect_success \
+ 'git archive' \
+ 'git archive --output=b20.tar HEAD'
+
+test_expect_success \
+ 'git tar-tree' \
+ 'git tar-tree HEAD >b21.tar'
+
+test_expect_success \
+ 'git archive vs. git tar-tree' \
+ 'diff b20.tar b21.tar'
+
+test_expect_success \
+ 'git archive in a bare repo' \
+ '(cd bare.git && git archive --output=../b22.tar HEAD)'
+
+test_expect_success \
+ 'git archive vs. the same in a bare repo' \
+ 'test_cmp b20.tar b22.tar'
+
+test_expect_success \
+ 'validate file modification time' \
+ 'mkdir extract2 &&
+ "$TAR" xf b20.tar -C extract2 a/a &&
+ test-chmtime -v +0 extract2/a/a |cut -f 1 >b20.mtime &&
+ echo "1117231200" >expected.mtime &&
+ diff expected.mtime b20.mtime'
+
+test_expect_success \
+ 'git get-tar-commit-id' \
+ 'git get-tar-commit-id <b20.tar >b20.commitid &&
+ diff .git/$(git symbolic-ref HEAD) b20.commitid'
+
+test_expect_success \
+ 'extract tar archive' \
+ '(mkdir b20 && cd b20 && "$TAR" xf -) <b20.tar'
+
+test_expect_success \
+ 'validate filenames' \
+ '(cd b20/a && find .) | sort >b20.lst &&
+ diff a.lst b20.lst'
+
+test_expect_success \
+ 'validate file contents' \
+ 'diff -r a b20/a'
+
+test_expect_success \
+ 'create archives with substfiles' \
+ 'echo "substfile?" export-subst >a/.gitattributes &&
+ git archive --output=f20.tar HEAD &&
+ git archive --prefix=prefix/ --output=g20.tar HEAD &&
+ rm a/.gitattributes'
+
+test_expect_success \
+ 'extract substfiles' \
+ '(mkdir f20 && cd f20 && "$TAR" xf -) <f20.tar'
+
+test_expect_success \
+ 'validate substfile contents' \
+ 'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
+ >f20/a/substfile1.expected &&
+ diff f20/a/substfile1.expected f20/a/substfile1 &&
+ diff a/substfile2 f20/a/substfile2
+'
+
+test_expect_success \
+ 'extract substfiles from archive with prefix' \
+ '(mkdir g20 && cd g20 && "$TAR" xf -) <g20.tar'
+
+test_expect_success \
+ 'validate substfile contents from archive with prefix' \
+ 'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
+ >g20/prefix/a/substfile1.expected &&
+ diff g20/prefix/a/substfile1.expected g20/prefix/a/substfile1 &&
+ diff a/substfile2 g20/prefix/a/substfile2
+'
+
+#
+# Zip tests
+#
+
test_expect_success \
'git archive --format=zip' \
'git archive --format=zip HEAD >d.zip'
@@ -172,6 +262,26 @@ test_expect_success \
'git archive --format=zip vs. the same in a bare repo' \
'test_cmp d.zip d1.zip'
+#
+# Same tests as above but now using redirection
+#
+
+test_expect_success \
+ 'git archive --format=zip --output=d10.zip' \
+ 'git archive --format=zip --output=d10.zip HEAD'
+
+test_expect_success \
+ 'git archive --format=zip --output=d11.zip in a bare repo' \
+ '(cd bare.git && git archive --format=zip --output=../d11.zip HEAD)'
+
+test_expect_success \
+ 'git archive --format=zip redirected output vs. the same in a bare repo' \
+ 'test_cmp d10.zip d11.zip'
+
+#
+# Zip tests
+#
+
$UNZIP -v >/dev/null 2>&1
if [ $? -eq 127 ]; then
echo "Skipping ZIP tests, because unzip was not found"
@@ -213,4 +323,42 @@ test_expect_success \
'git archive --list outside of a git repo' \
'GIT_DIR=some/non-existing/directory git archive --list'
+#
+# Same tests as above but now with redirected output
+#
+
+test_expect_success \
+ 'extract ZIP archive from redirected output archive' \
+ '(mkdir d10 && cd d10 && $UNZIP ../d10.zip)'
+
+test_expect_success \
+ 'validate filenames from redirected output archive' \
+ '(cd d10/a && find .) | sort >d10.lst &&
+ diff a.lst d10.lst'
+
+test_expect_success \
+ 'validate file contents from redirected output archive' \
+ 'diff -r a d10/a'
+
+test_expect_success \
+ 'git archive --format=zip with prefix from redirected output archive' \
+ 'git archive --format=zip --prefix=prefix/ --output=e10.zip HEAD'
+
+test_expect_success \
+ 'extract ZIP archive with prefix from redirected output archive' \
+ '(mkdir e10 && cd e10 && $UNZIP ../e10.zip)'
+
+test_expect_success \
+ 'validate filenames with prefix from redirected output archive' \
+ '(cd e10/prefix/a && find .) | sort >e10.lst &&
+ diff a.lst e10.lst'
+
+test_expect_success \
+ 'validate file contents with prefix from redirected output archive' \
+ 'diff -r a e10/prefix/a'
+
+test_expect_success \
+ 'git archive --list outside of a git repo' \
+ 'GIT_DIR=some/non-existing/directory git archive --list'
+
test_done
--
1.6.2.rc0.63.g7cbd0.dirty
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] git-archive: Add new option "--output" to write archive to a file instead of stdout.
2009-02-16 17:37 [PATCH v2] git-archive: Add new option "--output" to write archive to a file instead of stdout carlos.duclos
@ 2009-02-16 17:59 ` Sverre Rabbelier
2009-02-16 20:11 ` René Scharfe
1 sibling, 0 replies; 4+ messages in thread
From: Sverre Rabbelier @ 2009-02-16 17:59 UTC (permalink / raw)
To: carlos.duclos; +Cc: git
Heya,
On Mon, Feb 16, 2009 at 18:37, <carlos.duclos@nokia.com> wrote:
> 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.
>
> Signed-off by: Carlos Manuel Duclos Vergara <carlos.duclos@nokia.com>
> ---
>
> NOTE: I can only use a webmail client, so some of the tabs might
> have overwritten by it. If that's the case I'll resend the patch as
> MIME attachment.
>
> Documentation/git-archive.txt | 3 +
> archive-zip.c | 1 -
> archive.c | 17 +++++
> t/t0024-crlf-archive.sh | 19 +++++
> t/t5000-tar-tree.sh | 148 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 187 insertions(+), 1 deletions(-)
Fixed that for ya :).
Anything that should not go in the commit message should go after the
triple dashes. Also, please wrap your lines at 80 characters. If your
web-client does not support this, please do so manually.
For your convenience, here is a line of 79 characters:
-------------------------------------------------------------------------------
--
Cheers,
Sverre Rabbelier
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] git-archive: Add new option "--output" to write archive to a file instead of stdout.
2009-02-16 17:37 [PATCH v2] git-archive: Add new option "--output" to write archive to a file instead of stdout carlos.duclos
2009-02-16 17:59 ` Sverre Rabbelier
@ 2009-02-16 20:11 ` René Scharfe
2009-02-16 21:38 ` Junio C Hamano
1 sibling, 1 reply; 4+ messages in thread
From: René Scharfe @ 2009-02-16 20:11 UTC (permalink / raw)
To: carlos.duclos; +Cc: git
carlos.duclos@nokia.com schrieb:
> NOTE: I can only use a webmail client, so some of the tabs might have
> overwritten by it. If that's the case I'll resend the patch as MIME
> attachment.
Please do, as applying the patch as-is would be difficult, painful even.
> 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.
>
> Signed-off by: Carlos Manuel Duclos Vergara <carlos.duclos@nokia.com>
> ---
> Documentation/git-archive.txt | 3 +
> archive-zip.c | 1 -
> archive.c | 17 +++++
> t/t0024-crlf-archive.sh | 19 +++++
> t/t5000-tar-tree.sh | 148 +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 187 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
> index 41cbf9c..04b05f3 100644
> --- a/Documentation/git-archive.txt
> +++ b/Documentation/git-archive.txt
> @@ -47,6 +47,9 @@ OPTIONS
> --prefix=<prefix>/::
> Prepend <prefix>/ to each filename in the archive.
>
> +--output=<file>::
> + Writes the archive to <file> instead of stdout.
Please use imperative ("Write") instead of present tense to match the
style used to describe other options.
> +
> <extra>::
> This can be any options that the archiver backend understand.
> See next section.
> diff --git a/archive-zip.c b/archive-zip.c
> index cf28504..de5e540 100644
> --- a/archive-zip.c
> +++ b/archive-zip.c
> @@ -266,7 +266,6 @@ int write_zip_archive(struct archiver_args *args)
> int err;
>
> dos_time(&args->time, &zip_date, &zip_time);
> -
> zip_dir = xmalloc(ZIP_DIRECTORY_MIN_SIZE);
> zip_dir_size = ZIP_DIRECTORY_MIN_SIZE;
>
This hunk doesn't seem to belong in here.
> diff --git a/archive.c b/archive.c
> index e6de039..fde8602 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -239,6 +239,18 @@ static void parse_treeish_arg(const char **argv,
> ar_args->time = archive_time;
> }
>
> +static void create_output_file(const char *output_file)
> +{
> + int output_fd = creat(output_file, 0666);
> + if (output_fd < 0)
> + die("could not create archive file");
Reporting the file name here could help the user to spot typos.
> + if (dup2(output_fd, 1) != 1)
> + {
> + close(output_fd);
> + die("could not redirect output");
> + }
> +}
Style:
if (condition) {
do(something);
...
}
output_fd can be closed after dup2()ing.
A successful dup2() call can return 0 on some systems (mingw here).
> +
> #define OPT__COMPR(s, v, h, p) \
> { OPTION_SET_INT, (s), NULL, (v), NULL, (h), \
> PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, (p) }
> @@ -253,6 +265,7 @@ static int parse_archive_args(int argc, const char **argv,
> const char *base = NULL;
> const char *remote = NULL;
> const char *exec = NULL;
> + const char *output = NULL;
One tab indent, please.
> int compression_level = -1;
> int verbose = 0;
> int i;
> @@ -262,6 +275,7 @@ static int parse_archive_args(int argc, const char **argv,
> OPT_STRING(0, "format", &format, "fmt", "archive format"),
> OPT_STRING(0, "prefix", &base, "prefix",
> "prepend prefix to each pathname in the archive"),
> + OPT_STRING(0, "output", &output, "file", "write the results to this file"),
Please wrap lines to fit into 80 columns, like it was done for the
preceding option. And perhaps replace "results" with "archive"?
> OPT__VERBOSE(&verbose),
> OPT__COMPR('0', &compression_level, "store only", 0),
> OPT__COMPR('1', &compression_level, "compress faster", 1),
> @@ -294,6 +308,9 @@ static int parse_archive_args(int argc, const char **argv,
> if (!base)
> base = "";
>
> + if(output)
> + create_output_file(output);
> +
Ah, you make it possible to write the list of formats into a file,
too, as Junio mentioned. So the "results" above is correct after
all. But is this intended?
> if (list) {
> for (i = 0; i < ARRAY_SIZE(archivers); i++)
> printf("%s\n", archivers[i].name);
> diff --git a/t/t0024-crlf-archive.sh b/t/t0024-crlf-archive.sh
> index e533039..237a8f6 100755
> --- a/t/t0024-crlf-archive.sh
> +++ b/t/t0024-crlf-archive.sh
> @@ -26,6 +26,15 @@ test_expect_success 'tar archive' '
>
> '
>
> +test_expect_success 'tar archive output redirected' '
> +
> + git archive --format=tar --output=test.tar HEAD &&
> + ( mkdir untarred2 && cd untarred2 && "$TAR" -xf ../test.tar )
> +
> + test_cmp sample untarred2/sample
> +
> +'
> +
> "$UNZIP" -v >/dev/null 2>&1
> if [ $? -eq 127 ]; then
> echo "Skipping ZIP test, because unzip was not found"
> @@ -43,4 +52,14 @@ test_expect_success 'zip archive' '
>
> '
>
> +test_expect_success 'zip archive output redirected' '
> +
> + git archive --format=zip --output=test.zip HEAD &&
> +
> + ( mkdir unzipped2 && cd unzipped2 && unzip ../test.zip ) &&
> +
> + test_cmp sample unzipped2/sample
> +
> +'
> +
> test_done
> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> index c942c8b..b11e504 100755
> --- a/t/t5000-tar-tree.sh
> +++ b/t/t5000-tar-tree.sh
> @@ -66,6 +66,11 @@ test_expect_success \
> 'remove ignored file' \
> 'rm a/ignored'
>
> +
> +#
> +# Tar tests
> +#
> +
> test_expect_success \
> 'git archive' \
> 'git archive HEAD >b.tar'
> @@ -160,6 +165,91 @@ test_expect_success \
> diff a/substfile2 g/prefix/a/substfile2
> '
>
> +#
> +# Same tests as above but now using redirection
> +#
> +
> +test_expect_success \
> + 'git archive' \
> + 'git archive --output=b20.tar HEAD'
> +
> +test_expect_success \
> + 'git tar-tree' \
> + 'git tar-tree HEAD >b21.tar'
> +
> +test_expect_success \
> + 'git archive vs. git tar-tree' \
> + 'diff b20.tar b21.tar'
> +
> +test_expect_success \
> + 'git archive in a bare repo' \
> + '(cd bare.git && git archive --output=../b22.tar HEAD)'
> +
> +test_expect_success \
> + 'git archive vs. the same in a bare repo' \
> + 'test_cmp b20.tar b22.tar'
> +
> +test_expect_success \
> + 'validate file modification time' \
> + 'mkdir extract2 &&
> + "$TAR" xf b20.tar -C extract2 a/a &&
> + test-chmtime -v +0 extract2/a/a |cut -f 1 >b20.mtime &&
> + echo "1117231200" >expected.mtime &&
> + diff expected.mtime b20.mtime'
> +
> +test_expect_success \
> + 'git get-tar-commit-id' \
> + 'git get-tar-commit-id <b20.tar >b20.commitid &&
> + diff .git/$(git symbolic-ref HEAD) b20.commitid'
> +
> +test_expect_success \
> + 'extract tar archive' \
> + '(mkdir b20 && cd b20 && "$TAR" xf -) <b20.tar'
> +
> +test_expect_success \
> + 'validate filenames' \
> + '(cd b20/a && find .) | sort >b20.lst &&
> + diff a.lst b20.lst'
> +
> +test_expect_success \
> + 'validate file contents' \
> + 'diff -r a b20/a'
> +
> +test_expect_success \
> + 'create archives with substfiles' \
> + 'echo "substfile?" export-subst >a/.gitattributes &&
> + git archive --output=f20.tar HEAD &&
> + git archive --prefix=prefix/ --output=g20.tar HEAD &&
> + rm a/.gitattributes'
> +
> +test_expect_success \
> + 'extract substfiles' \
> + '(mkdir f20 && cd f20 && "$TAR" xf -) <f20.tar'
> +
> +test_expect_success \
> + 'validate substfile contents' \
> + 'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
> + >f20/a/substfile1.expected &&
> + diff f20/a/substfile1.expected f20/a/substfile1 &&
> + diff a/substfile2 f20/a/substfile2
> +'
> +
> +test_expect_success \
> + 'extract substfiles from archive with prefix' \
> + '(mkdir g20 && cd g20 && "$TAR" xf -) <g20.tar'
> +
> +test_expect_success \
> + 'validate substfile contents from archive with prefix' \
> + 'git log --max-count=1 "--pretty=format:A${SUBSTFORMAT}O" HEAD \
> + >g20/prefix/a/substfile1.expected &&
> + diff g20/prefix/a/substfile1.expected g20/prefix/a/substfile1 &&
> + diff a/substfile2 g20/prefix/a/substfile2
> +'
> +
> +#
> +# Zip tests
> +#
> +
> test_expect_success \
> 'git archive --format=zip' \
> 'git archive --format=zip HEAD >d.zip'
> @@ -172,6 +262,26 @@ test_expect_success \
> 'git archive --format=zip vs. the same in a bare repo' \
> 'test_cmp d.zip d1.zip'
>
> +#
> +# Same tests as above but now using redirection
> +#
> +
> +test_expect_success \
> + 'git archive --format=zip --output=d10.zip' \
> + 'git archive --format=zip --output=d10.zip HEAD'
> +
> +test_expect_success \
> + 'git archive --format=zip --output=d11.zip in a bare repo' \
> + '(cd bare.git && git archive --format=zip --output=../d11.zip HEAD)'
> +
> +test_expect_success \
> + 'git archive --format=zip redirected output vs. the same in a bare repo' \
> + 'test_cmp d10.zip d11.zip'
> +
> +#
> +# Zip tests
> +#
> +
> $UNZIP -v >/dev/null 2>&1
> if [ $? -eq 127 ]; then
> echo "Skipping ZIP tests, because unzip was not found"
> @@ -213,4 +323,42 @@ test_expect_success \
> 'git archive --list outside of a git repo' \
> 'GIT_DIR=some/non-existing/directory git archive --list'
>
> +#
> +# Same tests as above but now with redirected output
> +#
> +
> +test_expect_success \
> + 'extract ZIP archive from redirected output archive' \
> + '(mkdir d10 && cd d10 && $UNZIP ../d10.zip)'
> +
> +test_expect_success \
> + 'validate filenames from redirected output archive' \
> + '(cd d10/a && find .) | sort >d10.lst &&
> + diff a.lst d10.lst'
> +
> +test_expect_success \
> + 'validate file contents from redirected output archive' \
> + 'diff -r a d10/a'
> +
> +test_expect_success \
> + 'git archive --format=zip with prefix from redirected output archive' \
> + 'git archive --format=zip --prefix=prefix/ --output=e10.zip HEAD'
> +
> +test_expect_success \
> + 'extract ZIP archive with prefix from redirected output archive' \
> + '(mkdir e10 && cd e10 && $UNZIP ../e10.zip)'
> +
> +test_expect_success \
> + 'validate filenames with prefix from redirected output archive' \
> + '(cd e10/prefix/a && find .) | sort >e10.lst &&
> + diff a.lst e10.lst'
> +
> +test_expect_success \
> + 'validate file contents with prefix from redirected output archive' \
> + 'diff -r a e10/prefix/a'
> +
> +test_expect_success \
> + 'git archive --list outside of a git repo' \
> + 'GIT_DIR=some/non-existing/directory git archive --list'
> +
> test_done
> --
> 1.6.2.rc0.63.g7cbd0.dirty
>
Lot's of tests, nice! But I think comparing the created archive to
the one written to stdout using the same parameters is sufficient.
There's no need to untar, unzip or otherwise look at the archives
again if they are the the same.
René
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] git-archive: Add new option "--output" to write archive to a file instead of stdout.
2009-02-16 20:11 ` René Scharfe
@ 2009-02-16 21:38 ` Junio C Hamano
0 siblings, 0 replies; 4+ messages in thread
From: Junio C Hamano @ 2009-02-16 21:38 UTC (permalink / raw)
To: René Scharfe; +Cc: carlos.duclos, git
René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:
> carlos.duclos@nokia.com schrieb:
>> NOTE: I can only use a webmail client, so some of the tabs might have
>> overwritten by it. If that's the case I'll resend the patch as MIME
>> attachment.
>
> Please do, as applying the patch as-is would be difficult, painful even.
Thanks. I agree with everything you said in your review, and adding a bit
more.
>> diff --git a/archive.c b/archive.c
>> index e6de039..fde8602 100644
>> --- a/archive.c
>> +++ b/archive.c
>> @@ -239,6 +239,18 @@ static void parse_treeish_arg(const char **argv,
>> ar_args->time = archive_time;
>> }
>>
>> +static void create_output_file(const char *output_file)
>> +{
>> + int output_fd = creat(output_file, 0666);
"git grep -n -w -e 'creat' -- '*.c'" shows nothing; we seem to prefer
using a longhand:
open(path, O_CREAT | O_WRONLY | O_TRUNC, 0666);
instead. Personally I see nothing wrong in creat(2) per-se, but let's be
consistent.
>> + if (dup2(output_fd, 1) != 1)
>> + {
>> + close(output_fd);
>> + die("could not redirect output");
>> + }
>> +}
>
> Style:
> if (condition) {
> do(something);
> ...
> }
>
> output_fd can be closed after dup2()ing.
If the user is sick enough to close fd#1 from the shell when running
archive, it could already be pointing at fd#1 ;-)
> A successful dup2() call can return 0 on some systems (mingw here).
Yikes.
The logic would become:
fd = creat();
if (fd < 0)
die();
if (fd != 1) {
if (dup2(fd, 1) < 0 || close(fd))
die();
}
>> @@ -294,6 +308,9 @@ static int parse_archive_args(int argc, const char **argv,
>> if (!base)
>> base = "";
>>
>> + if(output)
Style: if (output)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2009-02-16 21:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-02-16 17:37 [PATCH v2] git-archive: Add new option "--output" to write archive to a file instead of stdout carlos.duclos
2009-02-16 17:59 ` Sverre Rabbelier
2009-02-16 20:11 ` René Scharfe
2009-02-16 21:38 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).