Re: `git bundle create -` may not write to `stdout`

From

Jeff King <peff@peff.net>

Date

Mar 4, 2023, 01:28 UTC

Message-ID

<ZAKexHiit5vOmv7M@coredump.intra.peff.net>

In-Reply-To

<xmqqpm9pcu6t.fsf@gitster.g>

On Fri, Mar 03, 2023 at 03:05:14PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > This test looks good to me. Let's also not forget about the doc fixes. I
> > don't think there's much urgency to get this into v2.40,
> 
> Doc?  Meaning 
> 
> 	<file> can be "-" to mean the standard output (for writing)
> 	or the standard input (for reading)
> 
> or something?

Yeah, I was referring to my earlier mail in the thread, which said:

>> So it seems like we'd want a three-patch series:
>> 
>>   1. The first hunk I showed above, along with a test to demonstrate the
>>      fix.
>> 
>>   2. Remove bogus references to --stdout in the docs.
>> 
>>   3. Document "-".

Your patch is (1), but we'd want (2) and (3) still.

> Given that the other three subcommands also take <file>
> 
>     'git bundle' create [-q | --quiet | --progress | --all-progress] ...
>                         [--version=<version>] <file> <git-rev-list-args>
>     'git bundle' verify [-q | --quiet] <file>
>     'git bundle' list-heads <file> [<refname>...]
>     'git bundle' unbundle [--progress] <file> [<refname>...]
> 
> but read_bundle_header() function all three calls begins like so:
> 
>     int read_bundle_header(const char *path, struct bundle_header *header)
>     {
>             int fd = open(path, O_RDONLY);
> 
>             if (fd < 0)
>                     return error(_("could not open '%s'"), path);
>             return read_bundle_header_fd(fd, header, path);
>     }
> 
> this function needs to be fixed first ;-)

diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt
index 18a022b4b4..ea6b5c24d1 100644
--- a/Documentation/git-bundle.txt
+++ b/Documentation/git-bundle.txt
@@ -65,9 +65,9 @@ OPTIONS
 create [options] <file> <git-rev-list-args>::
 	Used to create a bundle named 'file'.  This requires the
 	'<git-rev-list-args>' arguments to define the bundle contents.
 	'options' contains the options specific to the 'git bundle create'
-	subcommand.
+	subcommand. If 'file' is `-`, the bundle is written to stdout.
 
 verify <file>::
 	Used to check that a bundle file is valid and will apply
 	cleanly to the current repository.  This includes checks on the

> > but I can put
> > it together in the next day or three.
> 
> Thanks.  Just for reference, here is what I have (just a log
> message, the patch is the same and does not support input yet).

I don't mind supporting "-" for input, but I don't think it's strictly
necessary and nobody is really asking for it. I'm also not sure it won't
run afoul of problems in the lower-level code. I seem to recall that the
bundle code may want to seek() on read, but a quick grep doesn't seem to
turn anything up (so I'm not sure if I'm mis-remembering or just didn't
look hard enough).

> ----- >8 -----
> Subject: [PATCH] bundle: don't blindly apply prefix_filename() to "-"

Thanks.

-Peff