On Fri, Mar 03, 2023 at 03:05:14PM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> 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 ;-)
I wasn't thinking of changing the behavior for input, but just focusing
the docs in the right spot (the "create" option), like:
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
