carlos.duclos@xxxxxxxxx schrieb:
> Hi,
>
> Patch attached as MIME.
>
> Patch highlights:
> 1. Remove duplicated tests
> 1.1 Tests are present on t0024.
> 1.2 Introduced some comments in t5000 to make it easy to read.
> 2. Removed close(2) and comments from archive.c
> 3. Rewrote commit message.
> 4. Signed off.
>
> Cheers!
>
> From 3a143244b84d80adba91f37307e30ec8fb3a6701 Mon Sep 17 00:00:00 2001
> From: Carlos Manuel Duclos Vergara <carlos.duclos@xxxxxxxxx>
> Date: Mon, 16 Feb 2009 18:20:25 +0100
> Subject: [PATCH] git-archive: add --output=<file> to send output to a file instead of stdout.
> 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@xxxxxxxxx>
> ---
> Documentation/git-archive.txt | 3 +++
> archive.c | 16 ++++++++++++++++
> t/t0024-crlf-archive.sh | 19 +++++++++++++++++++
> t/t5000-tar-tree.sh | 8 ++++++++
> 4 files changed, 46 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt
> index 41cbf9c..5bde197 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>::
> + Write 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.c b/archive.c
> index e6de039..3a646e5 100644
> --- a/archive.c
> +++ b/archive.c
> @@ -239,6 +239,16 @@ 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 = open(output_file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
> + if (output_fd < 0)
> + die("could not create archive file: %s ", output_file);
> + if (output_fd != 1)
> + if (dup2(output_fd, 1) < 0)
> + die("could not redirect output");
> +}
> +
output_fd is leaked. Sure, it points to the same file as stdout, but I
don't see why we would want to keep two handles open. (Leaking when we
die() is OK, though.)
> 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
I had something even more simple in mind. In t5000 there are these tests:
test_expect_success \
'git archive' \
'git archive HEAD >b.tar'
test_expect_success \
'git tar-tree' \
'git tar-tree HEAD >b2.tar'
test_expect_success \
'git archive vs. git tar-tree' \
'diff b.tar b2.tar'
Could you add another one for archive with --output? (The "vs." test
could be switched to test_cmp and merged with the second one, but this
should be done in a separate patch.) The same can be done for zip. No
untarring or unzipping needed.
t5000 is the right file for these tests, by the way, as they are not
about CR/LF issues, but basic archive functionality.
> +
> +'
> +
> test_done
> diff --git a/t/t5000-tar-tree.sh b/t/t5000-tar-tree.sh
> index c942c8b..a74dd4a 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'
> @@ -159,6 +164,9 @@ test_expect_success \
> diff g/prefix/a/substfile1.expected g/prefix/a/substfile1 &&
> diff a/substfile2 g/prefix/a/substfile2
> '
> +#
> +# Zip tests
> +#
>
> test_expect_success \
> 'git archive --format=zip' \
> --
> 1.6.2.rc0.63.g7cbd0.dirty
>
These comments are nice additions, but they don't belong in this patch.
Even better would be to move the zip tests into their own file (also in
a separate patch).
We're getting there. :)
Thanks,
René
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
