Skip to content

[std] add minimal CLI parser std.cli#24881

Closed
thejoshwolfe wants to merge 13 commits into
masterfrom
cli
Closed

[std] add minimal CLI parser std.cli#24881
thejoshwolfe wants to merge 13 commits into
masterfrom
cli

Conversation

@thejoshwolfe

Copy link
Copy Markdown
Contributor

Closes #24601

Comment thread lib/std/cli.zig Outdated
Comment thread lib/std/cli.zig Outdated
@andrewrk

andrewrk commented Aug 17, 2025

Copy link
Copy Markdown
Member

I recommend giving it a trial run by porting all the files in tools/ to use this.

You can test at least that those compile successfully with zig build test-standalone.

@thejoshwolfe

thejoshwolfe commented Aug 17, 2025

Copy link
Copy Markdown
Contributor Author

Currently, just trying the parse() call results in an error return trace:

josh@nixos:~/dev/zig$ zig run --zig-lib-dir lib/ tools/docgen.zig -- --help
Usage: docgen [options] input output

   Generates an HTML document from a docgen template.

Options:
   --code-dir dir         Path to directory containing code example outputs
   --help                 Print this help and exit
error: Help
/home/josh/dev/zig/lib/std/cli.zig:275:13: 0x118c7af in innerParse__anon_25090 (std.zig)
            return error.Help;
            ^
/home/josh/dev/zig/lib/std/cli.zig:159:5: 0x118399b in parseIter__anon_24026 (std.zig)
    return innerParse(Args, arena, iter, prog, options.writer);
    ^
/home/josh/dev/zig/lib/std/cli.zig:115:5: 0x1169426 in parse__anon_22563 (std.zig)
    return parseIter(Args, arena, &iter, options);
    ^
/home/josh/dev/zig/tools/docgen.zig:41:18: 0x1166557 in main (docgen.zig)
    const args = try std.cli.parse(Args, arena, .{});
                 ^

This appears to be done unconditionally for all error codes:

zig/lib/std/start.zig

Lines 638 to 640 in 623290e

if (@errorReturnTrace()) |trace| {
std.debug.dumpStackTrace(trace.*);
}

Is there any appetite for special casing the cli errors error{Help,Usage} to bypass the tracing? This would be similar to how python's sys.exit() i.e. raise SystemExit() bypasses the typical traceback.

If we don't want to modify callMain() for this cli feature, then the design of this PR needs to be adjusted to fully call std.process.exit() by default, and have an option for returning errors. i'll proceed in this direction unless someone stops me.

@andrewrk

Copy link
Copy Markdown
Member

Is there any appetite for special casing the cli errors error{Help,Usage} to bypass the tracing?

sounds like #24510

@rohlem

rohlem commented Aug 17, 2025

Copy link
Copy Markdown
Contributor

Is there any appetite for special casing the cli errors error{Help,Usage} to bypass the tracing?

Just my 2c, but if I propagated error.Help or error.Usage anywhere else in my program, I'd be very confused if it caused a program exit without an error trace.
(iiuc, since the code that triggers the help/usage printing is within std.cli, this would just be silent, unexplained program termination.)
If this is chosen as strategy, I'd recommend at least making the names more specific, f.e. error.StdCliHelp and error.StdCliUsage, since users aren't expected to intercept them anyway.

adjusted to fully call std.process.exit() by default, and have an option for returning errors.

I think that would be the simplest, most local solution. Options can gain fields exit_on_help: bool = true, exit_on_usage: bool = true (or potentially a combined one).

@thejoshwolfe

thejoshwolfe commented Aug 17, 2025

Copy link
Copy Markdown
Contributor Author

exit_on_help: bool = true, exit_on_usage: bool = true

I went with exit_on_error, and i'm calling --help an error, but it turns out that's not obviously the right design. i've started a discussion here: #24601 (comment)

EDIT: options.exit controls both the error and non-error exits.

@thejoshwolfe

Copy link
Copy Markdown
Contributor Author

i'm calling --help an error,

I'm no longer calling --help an error.

Comment thread lib/std/cli.zig Outdated
/// First positional (non-named) argument:
input: [:0]const u8 = "",
/// Second positional argument is declared as optional:
reptitions: u32 = 1,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential typo of "repetitions"? (completely inconsequential)

@nobkd nobkd Aug 30, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(5a11f4c) 0a4ffc2 didn't really fix the typo. Now it's "repititions" 😅

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 😂 fixed again (in the next commit). thanks yall.

@thejoshwolfe thejoshwolfe force-pushed the cli branch 2 times, most recently from 60d5a4a to 6be202e Compare August 30, 2025 10:08

@thejoshwolfe thejoshwolfe left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per @andrewrk 's suggestion, i ported almost all the tools/ to this new cli parsing interface to try it out, and my general thoughts are:

  • it's easy to support --help by calling std.cli.parse() even with an empty Args struct, which means users get a more consistent/predictable interface. previously some tools were ignoring --help or interpreting it as a file path or something (and then hopefully crashing).
  • the generated --help output tends to be more verbose, such as listing all the arguments twice and stating that they're required. not always an improvement.
  • the use case of a fixed number of positional args that have specific names is a very good fit in this tools/ directory.

one tool was not touched yet: tools/doctest.zig. This is due to that tool using single-letter -i and -o named arguments, which means this PR would introduce an API break to that tool, and require updating all callers including build.zig at least. i will attempt this in a follow up commit, but didn't want to test changing too many things at once.

Comment thread tools/docgen.zig

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/docgen.zig -- --help
Usage: docgen [options] input output

   Generates an HTML document from a docgen template.

Options:
   --code-dir dir         Path to directory containing code example outputs
   -h, --help             Print this help and exit

after:

$ zig run --zig-lib-dir lib/ tools/docgen.zig -- --help
usage: docgen --code-dir=string input output

Generates an HTML document from a docgen template.

positional arguments:
  input                string. required
  output               string. required

named arguments:
  --code-dir=string    required. Path to directory containing code example outputs
  --help               print this help and exit
  • fixed --code-dir incorrectly appearing optional.
  • lost the dir metavar for --code-dir, now called string.

Comment thread tools/dump-cov.zig

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/dump-cov.zig -- --help
thread 296647 panic: index out of bounds: index 2, len 2
/home/josh/dev/zig.master/tools/dump-cov.zig:21:31: 0x1148d3d in main (dump-cov.zig)
    const cov_file_name = args[2];
                              ^
/home/josh/dev/zig.master/lib/std/start.zig:627:37: 0x114bbf9 in posixCallMainAndExit (std.zig)
            const result = root.main() catch |err| {
                                    ^
/home/josh/dev/zig.master/lib/std/start.zig:232:5: 0x1146be1 in _start (std.zig)
    asm volatile (switch (native_arch) {
    ^
???:?:?: 0x0 in ??? (???)
Aborted (core dumped)

after:

$ zig run --zig-lib-dir lib/ tools/dump-cov.zig -- --help
usage: dump-cov exe_file cov_file

positional arguments:
  exe_file             string. required
  cov_file             string. required

named arguments:
  --help               print this help and exit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/fetch_them_macos_headers.zig -- --help
info: fetch_them_macos_headers [options] [cc args]

Options:
  --sysroot     Path to macOS SDK

General Options:
-h, --help                    Print this help and exit

after:

$ zig run --zig-lib-dir lib/ tools/fetch_them_macos_headers.zig -- --help
usage: fetch_them_macos_headers [options] [cc_args...]

positional arguments:
  cc_args              string. can be specified multiple times

named arguments:
  --sysroot=string     default: ''. Path to macOS SDK
  --help               print this help and exit
  • no longer support -h
  • slightly more clear that cc args now called cc_args can be specified multiple times.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/gen_macos_headers_c.zig -- --help
info: gen_macos_headers_c [dir]

General Options:
-h, --help                    Print this help and exit

after:

$ zig run --zig-lib-dir lib/ tools/gen_macos_headers_c.zig -- --help
usage: gen_macos_headers_c dir

positional arguments:
  dir                  string. required

named arguments:
  --help               print this help and exit
  • fixed [dir] appearing optional
  • more verbose for little gain

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

josh@nixos:~/dev/zig.master$ r gen_outline_atomics.zig
$ zig run --zig-lib-dir lib/ tools/gen_outline_atomics.zig -- --help
//! This file is generated by tools/gen_outline_atomics.zig.
const builtin = @import("builtin");
const std = @import("std");
const common = @import("common.zig");
const always_has_lse = builtin.cpu.has(.aarch64, .lse);
[... ~2000 lines of output]

after:

$ zig run --zig-lib-dir lib/ tools/gen_outline_atomics.zig -- --help
usage: gen_outline_atomics 

named arguments:
  --help          print this help and exit

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

Usage: /home/josh/.cache/zig/o/80422d926c8fa91c39a5e0ca7c41471c/update-linux-headers [--search-path <dir>] --out <dir> --abi <name>
--search-path can be used any number of times.
    subdirectories of search paths look like, e.g. x86_64-linux-gnu
--out is a dir that will be created, and populated with the results

after:

$ zig run --zig-lib-dir lib/ tools/update-linux-headers.zig -- --help
usage: update-linux-headers [options] --out=string

named arguments:
  --search-path=string  can be specified multiple times. subdirectories of search paths look like, e.g. x86_64-linux-gnu
  --out=string          required. a dir that will be created, and populated with the results
  --help                print this help and exit
  • fixed incorrect --abi parameter, probably a copy-paste error from process_headers.zig.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/update_clang_options.zig -- --help
Usage: /home/josh/.cache/zig/o/2988e875c2d7781b249c2e4a40efde00/update_clang_options /path/to/llvm-tblgen /path/to/git/llvm/llvm-project
Alternative Usage: zig run /path/to/git/zig/tools/update_clang_options.zig -- /path/to/llvm-tblgen /path/to/git/llvm/llvm-project

Prints to stdout Zig code which you can use to replace the file src/clang_options_data.zig.

after:

$ zig run --zig-lib-dir lib/ tools/update_clang_options.zig -- --help
usage: update_clang_options /path/to/llvm-tblgen /path/to/git/llvm/llvm-project

Prints to stdout Zig code which you can use to replace the file src/clang_options_data.zig.

positional arguments:
  /path/to/llvm-tblgen            string. required
  /path/to/git/llvm/llvm-project  string. required

named arguments:
  --help                          print this help and exit
  • more verbose
  • removed zig run tutorial.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/update_cpu_features.zig -- --help
Usage: /home/josh/.cache/zig/o/8ecc2bee7f3cbf32ac512f1adaa7aa7e/update_cpu_features /path/to/llvm-tblgen /path/git/llvm-project /path/git/zig [zig_name filter]

Updates lib/std/target/<target>.zig from llvm/lib/Target/<Target>/<Target>.td .

On a less beefy system, or when debugging, compile with -fsingle-threaded.

after:

$ zig run --zig-lib-dir lib/ tools/update_cpu_features.zig -- --help
usage: update_cpu_features /path/to/llvm-tblgen /path/git/llvm-project /path/git/zig [zig_name_filter]

Updates lib/std/target/<target>.zig from llvm/lib/Target/<Target>/<Target>.td .

On a less beefy system, or when debugging, compile with -fsingle-threaded.

positional arguments:
  /path/to/llvm-tblgen    string. required
  /path/git/llvm-project  string. required
  /path/git/zig           string. required
  zig_name_filter         string. default: ''

named arguments:
  --help                  print this help and exit
  • more verbose

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/update_crc_catalog.zig -- --help
Usage: /home/josh/.cache/zig/o/29fd83f330fee72e83a7806ac7d8e0c2/update_crc_catalog /path/git/zig

after:

$ zig run --zig-lib-dir lib/ tools/update_crc_catalog.zig -- --help
usage: update_crc_catalog /path/git/zig

positional arguments:
  /path/git/zig        string. required

named arguments:
  --help               print this help and exit
  • less verbose arg0
  • more verbose everything else

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before:

$ zig run --zig-lib-dir lib/ tools/update_freebsd_libc.zig -- --help
thread 298062 panic: index out of bounds: index 2, len 2
/home/josh/dev/zig.master/tools/update_freebsd_libc.zig:21:30: 0x1135994 in main (update_freebsd_libc.zig)
    const zig_src_path = args[2];
                             ^
/home/josh/dev/zig.master/lib/std/start.zig:627:37: 0x11377c9 in posixCallMainAndExit (std.zig)
            const result = root.main() catch |err| {
                                    ^
/home/josh/dev/zig.master/lib/std/start.zig:232:5: 0x1131be1 in _start (std.zig)
    asm volatile (switch (native_arch) {
    ^
???:?:?: 0x0 in ??? (???)
Aborted (core dumped)

after:

$ zig run --zig-lib-dir lib/ tools/update_freebsd_libc.zig -- --help
usage: update_freebsd_libc freebsd_src_path zig_src_path

positional arguments:
  freebsd_src_path     string. required
  zig_src_path         string. required

named arguments:
  --help               print this help and exit

approximately the same for the remaining three tools:

update_glibc.zig
update_mingw.zig
update_netbsd_libc.zig

;
const Args = struct {
named: struct {
sysroot: []const u8 = "",

@rohlem rohlem Aug 31, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance it might be nicer to instead type it ?[]const u8 and default to null.
Then the code wouldn't have to check for length 0, and could use orelse instead.

iiuc, passing --sysroot="" leads to auto-detection, with that change I assume it would lead to an error and have to be omitted.
Is there a code or interface reason that I'm missing that makes "" more intuitive for auto-detection?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason is that i ported code that was expecting null after argument parsing, and so i preserved those semantics. optional arguments declared as ?[]const u8 = null makes a lot of intuitive sense, but i explained why i believe it is a misfeature in #24601.

it could be that the pattern here will be frequent where users want to conflate "" with null, like i'm doing, but a layer of code to translate from a deserialization system into the type you want to use in your code base is a common thing that doesn't mean the deserialization system is missing a feature.

if you disagree that lack of support for null is a misfeature, let's discuss it #24601. the real actual use cases you're pointing to in this review are evidence against my stance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iiuc, passing --sysroot="" leads to auto-detection

wait, are you saying that "" is different that not being specified? if so, then i've broken something. i'll take a look.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, i can't tell whether cc -isysroot '' is going to do something meaningful, so i don't know if i broke it or not. on my system, i get this:

error: no SDK found; you can provide one explicitly with '--sysroot' flag

based on the name and lack of comments (git log didn't help either), i guess this is supposed to run on MacOS perhaps? pinging @alexrp to shed insight. is --sysroot="" supposed to be different from omitting the --sysroot argument entirely?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iiuc, passing --sysroot="" leads to auto-detection

wait, are you saying that "" is different that not being specified?

No, my bad, should have been more specific - I was just talking about the behavior of the new code.
I'm not familiar with the old behavior either, but think it's unlikely that "" had any special meaning/behavior previously.

Comment thread tools/incr-check.zig
Comment on lines +18 to +23
@"zig-lib-dir": []const u8 = "",
@"debug-zcu": bool = false,
@"debug-dwarf": bool = false,
@"debug-link": bool = false,
preserve_tmp: bool = false,
@"zig-cc-binary": []const u8 = "",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, @"zig-lib-dir" and @"zig-cc-binary" could be ?[]const u8 and start out as null, then the code could use orelse instead of checking for length 0.

@"/path/to/llvm-tblgen": [:0]const u8,
@"/path/git/llvm-project": [:0]const u8,
@"/path/git/zig": [:0]const u8,
zig_name_filter: []const u8 = "",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, could be : ?[]const u8 = null

@sebastianoff

Copy link
Copy Markdown

I may be missing something, but why are we dealing with the questionable error.Help or error.Usage? Why can't we just use std.process.exit?

@rohlem

rohlem commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

why are we dealing with the questionable error.Help or error.Usage? Why can't we just use std.process.exit?

I think it's meant to be a more flexible API. Some programs might want to use std.cli for parsing, but not immediately std.process.exit in these scenarios.

@andrewrk

andrewrk commented Oct 9, 2025

Copy link
Copy Markdown
Member

This branch is not passing standard library test suite and hasn't been touched in over 30 days. Do you need help figuring out why the checks are failing?

@thejoshwolfe

Copy link
Copy Markdown
Contributor Author

this PR is in design discussion hell in #24601 . I have a plan to proceed despite the deadlock, but i haven't prioritized it recently. if the open PR is causing friction in the project management, I can open a new (rebased) PR at a later time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

std: minimal CLI parsing driven by struct fields

6 participants