threads / patch / 20967

patcharchive: Refuse to write the archive to a terminal.

Subject: [PATCH] archive: Refuse to write the archive to a terminal.

## tl;dr

7 messages between Sep 16, 2009 and Sep 17, 2009. Diffs are folded; open one to read it.

replies: 6people: 5as markdown or json

Josh Triplett· Sep 16, 2009, 10:31 UTC · lore

If not given the -o/--output option, git archive writes the archive to stdout. This proves unhelpful if not redirected or piped somewhere. Rather than spewing binary at the user's terminal, die with an appropriate message.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---

I considered adding a -f/--force option, like gzip has, but writing an archive to a tty seems like a sufficiently insane use case that I'll let whoever actually needs that write the patch for it. ;)

 builtin-archive.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to builtin-archive.c +3 −0
diff --git a/builtin-archive.c b/builtin-archive.c
index 12351e9..73accd0 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -101,6 +101,9 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 		create_output_file(output);
 		if (!format)
 			format = format_from_name(output);
+	} else if (isatty(1)) {
+		die("Archive not written to a terminal.\n"
+		    "Specify output filename or redirect output.");
 	}
 
 	if (format) {

-- 
1.6.3.3
Johannes Sixt· Sep 16, 2009, 11:11 UTC · re: Josh Triplett · lore

Re: [PATCH] archive: Refuse to write the archive to a terminal.

Josh Triplett schrieb:
> I considered adding a -f/--force option, like gzip has, but writing an
> archive to a tty seems like a sufficiently insane use case that I'll let
> whoever actually needs that write the patch for it. ;)
How about '--output -' instead?
-- Hannes
Mikael Magnusson· Sep 16, 2009, 11:27 UTC · re: Johannes Sixt · lore

Re: [PATCH] archive: Refuse to write the archive to a terminal.

2009/9/16 Johannes Sixt <j.sixt@viscovery.net>:
Show 6 quoted lines
> Josh Triplett schrieb:
>> I considered adding a -f/--force option, like gzip has, but writing an
>> archive to a tty seems like a sufficiently insane use case that I'll let
>> whoever actually needs that write the patch for it. ;)
>
> How about '--output -' instead?
You could always just add '|cat'.
-- 
Mikael Magnusson
Reece Dunn· Sep 16, 2009, 11:48 UTC · re: Mikael Magnusson · lore

Re: [PATCH] archive: Refuse to write the archive to a terminal.

2009/9/16 Mikael Magnusson <mikachu@gmail.com>:
Show 9 quoted lines
> 2009/9/16 Johannes Sixt <j.sixt@viscovery.net>:
>> Josh Triplett schrieb:
>>> I considered adding a -f/--force option, like gzip has, but writing an
>>> archive to a tty seems like a sufficiently insane use case that I'll let
>>> whoever actually needs that write the patch for it. ;)
>>
>> How about '--output -' instead?
>
> You could always just add '|cat'.

Except when running on Windows. Yes MSYS and cygwin provide a version of cat, but this cannot be guaranteed (e.g. with the series to support building with MSVC).

The `--output -` / `-o -` syntax looks reasonable (the issue with using -f/--force is: what are you forcing the operation of?). Is - used elsewhere in git for specifying stdout?

Also, the die message might be more useful (and in keeping with the other git commands) by showing the 'inline context help'; something like:

    Failed to generate the archive: output is a terminal.
    Please specify the file to write to (using `-o archive.tar`) or
redirect the output (e.g. `... | gzip`).
    If you want to write the archive out to the terminal, use `-o -`
to force the operation.
- Reece
Matthieu Moy· Sep 16, 2009, 12:57 UTC · re: Reece Dunn · lore

Re: [PATCH] archive: Refuse to write the archive to a terminal.

Reece Dunn <msclrhd@googlemail.com> writes:
Show 14 quoted lines
> 2009/9/16 Mikael Magnusson <mikachu@gmail.com>:
>> 2009/9/16 Johannes Sixt <j.sixt@viscovery.net>:
>>> Josh Triplett schrieb:
>>>> I considered adding a -f/--force option, like gzip has, but writing an
>>>> archive to a tty seems like a sufficiently insane use case that I'll let
>>>> whoever actually needs that write the patch for it. ;)
>>>
>>> How about '--output -' instead?
>>
>> You could always just add '|cat'.
>
> Except when running on Windows. Yes MSYS and cygwin provide a version
> of cat, but this cannot be guaranteed (e.g. with the series to support
> building with MSVC).

In general, autodectection features sometimes fail, so it's good to have an explicit override option.

> The `--output -` / `-o -` syntax looks reasonable
I like this too.
-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
Josh Triplett· Sep 17, 2009, 01:49 UTC · re: Johannes Sixt · lore

Re: [PATCH] archive: Refuse to write the archive to a terminal.

On Wed, Sep 16, 2009 at 01:11:26PM +0200, Johannes Sixt wrote:
Show 6 quoted lines
> Josh Triplett schrieb:
> > I considered adding a -f/--force option, like gzip has, but writing an
> > archive to a tty seems like a sufficiently insane use case that I'll let
> > whoever actually needs that write the patch for it. ;)
> 
> How about '--output -' instead?

Yeah, that seems significantly better than --force. Though I don't particularly care for the '-' convention to mean 'stdout'; in principle that ought to create a file named '-' in the current directory. /dev/stdout makes more sense, and doesn't require any work on git's part beyond this patch.

- Josh Triplett
Johannes Sixt· Sep 17, 2009, 05:53 UTC · re: Josh Triplett · lore

Re: [PATCH] archive: Refuse to write the archive to a terminal.

Josh Triplett schrieb:
Show 8 quoted lines
> On Wed, Sep 16, 2009 at 01:11:26PM +0200, Johannes Sixt wrote:
>> How about '--output -' instead?
> 
> Yeah, that seems significantly better than --force.  Though I don't
> particularly care for the '-' convention to mean 'stdout'; in principle
> that ought to create a file named '-' in the current directory.
> /dev/stdout makes more sense, and doesn't require any work on git's
> part beyond this patch.

Except that /dev/stdout is not portable. You can always say --output ./- if you want an oddly named file in the current directory.

-- Hannes

← back to recent threads