From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "René Scharfe" <l.s.r@web.de>
Subject: Re: [RFC/PATCH] archive: "--list" does not take further options
Date: Thu, 21 Dec 2023 03:58:08 -0500 [thread overview]
Message-ID: <20231221085808.GC545870@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqttocp98r.fsf@gitster.g>
On Wed, Dec 20, 2023 at 06:41:56PM -0800, Junio C Hamano wrote:
> ---
> * This was done to convince myself that even though cmd_archive()
> calls parse_options with PARSE_OPT_KEEP_UNKNOWN_OPT and then
> uses the resulting argc/argv without apparent filtering of the
> "--end-of-options" thing, it is correctly handling it, either
> locally or remotely.
>
> - locally, write_archive() uses parse_archive_args(), which calls
> parse_options() without KEEP_UNKNOWN_OPT and "--end-of-options"
> is handled there just fine.
Right. Not only is it handled by the second parser, but _not_ keeping it
would be a bug, because that second parser would have no idea that we
saw end-of-options (and so "git archive --end-of-options --foo" would
try to treat "--foo" as an option).
The recent commit 9385174627 did fix a case like this for fast-export,
but git-archive was not changed. It passed KEEP_DASHDASH along with
KEEP_UNKNOWN_OPT, so it already retained and passed along
--end-of-options.
> - remotely, run_remote_archiver() relays the local parameters,
> including "--end-of-options" via the "argument" packet. Then
> the arguments are assembled into a strvec and used by the
> upload-archive running on the other side to spawn an
> upload-archive--writer process with.
> cmd_upload_archive_writer() then makes the same write_archive()
> call; "--end-of-options" would still be in argv[] if the
> original "git archive --remote" invocation had one, but it is
> consumed the same way as the local case in write_archive() by
> calling parse_archive_args().
Right, and this is just the same case but with a lot of complicated
network-ferrying in between. :) We must retain --end-of-options so that
the next parser knows to stop treating them as arguments. And because it
doesn't use KEEP_UNKNOWN_OPT, the ferried "--end-of-options" is removed
then.
> diff --git c/archive.c w/archive.c
> index 9aeaf2bd87..3244e9f9f2 100644
> --- c/archive.c
> +++ w/archive.c
> @@ -641,6 +641,13 @@ static int parse_archive_args(int argc, const char **argv,
> base = "";
>
> if (list) {
> + if (argc) {
> + if (!is_remote)
> + die(_("extra command line parameter '%s'"), *argv);
> + else
> + printf("!ERROR! extra command line parameter '%s'\n",
> + *argv);
> + }
This general direction seems reasonable to me, since we're letting the
user know that their extra argument was ignored (though note that if it
was a misspelled option, like "--otuput", we would complain about that).
It's largely orthogonal to end-of-options, but I see how you got here by
wondering about it. :)
-Peff
next prev parent reply other threads:[~2023-12-21 8:58 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-20 23:19 [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add Junio C Hamano
2023-12-20 23:55 ` Josh Steadmon
2023-12-21 2:46 ` Junio C Hamano
2023-12-21 8:40 ` Jeff King
2023-12-21 17:02 ` Junio C Hamano
2023-12-21 21:45 ` Jeff King
2023-12-21 22:04 ` Junio C Hamano
2023-12-23 10:02 ` Jeff King
2023-12-23 15:38 ` rsbecker
2023-12-23 22:45 ` Elijah Newren
2023-12-24 1:02 ` Elijah Newren
2023-12-21 2:41 ` [RFC/PATCH] archive: "--list" does not take further options Junio C Hamano
2023-12-21 7:30 ` René Scharfe
2023-12-21 8:59 ` Jeff King
2023-12-21 18:13 ` Junio C Hamano
2023-12-21 21:35 ` Jeff King
2023-12-21 8:58 ` Jeff King [this message]
2023-12-21 8:36 ` [PATCH/RFC] sparse-checkout: take care of "--end-of-options" in set/add Jeff King
2023-12-21 18:20 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231221085808.GC545870@coredump.intra.peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=l.s.r@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).