From nobody Wed Apr 30 01:01:02 2025 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4ZnJlf3fjrz5vpDm; Wed, 30 Apr 2025 01:01:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4ZnJlf1T31z3t4D; Wed, 30 Apr 2025 01:01:02 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1745974862; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=GQ8EH3cNtppOMvLmj03D/MjzVnv+h97U02GYJyfq+cw=; b=kBhwAIaX05+CKk3q+XPKeX1tSzd4gXxc1Ljbfn8KSyubmqq7PzDWHI/+0g6YaCOP13dw+I s84nsvJu1JZVJXmy1+AZZZjcMoKu/C0IKI7vwrVwZc+QBGQSUFOE7OTwf6kyy61nvrj9bm 7lRpCyMKTHc0msjHUSI9eJJ7SD+vmJBE2eSJQ20vAWKf4B/vANWH/v8krN4XEJr+DIVRca f2bV+rWj+mleLV3FTz2370EFxSxGnWXczLPgpWkr3JXu/d9BnrHa3TgJ8G2j9QV/5bQ31R bmdW5ZqL4WV17stMtFhME+qi5nE2xg+kuV5JbfOiaUeUjE3nWed5hvxnU3Y1pw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1745974862; a=rsa-sha256; cv=none; b=Xv8feb8iqdkl+0adSwLyxNuSBfUMXT72YZGefYszovNDv8zghRglyt6inTRAgTs1e928sh 11u+GCm4Qwe63vw1d4wf4OBmoM0cmEQZYK88y3H6uyz8f3C5qKsge2ljloqLrWsPfVnDm/ fapm4LD6EISdJiQSkPsV/nhz9u6Nz2dblLi/P2iFtCbmxnHXoKnL0zEwduTwGvLiEy8q28 jq0g6Y8CZRoaqcc14c8Wy7gVxZkOACW956u1idESZa8s/nIABVNN71GlPgkWEPwwifuBS0 GIJCSP9a77VflA+77VGxnSh+tvPYjItNdnfJ3VsIFkhN7Aw1GU/+ANNRlXpZ/A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1745974862; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=GQ8EH3cNtppOMvLmj03D/MjzVnv+h97U02GYJyfq+cw=; b=ZovPlfHPWpFsFzvyslp4F5a/uqjh2Tz2ERmgPQzMd12CUnuk6lXp7kpRYfUMJ2Sd9il7K6 Rqu66B1rOLcPi+ITw3EpFuYEv9WKKq4mkEQDLMhgwOxeJpDY7QhiUCy/xK7lOGvfgSQUiC /vXCiAKNddtZWLqAU2DTo3yYHHiIgHtxHtgD2mluOoIbitcxEfkjdEpTLrrtpTetchr+Oj 0mp96l/MTiiXEhyAC7cCydL4ZIXaax14RAB70q4DQoGJN08/+sKhQsRF5kZ1fFOkZwFPHx NnuaUImybGNQX/K4IpqBxaATHDNa18UnaY9khnZccdQxDC7KBE4JSFm3+5wmcQ== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4ZnJlf13lHzl9S; Wed, 30 Apr 2025 01:01:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 53U112KX053509; Wed, 30 Apr 2025 01:01:02 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 53U112aW053506; Wed, 30 Apr 2025 01:01:02 GMT (envelope-from git) Date: Wed, 30 Apr 2025 01:01:02 GMT Message-Id: <202504300101.53U112aW053506@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Ed Maste Subject: git: be61deae0aa2 - main - pkg: clarify argument parsing List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: emaste X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: be61deae0aa2923bb9908fc5b37b35712603e622 Auto-Submitted: auto-generated The branch main has been updated by emaste: URL: https://cgit.FreeBSD.org/src/commit/?id=be61deae0aa2923bb9908fc5b37b35712603e622 commit be61deae0aa2923bb9908fc5b37b35712603e622 Author: Isaac Freund AuthorDate: 2025-04-23 10:27:35 +0000 Commit: Ed Maste CommitDate: 2025-04-30 01:00:17 +0000 pkg: clarify argument parsing Currently pkg parses all arguments and subcommands in a single getopt_long() loop. This results in quite a bit of accidental complexity, especially around the -r option for the bootstrap and add subcommands. This also results in the undocumented behavior of all options understood by pkg(7) being accepted anywhere in the argument list regardless of their position. This contradicts the behavior of pkg(8) and I find it quite unexpected. This patch splits the argument parsing done by pkg(7) into two phases for global options and subcommand options. This gives behavior that matches pkg(8), accepting the subset of pkg(8) options supported by pkg(7) in the exact same positions that pkg(8) accepts them. This is however a somewhat breaking change, as options are no longer accepted by pkg(7) in places they are rejected by pkg(8). For example, `pkg -f bootstrap` no longer works, only `pkg bootstrap -f`. The original motivation for writing this patch was investigating support for --rootdir in pkg(7). Since pkg(8) uses -r as the short form for both --rootdir and --repository depending on the position, I decided that pkg(7) needed to be brought inline with pkg(8)'s argument parsing behavior to avoid confusion. This patch also gets rid of args_add_message (unused since 40b9f924b1) Relnotes: yes Reviewed by: bapt Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D49977 --- usr.sbin/pkg/pkg.7 | 5 +- usr.sbin/pkg/pkg.c | 174 ++++++++++++++++++++++++++--------------------------- 2 files changed, 87 insertions(+), 92 deletions(-) diff --git a/usr.sbin/pkg/pkg.7 b/usr.sbin/pkg/pkg.7 index 467f62d5747b..d2246f74a3fc 100644 --- a/usr.sbin/pkg/pkg.7 +++ b/usr.sbin/pkg/pkg.7 @@ -22,7 +22,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd March 30, 2025 +.Dd April 29, 2025 .Dt PKG 7 .Os .Sh NAME @@ -33,8 +33,9 @@ .Op Fl d .Ar command ... .Nm +.Op Fl d .Cm add -.Op Fl dfy +.Op Fl fy .Op Fl r Ar reponame .Ar pkg.pkg .Nm diff --git a/usr.sbin/pkg/pkg.c b/usr.sbin/pkg/pkg.c index 92fdbf0ebff8..b62eecfc6dce 100644 --- a/usr.sbin/pkg/pkg.c +++ b/usr.sbin/pkg/pkg.c @@ -942,10 +942,6 @@ static const char args_bootstrap_message[] = "Too many arguments\n" "Usage: pkg [-4|-6] bootstrap [-f] [-y]\n"; -static const char args_add_message[] = -"Too many arguments\n" -"Usage: pkg add [-f] [-y] {pkg.pkg}\n"; - static int pkg_query_yes_no(void) { @@ -1072,137 +1068,138 @@ int main(int argc, char *argv[]) { char pkgpath[MAXPATHLEN]; + char **original_argv; const char *pkgarg, *repo_name; bool activation_test, add_pkg, bootstrap_only, force, yes; signed char ch; const char *fetchOpts; - char *command; struct repositories *repositories; activation_test = false; add_pkg = false; bootstrap_only = false; - command = NULL; fetchOpts = ""; force = false; + original_argv = argv; pkgarg = NULL; repo_name = NULL; yes = false; struct option longopts[] = { { "debug", no_argument, NULL, 'd' }, - { "force", no_argument, NULL, 'f' }, { "only-ipv4", no_argument, NULL, '4' }, { "only-ipv6", no_argument, NULL, '6' }, - { "yes", no_argument, NULL, 'y' }, { NULL, 0, NULL, 0 }, }; snprintf(pkgpath, MAXPATHLEN, "%s/sbin/pkg", getlocalbase()); - while ((ch = getopt_long(argc, argv, "-:dfr::yN46", longopts, NULL)) != -1) { + while ((ch = getopt_long(argc, argv, "+:dN46", longopts, NULL)) != -1) { switch (ch) { case 'd': debug++; break; - case 'f': - force = true; - break; case 'N': activation_test = true; break; - case 'y': - yes = true; - break; case '4': fetchOpts = "4"; break; case '6': fetchOpts = "6"; break; - case 'r': - /* - * The repository can only be specified for an explicit - * bootstrap request at this time, so that we don't - * confuse the user if they're trying to use a verb that - * has some other conflicting meaning but we need to - * bootstrap. - * - * For that reason, we specify that -r has an optional - * argument above and process the next index ourselves. - * This is mostly significant because getopt(3) will - * otherwise eat the next argument, which could be - * something we need to try and make sense of. - * - * At worst this gets us false positives that we ignore - * in other contexts, and we have to do a little fudging - * in order to support separating -r from the reponame - * with a space since it's not actually optional in - * the bootstrap/add sense. - */ - if (add_pkg || bootstrap_only) { - if (optarg != NULL) { - repo_name = optarg; - } else if (optind < argc) { - repo_name = argv[optind]; - } + default: + break; + } + } + if (debug > 1) + fetchDebug = 1; - if (repo_name == NULL || *repo_name == '\0') { - fprintf(stderr, - "Must specify a repository with -r!\n"); - exit(EXIT_FAILURE); - } + argc -= optind; + argv += optind; + + if (argc >= 1) { + if (strcmp(argv[0], "bootstrap") == 0) { + bootstrap_only = true; + } else if (strcmp(argv[0], "add") == 0) { + add_pkg = true; + } - if (optarg == NULL) { - /* Advance past repo name. */ - optreset = 1; - optind++; + optreset = 1; + optind = 1; + if (bootstrap_only || add_pkg) { + struct option sub_longopts[] = { + { "force", no_argument, NULL, 'f' }, + { "yes", no_argument, NULL, 'y' }, + { NULL, 0, NULL, 0 }, + }; + while ((ch = getopt_long(argc, argv, "+:fr:y", + sub_longopts, NULL)) != -1) { + switch (ch) { + case 'f': + force = true; + break; + case 'r': + repo_name = optarg; + break; + case 'y': + yes = true; + break; + case ':': + fprintf(stderr, "Option -%c requires an argument\n", optopt); + exit(EXIT_FAILURE); + break; + default: + break; } } - break; - case 1: - // Non-option arguments, first one is the command - if (command == NULL) { - command = argv[optind-1]; - if (strcmp(command, "add") == 0) { - add_pkg = true; - } - else if (strcmp(command, "bootstrap") == 0) { - bootstrap_only = true; + } else { + /* + * Parse -y and --yes regardless of the pkg subcommand + * specified. This is necessary to make, for example, + * `pkg install -y foobar` work as expected when pkg is + * not yet bootstrapped. + */ + struct option sub_longopts[] = { + { "yes", no_argument, NULL, 'y' }, + { NULL, 0, NULL, 0 }, + }; + while ((ch = getopt_long(argc, argv, "+y", + sub_longopts, NULL)) != -1) { + switch (ch) { + case 'y': + yes = true; + break; + default: + break; } } - // bootstrap doesn't accept other arguments - else if (bootstrap_only) { - fprintf(stderr, args_bootstrap_message); + + } + argc -= optind; + argv += optind; + + if (bootstrap_only && argc > 0) { + fprintf(stderr, args_bootstrap_message); + exit(EXIT_FAILURE); + } + + if (add_pkg) { + if (argc < 1) { + fprintf(stderr, "Path to pkg.pkg required\n"); exit(EXIT_FAILURE); - } - else if (add_pkg && pkgarg != NULL) { + } else if (argc == 1 && pkg_is_pkg_pkg(argv[0])) { + pkgarg = argv[0]; + } else { /* - * Additional arguments also means it's not a - * local bootstrap request. + * If the target package is not pkg.pkg + * or there is more than one target package, + * this is not a local bootstrap request. */ add_pkg = false; } - else if (add_pkg) { - /* - * If it's not a request for pkg or pkg-devel, - * then we must assume they were trying to - * install some other local package and we - * should try to bootstrap from the repo. - */ - if (!pkg_is_pkg_pkg(argv[optind-1])) { - add_pkg = false; - } else { - pkgarg = argv[optind-1]; - } - } - break; - default: - break; } } - if (debug > 1) - fetchDebug = 1; if ((bootstrap_only && force) || access(pkgpath, X_OK) == -1) { struct repository *repo; @@ -1218,10 +1215,7 @@ main(int argc, char *argv[]) config_init(repo_name); if (add_pkg) { - if (pkgarg == NULL) { - fprintf(stderr, "Path to pkg.pkg required\n"); - exit(EXIT_FAILURE); - } + assert(pkgarg != NULL); if (access(pkgarg, R_OK) == -1) { fprintf(stderr, "No such file: %s\n", pkgarg); exit(EXIT_FAILURE); @@ -1263,7 +1257,7 @@ main(int argc, char *argv[]) exit(EXIT_SUCCESS); } - execv(pkgpath, argv); + execv(pkgpath, original_argv); /* NOT REACHED */ return (EXIT_FAILURE);