All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: "René Scharfe" <l.s.r@web.de>, "Jeff King" <peff@peff.net>
Subject: [RFC/PATCH] archive: "--list" does not take further options
Date: Wed, 20 Dec 2023 18:41:56 -0800	[thread overview]
Message-ID: <xmqqttocp98r.fsf@gitster.g> (raw)
In-Reply-To: <xmqqbkakqx6s.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 20 Dec 2023 15:19:23 -0800")

"git archive --list blah" should notice an extra command line
parameter that goes unused.  Make it so.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * 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.

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

   I do not like the remote error behaviour this one adds at all.
   Do we use a more proper mechanism to propagate a remote error
   back for other subcommands we can reuse here?

 archive.c           |  7 +++++++
 t/t5000-tar-tree.sh | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

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);
+		}
 		for (i = 0; i < nr_archivers; i++)
 			if (!is_remote || archivers[i]->flags & ARCHIVER_REMOTE)
 				printf("%s\n", archivers[i]->name);
diff --git c/t/t5000-tar-tree.sh w/t/t5000-tar-tree.sh
index 918a2fc7c6..04592f45b0 100755
--- c/t/t5000-tar-tree.sh
+++ w/t/t5000-tar-tree.sh
@@ -124,6 +124,20 @@ test_expect_success 'setup' '
 	EOF
 '
 
+test_expect_success '--list notices extra parameters' '
+	test_must_fail git archive --list blah &&
+	# NEEDSWORK: remote error does not result in non-zero
+	# exit, which we might want to change later.
+	git archive --remote=. --list blah >remote-out &&
+	grep "!ERROR! " remote-out
+'
+
+test_expect_success 'end-of-options is correctly eaten' '
+	git archive --list --end-of-options &&
+	git archive --remote=. --list --end-of-options >remote-out &&
+	! grep "!ERROR! " remote-out
+'
+
 test_expect_success 'populate workdir' '
 	mkdir a &&
 	echo simple textfile >a/a &&

  parent reply	other threads:[~2023-12-21  2:42 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 ` Junio C Hamano [this message]
2023-12-21  7:30   ` [RFC/PATCH] archive: "--list" does not take further options 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
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=xmqqttocp98r.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.