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

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Josh Triplett-2
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 <[hidden email]>
---

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(-)

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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Johannes Sixt-2
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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Mikael Magnusson
2009/9/16 Johannes Sixt <[hidden email]>:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Reece Dunn
2009/9/16 Mikael Magnusson <[hidden email]>:
> 2009/9/16 Johannes Sixt <[hidden email]>:
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Matthieu Moy-2
Reece Dunn <[hidden email]> writes:

> 2009/9/16 Mikael Magnusson <[hidden email]>:
>> 2009/9/16 Johannes Sixt <[hidden email]>:
>>> 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/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Josh Triplett-2
In reply to this post by Johannes Sixt-2
On Wed, Sep 16, 2009 at 01:11:26PM +0200, Johannes Sixt wrote:
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Johannes Sixt-2
Josh Triplett schrieb:
> 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html