mport: restart getopt before every subcommand parses its options

[laffer1]

Problem

mport -U info bind920 printed the usage text instead of the package, and mport -U version -t 1 1 printed nothing at all. Any global option before info, annotate, version or which misbehaves this way, including -q, -V and the new --allow-old-release.

Each subcommand parses its own options with getopt(3) over the argument vector left after the global options. getopt keeps its position in optind (and on BSD, internal state that only optreset clears), so it has to be restarted first. add, install, delete, audit, verify and query did that inline; annotate, info, version and which did not. After a global option, optind already pointed past the subcommand's arguments and the parse came up empty, so the handler saw no arguments. Without a global option optind happened to be 1 and those handlers worked by accident.

Change

A reset_getopt() helper (optreset = 1 under the __MidnightBSD__ guard, optind = 1) called at the top of every handler that runs getopt, replacing the six inline copies. verify had set optind without optreset and now uses the helper too.

The if (local_argc > 1) guards around the per-command parses are left as they are; removing them is not needed for the fix.

Tests

mport_cli_test gains:

• global_option_before_subcommand (no registry needed): mport -U version -t 1 1 and mport -q -U version -t 2.0 2.0 must print =. The installed 2.8.3 binary prints nothing for these today.
• global_option_before_info (registry required, skipped otherwise): mport -U info no-such-package-zzz must not come back with the usage text.

kyua test mport_cli_test
7/7 passed (0 failed)

🤖 Generated with Claude Code

Summary by Sourcery

Reset getopt state before each subcommand parses options so global and subcommand options work together reliably.

Bug Fixes:

• Ensure every subcommand restarts getopt before parsing its options, fixing global options being ignored by annotate, info, version, and which.

Enhancements:

• Centralize getopt state reset logic, including BSD-specific state handling, across all subcommand option parsers.

Tests:

• Add CLI coverage for global options preceding version and info subcommands, with the full test suite passing.

[sourcery-ai]

Reviewer's Guide

Fix subcommands that incorrectly display usage or miss arguments after a global option by restarting getopt consistently before every command-level parse, and add CLI regressions covering version and info.

Sequence diagram for resetting getopt before subcommand parsing

sequenceDiagram
    participant User
    participant main
    participant reset_getopt
    participant getopt
    participant Handler

    User->>main: mport -U info package
    main->>getopt: getopt(global options)
    main->>Handler: dispatch subcommand
    Handler->>reset_getopt: reset_getopt()
    reset_getopt->>reset_getopt: optreset = 1 on MidnightBSD
    reset_getopt->>reset_getopt: optind = 1
    Handler->>getopt: getopt(command options)
    getopt-->>Handler: package arguments
    Handler-->>User: command result instead of usage

Loading

Flow diagram for global and subcommand option parsing

flowchart LR
    A[Global options parsed] --> B[Subcommand selected]
    B --> C["reset_getopt()"]
    C --> D["getopt(command options)"]
    D --> E[Handler receives package arguments]
    E --> F[Print command result]

Loading

File-Level Changes

Change	Details	Files
Centralize and apply getopt state reset before each subcommand-specific option parse.	Add a helper that sets optind and, on MidnightBSD, optreset. Replace existing inline resets and add resets to annotate, info, version, and which. Ensure verify also clears BSD getopt state consistently.	mport/mport.c
Add regression coverage for global options preceding subcommands.	Test version with global options and command options without requiring a registry. Test info after a global option and avoid treating usage output as successful handling.	tests/mport_cli_test

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[github-actions]

Claude finished @laffer1's task in 16s —— View job

---

Review of PR #207

No blocking issues found.

• The fix is correct. After the global loop, main does argc -= optind; argv += optind (mport/mport.c:331). argv[0] is then the subcommand name, and getopt has to start at 1. reset_getopt() now does that before every getopt call in main, plus the one in query(). I grepped for getopt(: all 11 call sites are preceded by reset_getopt(), and no other file in mport/ touches optind or optreset.
• verify previously set optind = 1 without optreset, so it was also broken on MidnightBSD. The helper fixes that.
• The helper is declared static and defined after main, with a forward declaration, so -Werror won't flag it.
• Minor, non-blocking: reset_getopt() resets only optind and optreset. optarg is unaffected, so this is fine. On non-MidnightBSD platforms (glibc), optind = 1 alone restarts correctly.
• Test note: the -e not-match:"usage: mport" check in global_option_before_subcommand passes if stderr is empty, so it relies on -o inline:"=\n" to catch a regression. That is sufficient. The tests can't catch a case where version prints = but takes the wrong code path.

I did not build or run the tests in this session. This review is from reading the diff and tracing the call sites.

[github-actions]

Claude finished @laffer1's task in 23s —— View job

---

Review of PR #207: restart getopt before every subcommand parse

Verdict: no blocking issues found.

• Read the diff against origin/main
• Traced the optind and argv handling in mport/mport.c
• Not built or run: I had no BSD toolchain here, so the Kyua results in the PR description are unverified by me.

Findings

The fix is correct. After the global getopt loop, main does argc -= optind; argv += optind (mport/mport.c:331). So argv[0] is the subcommand name, and a fresh getopt over local_argv must start at optind = 1. reset_getopt() does that, and sets optreset on MidnightBSD. I checked each handler:

• annotate, info, version, which: these now reset. Previously they relied on optind being 1 by accident.
• verify: it set optind without optreset. It now uses the helper, which fixes a latent glibc-style/BSD mismatch.
• download and audit: the code that reads optind after the loop (argc - optind, argv + optind) still runs after the reset and parse, so it sees the subcommand's optind. This is correct, and the same holds for the local_argc -= optind sites.
• query() (mport/mport.c:1130): it still reads optind after its own parse, so the reset there is fine.
• Placement: reset_getopt() is declared before main and defined after it. That is fine for a static function.
• Non-MidnightBSD builds: they skip optreset, as before. That matches the previous behavior.

Minor notes (non-blocking)

1. Test coverage: global_option_before_subcommand covers only version. info, annotate and which rely on the registry-gated info test, which skips without /var/db/mport/master.db. In CI, info, annotate and which may not be exercised at all.
2. global_option_before_info is weak: it uses -s ignore and checks only that the usage text is absent. An info that silently printed nothing and exited 0 would still pass. If the registry is present, consider also asserting that the exit status is non-zero, or that the output contains the package name or a "not found" message.
3. which and annotate: no test covers a global option before them. A -q -U which -o-style check that needs no registry would be cheap, if such a path exists.
4. local_argc > 1 guards: I agree with the PR that they can stay for now. A single-argument form such as mport -U info falls through to each handler's own argument check, which is unchanged.

Per AGENTS.md, remember to run the cppcheck/clang-format precommit script and the splint script before merging. I didn't run them.

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook