upstream/opnsense-src
1
0
Fork 0
mirror of https://github.com/opnsense/src.git synced 2026-06-11 17:51:26 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

pkg: clarify argument parsing

Browse source 
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
This commit is contained in:
Isaac Freund 2025-04-23 10:27:35 +00:00 committed by Ed Maste
parent d5e3cf41e8
commit be61deae0a
2 changed files with 94 additions and 99 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
usr.sbin/pkg/pkg.7
View file

@ -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

188
usr.sbin/pkg/pkg.c
View file

@ -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,131 +1068,46 @@ 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];
}
if (repo_name == NULL || *repo_name == '\0') {
fprintf(stderr,
"Must specify a repository with -r!\n");
exit(EXIT_FAILURE);
}
if (optarg == NULL) {
/* Advance past repo name. */
optreset = 1;
optind++;
}
}
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;
}
}
// bootstrap doesn't accept other arguments
else if (bootstrap_only) {
fprintf(stderr, args_bootstrap_message);
exit(EXIT_FAILURE);
}
else if (add_pkg && pkgarg != NULL) {
/*
* Additional arguments also means it's 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;
}
@ -1204,6 +1115,92 @@ main(int argc, char *argv[])
if (debug > 1)
fetchDebug = 1;
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;
}
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;
}
}
} 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;
}
}
}
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 (argc == 1 && pkg_is_pkg_pkg(argv[0])) {
pkgarg = argv[0];
} else {
/*
* 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;
}
}
}
if ((bootstrap_only && force) || access(pkgpath, X_OK) == -1) {
struct repository *repo;
int ret = 0;
@ -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);