From 7a002daf4e451e8c03d5fcd9ab1ea07255e40481 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 27 Sep 2026 12:18:13 +0200 Subject: [PATCH 1/6] Give rbgo a ruby command line: ARGV, $0 and -w/-W (#708, #709) The front end rejected every command line that carried an argument for the script (`rbgo t.rb a b`) and every warning switch (`-w`, `-W0`, `-W1`, `-W2`), printing its usage text and exiting 2. ARGV existed and was always empty. The rule implemented is ruby.c's, in two parts: - proc_options (ruby.c:1558, ruby_4_0) scans argv[1..] and STOPS at the first entry that is not "-X..." -- `if (!arg || arg[0] != '-' || !arg[1]) break;` -- so a bare word and "-" alone both end option processing and stay in place. "--" also ends it and is consumed. - process_options (ruby.c:2361) then takes ONE more entry as the script, but only `if (!opt->e_script)`, and hands the rest to ruby_set_argv. -w/-W come from proc_W_option (v3_4_0 ruby.c:1250-1268): 0 -> $VERBOSE nil, 1 -> false, >=2 -> true, with `-W` alone meaning -W2 and scan_oct reading one octal digit. Measured against the oracle (ruby 4.0.5), not reasoned about: - `ruby s.rb -- -w` puts BOTH "--" and "-w" in ARGV: options stopped at the script, so that "--" is an ordinary argument. - `ruby -e 'p ARGV' -w` prints [] -- -e does not end option processing. - the last -w/-W on one command line wins, in either order. An unknown switch now gets MRI's own uncaught-RuntimeError line and exit 1 rather than a usage summary. A switch ruby has and rbgo does not is reported as unimplemented instead, because calling -n invalid would be a false statement about Ruby. A closed-world binary (`rbgo build --closed`) now passes its OWN arguments to ARGV and sets $0 to the binary, with no option parsing: it is the program, so every argument belongs to it. It reads os.Args on native and on wasm alike, so no build tag decides whether a standalone binary can take arguments. internal/vm gains the two host entry points this needs, in a new file: SetARGV (ruby_set_argv, refilling the ONE Array that ARGV and $* both name, with frozen elements) and SetWarningLevel (proc_W_option's three states). The comment at vm.go:1046 already cited SetARGV; it did not exist until now. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/closed_argv.go | 39 +++ cmd/rbgo/closed_main.go | 3 + cmd/rbgo/closed_main_wasm.go | 3 + cmd/rbgo/main.go | 126 +++++++--- cmd/rbgo/options.go | 293 ++++++++++++++++++++++ cmd/rbgo/options_test.go | 411 +++++++++++++++++++++++++++++++ internal/vm/host_cmdline.go | 66 +++++ internal/vm/host_cmdline_test.go | 161 ++++++++++++ 8 files changed, 1066 insertions(+), 36 deletions(-) create mode 100644 cmd/rbgo/closed_argv.go create mode 100644 cmd/rbgo/options.go create mode 100644 cmd/rbgo/options_test.go create mode 100644 internal/vm/host_cmdline.go create mode 100644 internal/vm/host_cmdline_test.go diff --git a/cmd/rbgo/closed_argv.go b/cmd/rbgo/closed_argv.go new file mode 100644 index 00000000..f96c367e --- /dev/null +++ b/cmd/rbgo/closed_argv.go @@ -0,0 +1,39 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build rbgo_closed + +package main + +import ( + "os" + + "github.com/go-embedded-ruby/ruby/internal/vm" +) + +// seedProcessArgs gives the embedded program the arguments this process was +// started with. A closed-world binary IS the program — there is no interpreter +// command line in front of it — so there is nothing to parse and nothing to +// split: every argument belongs to the program, exactly as it would for a C +// program, and $0 is the binary that was invoked. +// +// This is deliberately NOT ruby.c's proc_options. `./game -W0 level.dat` passes +// "-W0" to the game; a closed binary that swallowed it as an interpreter switch +// would be lying about whose command line it is. +// +// It is shared by the native and the wasm closed mains, and it reads os.Args on +// both, so no build tag decides whether a closed binary can take arguments. What +// differs is what the host puts there: wasip1 fills os.Args from the WASI +// args_get call, so `wasmtime prog.wasm a b` arrives intact, while a js/wasm +// module loaded by a browser has no process arguments at all and os.Args carries +// only the program name — ARGV is then empty, which is the honest answer rather +// than a tag-selected one. +func seedProcessArgs(machine *vm.VM) { + if len(os.Args) > 0 { + machine.SetScriptName(os.Args[0]) + machine.SetARGV(os.Args[1:]) + return + } + machine.SetARGV(nil) +} diff --git a/cmd/rbgo/closed_main.go b/cmd/rbgo/closed_main.go index 66d9c182..d3683e5b 100644 --- a/cmd/rbgo/closed_main.go +++ b/cmd/rbgo/closed_main.go @@ -19,6 +19,9 @@ import ( // runtime stays alive for browser callbacks. func main() { machine := vm.New(os.Stdout) + // The program's own arguments: a standalone binary that cannot take arguments + // is not a usable program (#708). See seedProcessArgs in closed_argv.go. + seedProcessArgs(machine) _, err := machine.Run(embeddedProgram()) finish(machine, err) // never returns; see exit_status.go } diff --git a/cmd/rbgo/closed_main_wasm.go b/cmd/rbgo/closed_main_wasm.go index e10adb04..b03ff439 100644 --- a/cmd/rbgo/closed_main_wasm.go +++ b/cmd/rbgo/closed_main_wasm.go @@ -24,6 +24,9 @@ import ( // the registered js.Func callbacks remain live (this mirrors cmd/wasm/main.go). func main() { machine := vm.New(os.Stdout) + // The program's own arguments, from whatever the wasm host supplied (#708). + // See seedProcessArgs in closed_argv.go. + seedProcessArgs(machine) if _, err := machine.Run(embeddedProgram()); err != nil { finish(machine, err) // never returns; see exit_status.go } diff --git a/cmd/rbgo/main.go b/cmd/rbgo/main.go index 250ba287..c3bcc02d 100644 --- a/cmd/rbgo/main.go +++ b/cmd/rbgo/main.go @@ -6,12 +6,17 @@ // generated main in closed_main.go, which runs a single embedded program with no // front-end linked — so this CLI is excluded from that build. // -// rbgo run compile in memory and interpret -// rbgo run -e "" run a one-liner -// rbgo shorthand for `rbgo run` -// rbgo build [-o out] AOT-compile the program's lowerable methods to -// native Go and link a specialised binary (see -// internal/aot and docs/aot-compiler.md) +// rbgo [options] [--] [arguments...] interpret, ARGV = the arguments +// rbgo [options] -e "" [arguments...] run a one-liner +// rbgo run ... the same, spelled explicitly +// rbgo build [-o out] AOT-compile the program's +// lowerable methods to native Go +// and link a specialised binary +// (see internal/aot and +// docs/aot-compiler.md) +// +// The options and the script/ARGV boundary follow ruby.c; options.go holds the +// rule and the citations. // // `repl` arrives in a later phase (plan-rbgo.md §17). package main @@ -28,50 +33,67 @@ import ( func main() { args := os.Args[1:] if len(args) == 0 { - usage() + // No arguments at all is a usage error. MRI would read the program from + // stdin here; rbgo keeps the usage message because it is a multi-command + // front-end (run/build) and a bare `rbgo` silently waiting on a terminal + // would be worse. `rbgo -` still reads stdin, as does `rbgo` with options + // but no script — see options.load. + usageError() } - cmd := args[0] - switch cmd { + switch args[0] { case "run": + if len(args) == 1 { + usageError() + } runCmd(args[1:]) case "build": buildCmd(args[1:]) - case "-h", "--help", "help": - usage() + case "help": + help() default: - // Shorthand: `rbgo file.rb`. + // Everything else is `ruby`'s own command line, including -h/--help, which + // parseOptions recognises. A script whose name happens to be "run", + // "build" or "help" needs a path ("./run"), which was already true. runCmd(args) } } func runCmd(args []string) { - var src, name string - switch { - case len(args) == 2 && args[0] == "-e": - src, name = args[1], "-e" - case len(args) == 1: - b, err := os.ReadFile(args[0]) - if err != nil { - fatal("rbgo: %v", err) - } - src, name = string(b), args[0] - default: - usage() + o, err := parseOptions(args, os.Stderr) + if err != nil { + // MRI's own wording and exit status, not a usage summary (#708). + fatal("%s", err) + } + if o.help { + help() + } + src, name, fromFile, err := o.load() + if err != nil { + fatal("rbgo: %v", err) } // finish never returns: it is MRI's ruby_run_node -> rb_ec_cleanup, which // turns the terminal exception into this process's exit status (or death by // signal) rather than always exiting 0 or 1. See exit_status.go. - finish(run(src, name)) + finish(runProgram(src, name, fromFile, o)) +} + +// run compiles and interprets one program with an empty ARGV and the default +// $VERBOSE. It is the shape the tests and the embedded callers want; the CLI goes +// through runProgram, which also seeds what the command line asked for. +func run(src, name string) (*vm.VM, error) { + return runProgram(src, name, name != "-e", nil) } -// run compiles and interprets one program, returning the machine it ran on +// runProgram compiles and interprets one program, returning the machine it ran on // alongside the error, because the terminal exception has to be classified // against that machine's class hierarchy (vm.ExitingSplit). The machine is // non-nil whenever the program actually ran; a parse or compile failure returns // nil, which finish handles. -func run(src, name string) (*vm.VM, error) { +// +// o may be nil, which means an empty ARGV and an untouched $VERBOSE. +func runProgram(src, name string, fromFile bool, o *options) (*vm.VM, error) { prog, err := parser.Parse(src) if err != nil { return nil, err @@ -85,21 +107,53 @@ func run(src, name string) (*vm.VM, error) { // $stderr/STDERR writes, Kernel#abort's message — goes to stderr, so a // redirected or piped stdout carries only the program's data (#667). machine := vm.NewWithStderr(os.Stdout, os.Stderr) - if name == "-e" { - // A -e one-liner has no file on disk; record "-e" as the program name so - // backtraces label its frames "-e" the way MRI does (require_relative still - // resolves against the CWD, since there is no script directory). - machine.SetScriptName(name) - } else { + if fromFile { machine.SetScriptPath(name) // so require/require_relative resolve relative to the script + } else { + // A -e one-liner and a stdin program have no file on disk; record the name + // ("-e" / "-") so backtraces label their frames the way MRI does + // (require_relative still resolves against the CWD, since there is no + // script directory). + machine.SetScriptName(name) + } + if o != nil { + // ruby_set_argv, after the interpreter exists and before the program runs. + machine.SetARGV(o.argv) + if o.warnSet { + // proc_W_option only assigns ruby_verbose when a -w/-W was given, so an + // untouched $VERBOSE stays unset here too rather than being written with + // the value it already reads as (globals.go verboseSlot). + machine.SetWarningLevel(o.warnLevel) + } } _, err = machine.Run(iseq) return machine, err } -func usage() { - fmt.Fprintln(os.Stderr, "usage: rbgo run | rbgo run -e \"\" | rbgo ") - fmt.Fprintln(os.Stderr, " rbgo build [-o out] AOT-compile methods and link a native binary") +// usageText is rbgo's own summary. MRI's -h describes switches rbgo does not +// have, so this is deliberately rbgo's interface and not a transcription. +const usageText = `usage: rbgo [options] [--] [arguments...] interpret a script; ARGV = [arguments...] + rbgo [options] -e "" [arguments...] run a one-liner + rbgo - read the program from stdin + rbgo run ... the same, spelled explicitly + rbgo build [-o out] [--closed] AOT-compile methods and link a native binary + +options: -w, -W, -W0 | -W1 | -W2, -W: $VERBOSE: -W0 silent, -W1 default, -w/-W2 verbose + -e may be repeated; then ARGV takes no script + -h, --help this message` + +// help prints the usage summary as a successful answer to a question — stdout, +// exit 0, which is what `ruby -h` does and what a shell pipeline expects. It +// never returns. +func help() { + fmt.Fprintln(os.Stdout, usageText) + os.Exit(0) +} + +// usageError prints the same summary as a complaint: stderr, exit 2. It never +// returns. +func usageError() { + fmt.Fprintln(os.Stderr, usageText) os.Exit(2) } diff --git a/cmd/rbgo/options.go b/cmd/rbgo/options.go new file mode 100644 index 00000000..3f9485cf --- /dev/null +++ b/cmd/rbgo/options.go @@ -0,0 +1,293 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !rbgo_closed + +package main + +import ( + "fmt" + "io" + "os" + "strings" +) + +// This file is the front-end's command line, ported from ruby.c. +// +// MRI's contract is `ruby [options] [--] [filepath] [arguments]`, and the rule +// that splits it lives in exactly two places: +// +// - proc_options (ruby.c:1558 in ruby_4_0) scans argv[1..] and STOPS at the +// first entry that is not of the form "-X..." — `if (!arg || arg[0] != '-' +// || !arg[1]) break;`. So a bare word, and "-" by itself, both end option +// processing and stay in place. "--" is the one option that also stops the +// scan, and it is consumed (case '-': `if (!s[1]) { argc--, argv++; goto +// switch_end; }`). +// +// - process_options (ruby.c:2361) then takes ONE more entry as the script — +// `if (!opt->e_script) { ... opt->script = argv[0]; argc--; argv++; }` — and +// hands everything that is left to ruby_set_argv, which fills ARGV. +// +// Three consequences are easy to get wrong and were each measured against the +// oracle (ruby 4.0.5) rather than reasoned about: +// +// - `ruby s.rb -- -w` puts BOTH "--" and "-w" in ARGV. Options stopped at +// "s.rb"; the "--" after it is an ordinary argument, not a separator. +// - `ruby -e 'p ARGV' -w` prints []. -e does not end option processing — the +// scan resumes at the next entry — so "-w" is still a flag. +// - with -e there is NO script entry, so everything left after the scan is ARGV. +type options struct { + eScript string // the -e chunks, each terminated by "\n" as proc_e_option does + haveE bool // -e was given: no script entry is taken from argv + script string // the script path; "" (nothing left) and "-" both mean stdin + argv []string // what ruby_set_argv would receive + warnLevel int // -W: 0 silent, 1 default, >=2 verbose + warnSet bool // a -w or -W actually chose a level + help bool // -h / --help +} + +// optError is an option-parsing failure whose Error() is the COMPLETE stderr +// line, because MRI's wording is part of the contract: an unknown switch is an +// uncaught RuntimeError, so the process prints +// "ruby: invalid option -Z (-h will show valid options) (RuntimeError)" (two +// spaces before the parenthesis) and exits 1 — not a usage summary, and not +// exit 2. Issue #708 was found because rbgo printed its usage text here. +type optError struct{ text string } + +func (e *optError) Error() string { return e.text } + +// rubyRaise renders what an uncaught RuntimeError from proc_options looks like on +// stderr, with this program's name in place of rb_progname. +func rubyRaise(format string, args ...any) *optError { + return &optError{text: "rbgo: " + fmt.Sprintf(format, args...) + " (RuntimeError)"} +} + +// mriOnlyOptions are switches ruby accepts and rbgo does not implement. They are +// kept apart from the unknown ones deliberately: telling a user that -n is an +// "invalid option" would be a false statement about Ruby, so rbgo says it is +// unimplemented instead and still fails rather than ignoring it. +var mriOnlyOptions = map[byte]string{ + 'a': "split each input line into $F", 'p': "loop and print $_", + 'n': "loop over input lines", 'd': "set $DEBUG", 'y': "parser debug output", + 'v': "print the version and set $VERBOSE", 'c': "syntax check only", + 's': "parse switches after the script name", 'l': "line-ending processing", + 'S': "search $PATH for the script", 'r': "require a library first", + 'i': "in-place edit", 'x': "skip to #!ruby", 'C': "chdir first", + 'X': "chdir first", 'F': "set the input field separator", + 'E': "set the external/internal encoding", 'U': "internal encoding UTF-8", + 'K': "1.8 kcode", 'I': "prepend to $LOAD_PATH", '0': "set $/", +} + +// mriOnlyLongOptions is the same list for ruby's long switches +// (proc_long_options in ruby_4_0). +var mriOnlyLongOptions = map[string]bool{ + "copyright": true, "crash-report": true, "debug": true, "disable": true, + "dump": true, "enable": true, "encoding": true, "external-encoding": true, + "internal-encoding": true, "source-encoding": true, "jit": true, + "parser": true, "prism": true, "verbose": true, "version": true, + "yjit": true, "yydebug": true, "zjit": true, "backtrace-limit": true, +} + +// warnCategories are proc_W_option's four `-W:name` categories. rbgo accepts the +// spelling and does not yet route the per-category bits, which is why they are +// listed rather than acted on; an unknown name gets MRI's rb_warn, gated on +// $VERBOSE exactly as rb_warn is (so `-W0 -W:bogus` is silent, as measured). +var warnCategories = map[string]bool{ + "deprecated": true, "experimental": true, "performance": true, + "strict_unused_block": true, +} + +// parseOptions is proc_options followed by the script/ARGV split in +// process_options. args is os.Args[1:], i.e. MRI's argv from index 1, which is +// where proc_options' loop begins (`for (argc--, argv++; ...)`). +// +// stderr is where an accepted-but-meaningless `-W:name` warning goes; it is a +// parameter so a test can read it without touching the process's descriptors. +func parseOptions(args []string, stderr io.Writer) (*options, error) { + o := &options{warnLevel: defaultWarnLevel} + i := 0 +scan: + for ; i < len(args); i++ { + arg := args[i] + // proc_options: the scan ends at the first entry that is not "-X...". + // len(arg) < 2 covers both "" and "-" (MRI's `!arg[1]`), and "-" must stay + // in place because it is a script name meaning stdin. + if len(arg) < 2 || arg[0] != '-' { + break + } + s := arg[1:] + // MRI's `goto reswitch` chews one letter at a time off a cluster like + // "-wW0"; a `break` out of its switch abandons the rest of the word and + // moves to the next entry, which is `break reswitch` here. + reswitch: + for len(s) > 0 { + switch s[0] { + case 'w': + // case 'w': ruby_verbose = Qtrue, i.e. -W2's level. + o.setWarn(2) + s = s[1:] + case 'W': + rest, more, err := o.procW(s, stderr) + if err != nil { + return nil, err + } + if !more { + break reswitch + } + s = rest + case 'e': + // proc_e_option: the code is the rest of the word, or the next + // entry when the word ends at "-e". + code := s[1:] + if code == "" { + if i+1 >= len(args) { + return nil, rubyRaise("no code specified for -e") + } + i++ + code = args[i] + } + o.haveE = true + o.eScript += code + "\n" // proc_e_option appends a newline per chunk + break reswitch + case 'h': + // case 'h' jumps straight to switch_end: the rest of the command + // line is never looked at. + o.help = true + break scan + case '-': + if s == "-" { // the argument was exactly "--" + i++ // MRI consumes it (argc--, argv++) before switch_end + break scan + } + name := s[1:] + if eq := strings.IndexByte(name, '='); eq >= 0 { + name = name[:eq] + } + switch { + case name == "help": + o.help = true + break scan + case mriOnlyLongOptions[name]: + return nil, unimplemented("--" + name) + default: + return nil, rubyRaise("invalid option %s (-h will show valid options)", arg) + } + default: + c := s[0] + if _, ok := mriOnlyOptions[c]; ok { + return nil, unimplemented("-" + string(rune(c))) + } + // The default arm prints ONE character, not the rest of the word: + // rb_enc_precise_mbclen gives the first character's length and the + // raise uses "-%.*s" with it. `ruby -Zabc` says "invalid option -Z". + return nil, rubyRaise("invalid option -%s (-h will show valid options)", firstChar(s)) + } + } + } + + if o.help { + // process_options tests the usage/help dump bit BEFORE it does `argc -= i`, + // and returns straight away — no script entry is taken and ARGV is never + // filled. Mirror that: -h answers a question about rbgo, it does not run a + // program. + o.argv = nil + return o, nil + } + + rest := args[i:] + // process_options: one entry becomes the script, but only when -e did not + // already supply the program. + if !o.haveE && len(rest) > 0 { + o.script = rest[0] + rest = rest[1:] + } + o.argv = rest + return o, nil +} + +// defaultWarnLevel is $VERBOSE == false, MRI's starting state and what -W1 asks +// for explicitly. +const defaultWarnLevel = 1 + +// unimplemented reports an option ruby has and rbgo does not. It is a failure, +// not a warning: silently ignoring -n would run the program under semantics the +// user did not ask for. +func unimplemented(opt string) *optError { + return &optError{text: "rbgo: " + opt + " is a ruby option that rbgo does not implement"} +} + +// firstChar returns s's first UTF-8 character (or its first byte when that byte +// does not begin a valid sequence), which is the unit MRI names in the +// invalid-option message. +func firstChar(s string) string { + for i := 1; i <= len(s); i++ { + if i == len(s) || (s[i]&0xC0) != 0x80 { + return s[:i] + } + } + return s +} + +// setWarn records a level. proc_options reads opt->warning, which it only commits +// at switch_end, so every -w/-W on one command line takes effect and the LAST one +// wins: `ruby -W0 -w` is verbose and `ruby -w -W0` is silent (both measured). +func (o *options) setWarn(level int) { + o.warnLevel = level + o.warnSet = true +} + +// procW is proc_W_option. s starts at the 'W'. It returns the unconsumed tail of +// the cluster and whether the scan should continue chewing it (MRI's `goto +// reswitch` versus its `break`). +func (o *options) procW(s string, stderr io.Writer) (rest string, more bool, err error) { + if len(s) > 1 && s[1] == ':' { + // -W:name / -W:no-name. proc_W_option returns 0 here, so the rest of the + // word is never re-scanned. + name := strings.TrimPrefix(s[2:], "no-") + if !warnCategories[name] { + // rb_warn, so it is silent when $VERBOSE is already nil. + if o.warnLevel > 0 { + fmt.Fprintf(stderr, "rbgo: warning: unknown warning category: '%s'\n", name) + } + } + return "", false, nil + } + // The numeric form. scan_oct(s, 1, &numlen) reads AT MOST ONE octal digit; + // numlen == 0 leaves v at 2 and leaves the character in place, so "-W9" + // selects level 2 and then fails on "-9" the way MRI does. + v := 2 + tail := s[1:] + if tail != "" && tail[0] >= '0' && tail[0] <= '7' { + v = int(tail[0] - '0') + tail = tail[1:] + } + o.setWarn(v) + return tail, true, nil +} + +// load resolves the program text and the name it runs under ($0, __FILE__ and the +// label in a backtrace). fromFile says whether the name is a real path, which is +// what decides between SetScriptPath (require_relative resolves against the +// script's directory) and SetScriptName. +func (o *options) load() (src, name string, fromFile bool, err error) { + if o.haveE { + // proc_e_option: `if (opt->script == 0) opt->script = "-e";` + return o.eScript, "-e", false, nil + } + if o.script == "" || o.script == "-" { + // process_options: `if (argc <= 0) opt->script = "-";` — with no script + // entry left, and with an explicit "-", the program comes from stdin and + // $0 is "-". + b, rerr := io.ReadAll(os.Stdin) + if rerr != nil { + return "", "", false, rerr + } + return string(b), "-", false, nil + } + b, rerr := os.ReadFile(o.script) + if rerr != nil { + return "", "", false, rerr + } + return string(b), o.script, true, nil +} diff --git a/cmd/rbgo/options_test.go b/cmd/rbgo/options_test.go new file mode 100644 index 00000000..4a5d00b2 --- /dev/null +++ b/cmd/rbgo/options_test.go @@ -0,0 +1,411 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +//go:build !rbgo_closed + +package main + +import ( + "io" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// Every "want" in this file was measured against the oracle, MRI ruby 4.0.5 +// (2026-05-20 revision 64336ffd0e) +PRISM [arm64-darwin25], by running the same +// argv through `ruby` with stdout, stderr and the exit status captured +// separately. Where the oracle and the cited v3_4_0 source could disagree the +// oracle decided; the two agree everywhere below. +// +// These are in-process tests of parseOptions and runProgram, so they are coverage +// of THIS package's code. The process-level table (rbgo versus ruby, byte for +// byte) lives in the pull request body: it measures the built binary and cannot +// cover the functions it exercises. + +// TestParseOptionsSplitsTheCommandLine is the witness for #708. On origin/main +// runCmd accepted only `-e code` or a single bare word, so every row here with an +// argument after the script was a usage error and exit 2. +// +// A/B: change the loop's guard from `len(arg) < 2 || arg[0] != '-'` to +// `arg[0] != '-'` — it compiles, and rows "dash alone is a script" and "empty +// string is not an option" then fail, because "-" and "" are no longer the +// boundary proc_options makes them. +func TestParseOptionsSplitsTheCommandLine(t *testing.T) { + cases := []struct { + name string + args []string + script string + eScript string + argv []string + warnLevel int + warnSet bool + help bool + }{ + {name: "script plus arguments", args: []string{"t.rb", "alpha", "beta"}, + script: "t.rb", argv: []string{"alpha", "beta"}, warnLevel: 1}, + {name: "script alone", args: []string{"t.rb"}, + script: "t.rb", argv: []string{}, warnLevel: 1}, + // Measured: `ruby t.rb -- -w` prints ["--", "-w"]. Options stopped at the + // script, so the "--" after it is an ordinary argument. The brief for this + // work predicted ["-w"]; the oracle says otherwise. + {name: "dash-dash AFTER the script is just an argument", args: []string{"t.rb", "--", "-w"}, + script: "t.rb", argv: []string{"--", "-w"}, warnLevel: 1}, + {name: "dash-dash BEFORE the script ends the options", args: []string{"--", "t.rb", "a", "b"}, + script: "t.rb", argv: []string{"a", "b"}, warnLevel: 1}, + {name: "a flag-shaped argument after the script is an argument", args: []string{"t.rb", "-w"}, + script: "t.rb", argv: []string{"-w"}, warnLevel: 1}, + {name: "dash alone is a script (stdin), not an option", args: []string{"-", "a"}, + script: "-", argv: []string{"a"}, warnLevel: 1}, + {name: "empty string is not an option", args: []string{"", "a"}, + script: "", argv: []string{"a"}, warnLevel: 1}, + // -e takes no script entry, so everything left is ARGV... + {name: "-e with arguments", args: []string{"-e", "p ARGV", "a", "b"}, + eScript: "p ARGV\n", argv: []string{"a", "b"}, warnLevel: 1}, + // ...but -e does NOT end option processing: measured, `ruby -e 'p ARGV' -w` + // prints [] and runs verbose. + {name: "-e then a flag: still a flag", args: []string{"-e", "p ARGV", "-w"}, + eScript: "p ARGV\n", argv: []string{}, warnLevel: 2, warnSet: true}, + {name: "-e with attached code", args: []string{"-ep ARGV", "a"}, + eScript: "p ARGV\n", argv: []string{"a"}, warnLevel: 1}, + {name: "-e twice concatenates with newlines", args: []string{"-e", "p 1", "-e", "p 2", "a"}, + eScript: "p 1\np 2\n", argv: []string{"a"}, warnLevel: 1}, + {name: "-e then dash-dash", args: []string{"-e", "p ARGV", "--", "-a"}, + eScript: "p ARGV\n", argv: []string{"-a"}, warnLevel: 1}, + // The four -W states, and all of them with script arguments, which is where + // #708 and #709 meet. + {name: "-W0 with arguments", args: []string{"-W0", "t.rb", "a"}, + script: "t.rb", argv: []string{"a"}, warnLevel: 0, warnSet: true}, + {name: "-W1 with arguments", args: []string{"-W1", "t.rb", "a"}, + script: "t.rb", argv: []string{"a"}, warnLevel: 1, warnSet: true}, + {name: "-W2 with arguments", args: []string{"-W2", "t.rb", "a"}, + script: "t.rb", argv: []string{"a"}, warnLevel: 2, warnSet: true}, + {name: "-w with arguments", args: []string{"-w", "t.rb", "a"}, + script: "t.rb", argv: []string{"a"}, warnLevel: 2, warnSet: true}, + {name: "bare -W is -W2", args: []string{"-W", "t.rb"}, + script: "t.rb", argv: []string{}, warnLevel: 2, warnSet: true}, + // MRI does NOT clamp: scan_oct gives v == 3 and the switch's `default` + // arm sets ruby_verbose = Qtrue. The level is kept raw here for the same + // reason, so the mapping to $VERBOSE lives in one place + // (vm.SetWarningLevel); what a program can observe is pinned by + // TestCommandLineReachesTheProgram's -W3 row, where $VERBOSE is true. + {name: "-W3 is level 3, which maps to verbose", args: []string{"-W3", "t.rb"}, + script: "t.rb", argv: []string{}, warnLevel: 3, warnSet: true}, + // Measured both ways round: within ONE command line the last -w/-W wins, + // because proc_options only commits opt->warning at switch_end. + {name: "-W0 then -w: verbose", args: []string{"-W0", "-w", "t.rb"}, + script: "t.rb", argv: []string{}, warnLevel: 2, warnSet: true}, + {name: "-w then -W0: silent", args: []string{"-w", "-W0", "t.rb"}, + script: "t.rb", argv: []string{}, warnLevel: 0, warnSet: true}, + {name: "clustered -wW0 is silent", args: []string{"-wW0", "t.rb"}, + script: "t.rb", argv: []string{}, warnLevel: 0, warnSet: true}, + {name: "-W:category does not change the level", args: []string{"-W:deprecated", "t.rb", "a"}, + script: "t.rb", argv: []string{"a"}, warnLevel: 1}, + {name: "-W:no-category does not change the level", args: []string{"-W:no-performance", "t.rb"}, + script: "t.rb", argv: []string{}, warnLevel: 1}, + // -h stops the scan where it stands (MRI's `goto switch_end`), so nothing + // after it is even looked at — including an otherwise invalid option. + {name: "-h stops the scan", args: []string{"-h", "-Z"}, + argv: nil, warnLevel: 1, help: true}, + {name: "--help stops the scan", args: []string{"--help", "-Z"}, + argv: nil, warnLevel: 1, help: true}, + {name: "no arguments: stdin, empty ARGV", args: []string{}, + argv: []string{}, warnLevel: 1}, + {name: "options but no script: stdin", args: []string{"-W0"}, + argv: []string{}, warnLevel: 0, warnSet: true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + o, err := parseOptions(c.args, io.Discard) + if err != nil { + t.Fatalf("parseOptions(%q): %v", c.args, err) + } + if o.script != c.script { + t.Errorf("script = %q, want %q", o.script, c.script) + } + if o.eScript != c.eScript { + t.Errorf("eScript = %q, want %q", o.eScript, c.eScript) + } + if o.haveE != (c.eScript != "") { + t.Errorf("haveE = %v, want %v", o.haveE, c.eScript != "") + } + // An empty ARGV and a nil ARGV are the same program-visible value, so + // compare lengths first and contents only when there is something. + if len(o.argv) != len(c.argv) || (len(c.argv) > 0 && !reflect.DeepEqual(o.argv, c.argv)) { + t.Errorf("argv = %#v, want %#v", o.argv, c.argv) + } + if o.warnLevel != c.warnLevel { + t.Errorf("warnLevel = %d, want %d", o.warnLevel, c.warnLevel) + } + if o.warnSet != c.warnSet { + t.Errorf("warnSet = %v, want %v", o.warnSet, c.warnSet) + } + if o.help != c.help { + t.Errorf("help = %v, want %v", o.help, c.help) + } + }) + } +} + +// TestParseOptionsErrorsAreMRIs pins the exact stderr line, because the whole +// point of #708 is that rbgo printed its usage text where ruby raises. The text +// comes from the oracle, with "ruby: " replaced by "rbgo: " (rb_progname) and +// nothing else changed — note the TWO spaces before "(-h". +// +// A/B: drop the second space from the invalid-option format and every +// "invalid option" row fails while the code still compiles. +func TestParseOptionsErrorsAreMRIs(t *testing.T) { + cases := []struct { + args []string + want string + }{ + {[]string{"-Z", "t.rb"}, "rbgo: invalid option -Z (-h will show valid options) (RuntimeError)"}, + // Measured: ruby names ONE character, not the rest of the cluster. + {[]string{"-Zabc", "t.rb"}, "rbgo: invalid option -Z (-h will show valid options) (RuntimeError)"}, + // -W9: scan_oct reads one OCTAL digit, so '9' is not consumed; the level + // stays 2 and the scan then rejects "-9". + {[]string{"-W9", "t.rb"}, "rbgo: invalid option -9 (-h will show valid options) (RuntimeError)"}, + {[]string{"--bogus", "t.rb"}, "rbgo: invalid option --bogus (-h will show valid options) (RuntimeError)"}, + {[]string{"-e"}, "rbgo: no code specified for -e (RuntimeError)"}, + // A switch ruby HAS and rbgo does not: calling it "invalid" would be a + // false statement about Ruby, so it gets its own wording — and still fails. + {[]string{"-n", "t.rb"}, "rbgo: -n is a ruby option that rbgo does not implement"}, + {[]string{"--version"}, "rbgo: --version is a ruby option that rbgo does not implement"}, + } + for _, c := range cases { + t.Run(strings.Join(c.args, " "), func(t *testing.T) { + _, err := parseOptions(c.args, io.Discard) + if err == nil { + t.Fatalf("parseOptions(%q) = no error, want %q", c.args, c.want) + } + if err.Error() != c.want { + t.Errorf("error = %q, want %q", err.Error(), c.want) + } + }) + } +} + +// TestUnknownWarningCategoryIsGatedLikeRbWarn: proc_W_option reports an unknown +// -W:name through rb_warn, which is silent when $VERBOSE is nil. Measured: +// `ruby -W:bogus t.rb` warns and `ruby -W0 -W:bogus t.rb` does not. +// +// A/B: remove the `if o.warnLevel > 0` guard in procW — it compiles, and the +// second row fails. +func TestUnknownWarningCategoryIsGatedLikeRbWarn(t *testing.T) { + for _, c := range []struct { + name string + args []string + want string + }{ + {"unknown category warns", []string{"-W:bogus", "t.rb"}, + "rbgo: warning: unknown warning category: 'bogus'\n"}, + {"silent under -W0", []string{"-W0", "-W:bogus", "t.rb"}, ""}, + {"known category says nothing", []string{"-W:experimental", "t.rb"}, ""}, + {"known category with no- prefix says nothing", []string{"-W:no-experimental", "t.rb"}, ""}, + } { + t.Run(c.name, func(t *testing.T) { + var sb strings.Builder + if _, err := parseOptions(c.args, &sb); err != nil { + t.Fatalf("parseOptions(%q): %v", c.args, err) + } + if sb.String() != c.want { + t.Errorf("stderr = %q, want %q", sb.String(), c.want) + } + }) + } +} + +// TestLoadResolvesTheProgram covers options.load: the three sources a program can +// come from and the name each runs under ($0 / __FILE__ / the backtrace label). +func TestLoadResolvesTheProgram(t *testing.T) { + dir := t.TempDir() + script := filepath.Join(dir, "s.rb") + if err := os.WriteFile(script, []byte("puts 1\n"), 0o644); err != nil { + t.Fatal(err) + } + + t.Run("a file keeps its path and is require-relative-able", func(t *testing.T) { + o, err := parseOptions([]string{script, "a"}, io.Discard) + if err != nil { + t.Fatal(err) + } + src, name, fromFile, err := o.load() + if err != nil { + t.Fatal(err) + } + if src != "puts 1\n" || name != script || !fromFile { + t.Errorf("load = (%q, %q, %v), want (%q, %q, true)", src, name, fromFile, "puts 1\n", script) + } + }) + + t.Run("-e is named -e and has no script directory", func(t *testing.T) { + o, err := parseOptions([]string{"-e", "p 1"}, io.Discard) + if err != nil { + t.Fatal(err) + } + src, name, fromFile, err := o.load() + if err != nil { + t.Fatal(err) + } + if src != "p 1\n" || name != "-e" || fromFile { + t.Errorf("load = (%q, %q, %v), want (%q, \"-e\", false)", src, name, fromFile, "p 1\n") + } + }) + + t.Run("dash reads stdin and is named -", func(t *testing.T) { + src, name, fromFile := loadWithStdin(t, []string{"-", "a"}, "p ARGV\n") + if src != "p ARGV\n" || name != "-" || fromFile { + t.Errorf("load = (%q, %q, %v), want (%q, \"-\", false)", src, name, fromFile, "p ARGV\n") + } + }) + + t.Run("no script left also reads stdin", func(t *testing.T) { + src, name, fromFile := loadWithStdin(t, []string{"-W0"}, "p 2\n") + if src != "p 2\n" || name != "-" || fromFile { + t.Errorf("load = (%q, %q, %v), want (%q, \"-\", false)", src, name, fromFile, "p 2\n") + } + }) + + t.Run("a missing file is an error, not a panic", func(t *testing.T) { + o, err := parseOptions([]string{filepath.Join(dir, "nope.rb")}, io.Discard) + if err != nil { + t.Fatal(err) + } + if _, _, _, err := o.load(); err == nil { + t.Error("load of a missing file returned no error") + } + }) +} + +// loadWithStdin points os.Stdin at text for the duration of one load(). +func loadWithStdin(t *testing.T, args []string, text string) (src, name string, fromFile bool) { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + saved := os.Stdin + os.Stdin = r + defer func() { os.Stdin = saved; r.Close() }() + go func() { _, _ = w.WriteString(text); w.Close() }() + + o, err := parseOptions(args, io.Discard) + if err != nil { + t.Fatal(err) + } + src, name, fromFile, err = o.load() + if err != nil { + t.Fatal(err) + } + return src, name, fromFile +} + +// TestCommandLineReachesTheProgram is the end of the wire: the parsed options +// actually change what the interpreted program sees. It runs in-process with +// os.Stdout and os.Stderr pointed at pipes, so it covers runProgram's seeding +// calls rather than a subprocess's behaviour. +// +// Each want was measured with the same argv through MRI 4.0.5. +// +// A/B: delete the `machine.SetARGV(o.argv)` line and the argv rows fail; delete +// the `machine.SetWarningLevel` call and the -W0 row fails. Both still compile. +func TestCommandLineReachesTheProgram(t *testing.T) { + dir := t.TempDir() + script := filepath.Join(dir, "probe.rb") + // $0, __FILE__ and $PROGRAM_NAME are one value in MRI and must be the path as + // given; ARGV's members are frozen (ruby_set_argv's OBJ_FREEZE); ARGV and $* + // are the same object. + program := "p [$0 == __FILE__, $0 == $PROGRAM_NAME, ARGV, ARGV.map(&:frozen?), ARGV.equal?($*), $VERBOSE]\n" + + "K = 1\nK = 2\n" + if err := os.WriteFile(script, []byte(program), 0o644); err != nil { + t.Fatal(err) + } + + for _, c := range []struct { + name string + args []string + wantOut string + wantErr bool // a redefinition warning on fd 2 + }{ + {"arguments reach ARGV", []string{script, "alpha", "beta"}, + "[true, true, [\"alpha\", \"beta\"], [true, true], true, false]\n", true}, + {"-W0 silences and is visible as nil", []string{"-W0", script, "a"}, + "[true, true, [\"a\"], [true], true, nil]\n", false}, + {"-w with arguments is verbose AND keeps ARGV", []string{"-w", script, "a"}, + "[true, true, [\"a\"], [true], true, true]\n", true}, + {"-W3 is verbose too", []string{"-W3", script, "a"}, + "[true, true, [\"a\"], [true], true, true]\n", true}, + {"-W1 is the default", []string{"-W1", script, "a"}, + "[true, true, [\"a\"], [true], true, false]\n", true}, + {"-- before the script", []string{"--", script, "a"}, + "[true, true, [\"a\"], [true], true, false]\n", true}, + {"a -- after the script is an argument", []string{script, "--", "-w"}, + "[true, true, [\"--\", \"-w\"], [true, true], true, false]\n", true}, + } { + t.Run(c.name, func(t *testing.T) { + o, err := parseOptions(c.args, io.Discard) + if err != nil { + t.Fatalf("parseOptions: %v", err) + } + src, name, fromFile, err := o.load() + if err != nil { + t.Fatalf("load: %v", err) + } + gotOut, gotErr := captureRun(t, func() { + if _, err := runProgram(src, name, fromFile, o); err != nil { + t.Errorf("runProgram: %v", err) + } + }) + if gotOut != c.wantOut { + t.Errorf("fd 1 = %q, want %q", gotOut, c.wantOut) + } + if warned := strings.Contains(gotErr, "already initialized constant K"); warned != c.wantErr { + t.Errorf("fd 2 warned = %v (%q), want %v", warned, gotErr, c.wantErr) + } + }) + } +} + +// TestRunKeepsAnEmptyARGV pins the embedded/test entry point: run() must not +// invent arguments for a program that was given none. +func TestRunKeepsAnEmptyARGV(t *testing.T) { + gotOut, _ := captureRun(t, func() { + if _, err := run("p [ARGV, $VERBOSE, $0]", "-e"); err != nil { + t.Errorf("run: %v", err) + } + }) + if want := "[[], false, \"-e\"]\n"; gotOut != want { + t.Errorf("fd 1 = %q, want %q", gotOut, want) + } +} + +// captureRun points os.Stdout and os.Stderr at pipes for the duration of fn and +// returns what each received. Both pipes are drained concurrently: a pipe holds +// only a buffer's worth, and a blocked write would hang the run rather than fail +// the test. +func captureRun(t *testing.T, fn func()) (stdout, stderr string) { + t.Helper() + outR, outW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + errR, errW, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + savedOut, savedErr := os.Stdout, os.Stderr + os.Stdout, os.Stderr = outW, errW + outCh, errCh := make(chan string, 1), make(chan string, 1) + go func() { b, _ := io.ReadAll(outR); outCh <- string(b) }() + go func() { b, _ := io.ReadAll(errR); errCh <- string(b) }() + + fn() + + outW.Close() + errW.Close() + os.Stdout, os.Stderr = savedOut, savedErr + return <-outCh, <-errCh +} diff --git a/internal/vm/host_cmdline.go b/internal/vm/host_cmdline.go new file mode 100644 index 00000000..90068b2b --- /dev/null +++ b/internal/vm/host_cmdline.go @@ -0,0 +1,66 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import "github.com/go-embedded-ruby/ruby/internal/object" + +// This file is the HOST side of MRI's command line: the two things ruby.c does to +// a fresh interpreter before it runs the program, which no Ruby-level construct +// can do for itself. The CLI front-end (cmd/rbgo) is the only caller; an +// embedding host may use them for the same purpose. + +// SetARGV replaces ARGV's contents with args, which is what ruby.c ruby_set_argv +// does (`rb_ary_clear(rb_argv)` then a push per argument). +// +// It mutates the EXISTING Array in place rather than installing a new one, +// because ARGV (the constant) and $* are one object here as in MRI: replacing the +// constant would leave $* pointing at the old, empty Array, and ARGF — which +// shifts filenames off the live ARGV (argf.go) — would then draw from whichever +// of the two it happened to hold. +// +// Each element is frozen, matching ruby_set_argv's OBJ_FREEZE(arg): `ARGV.first +// << "x"` raises FrozenError in MRI, and a test asserting frozen? on ARGV's +// members would otherwise pass for the wrong reason. +func (vm *VM) SetARGV(args []string) { + argv, ok := vm.consts["ARGV"].(*object.Array) + if !ok { + // A host that replaced the constant with something else; install a fresh + // Array so $* and ARGV are at least the same object again. + argv = object.NewArray() + vm.consts["ARGV"] = argv + vm.globals["$*"] = argv + } + elems := argv.Elems[:0] + for _, a := range args { + elems = append(elems, object.NewFrozenStringView(a)) + } + argv.Elems = elems +} + +// SetWarningLevel puts $VERBOSE into the state -W selects, which is the +// numeric branch of ruby.c proc_W_option (v3_4_0 ruby.c:1250-1268, moved into +// proc_W_option by ruby_4_0): +// +// 0 ruby_verbose = Qnil every rb_warn and rb_warning is silent +// 1 ruby_verbose = Qfalse rb_warn speaks, rb_warning stays quiet (the default) +// >=2 ruby_verbose = Qtrue both speak (this is what -w and -W2 select) +// +// A negative level cannot come from proc_W_option (scan_oct reads one octal +// digit) and is treated as 0, the silent end of the same scale. +// +// It writes the slot directly rather than through the $VERBOSE setter because +// there is no Ruby frame to raise in yet; the three values it writes are exactly +// the three verbose_setter can produce, so no second representation is created +// (see warnEnabled/warningEnabled in globals.go). +func (vm *VM) SetWarningLevel(level int) { + switch { + case level <= 0: + vm.globals["$VERBOSE"] = object.NilVal() + case level == 1: + vm.globals["$VERBOSE"] = object.Bool(false) + default: + vm.globals["$VERBOSE"] = object.Bool(true) + } +} diff --git a/internal/vm/host_cmdline_test.go b/internal/vm/host_cmdline_test.go new file mode 100644 index 00000000..dce90657 --- /dev/null +++ b/internal/vm/host_cmdline_test.go @@ -0,0 +1,161 @@ +// Copyright (c) the go-embedded-ruby/ruby authors +// +// SPDX-License-Identifier: BSD-3-Clause + +package vm + +import ( + "bytes" + "testing" + + "github.com/go-embedded-ruby/ruby/internal/compiler" + "github.com/go-embedded-ruby/ruby/internal/object" + "github.com/go-ruby-parser/parser" +) + +// runHost compiles and runs src in one VM after letting seed configure it the way +// a host does before Run, and returns everything the program wrote. +func runHost(t *testing.T, src string, seed func(*VM)) string { + t.Helper() + prog, err := parser.Parse(src) + if err != nil { + t.Fatalf("parse: %v", err) + } + iseq, err := compiler.Compile(prog) + if err != nil { + t.Fatalf("compile: %v", err) + } + var buf bytes.Buffer + vm := New(&buf) + if seed != nil { + seed(vm) + } + if _, err := vm.Run(iseq); err != nil { + t.Fatalf("run: %v", err) + } + return buf.String() +} + +// TestSetARGVIsRubySetArgv covers the host half of #708. Every "want" was measured +// against MRI ruby 4.0.5 with the equivalent command line. +// +// The identity rows are the point: ruby_set_argv clears and refills the ONE Array +// that ARGV and $* both name, so a fresh Array installed over the constant would +// leave $* empty and ARGF (argf.go, which shifts filenames off the live ARGV) +// drawing from whichever of the two it happened to hold. +// +// A/B: replace the in-place refill with `vm.consts["ARGV"] = object.NewArray(...)` +// — it compiles, and the `$*` rows fail while the plain `p ARGV` row still passes. +func TestSetARGVIsRubySetArgv(t *testing.T) { + for _, c := range []struct { + name string + args []string + src string + want string + }{ + {"arguments arrive in order", []string{"a", "b"}, "p ARGV", "[\"a\", \"b\"]\n"}, + {"no arguments is an empty Array, not nil", nil, "p ARGV", "[]\n"}, + {"$* is the same object", []string{"a"}, "p ARGV.equal?($*)", "true\n"}, + {"a mutation through ARGV shows in $*", []string{"a", "b"}, "ARGV.shift; p $*", "[\"b\"]\n"}, + {"a mutation through $* shows in ARGV", []string{"a", "b"}, "$*.pop; p ARGV", "[\"a\"]\n"}, + // ruby_set_argv does OBJ_FREEZE(arg) on every element. + {"elements are frozen", []string{"a", "b"}, "p ARGV.map(&:frozen?)", "[true, true]\n"}, + {"a frozen element raises on mutation", []string{"a"}, + "begin; ARGV[0] << \"x\"; rescue => e; p e.class; end", "FrozenError\n"}, + {"the empty string survives as an argument", []string{"", "b"}, "p ARGV", "[\"\", \"b\"]\n"}, + {"a second call replaces rather than appends", []string{"a"}, "p ARGV", "[\"a\"]\n"}, + } { + t.Run(c.name, func(t *testing.T) { + got := runHost(t, c.src, func(vm *VM) { + if c.name == "a second call replaces rather than appends" { + vm.SetARGV([]string{"stale", "values"}) + } + vm.SetARGV(c.args) + }) + if got != c.want { + t.Errorf("%s = %q, want %q", c.src, got, c.want) + } + }) + } +} + +// TestSetARGVRepairsAReplacedConstant covers the branch where a host put +// something other than an Array under ARGV: the Array is rebuilt and $* is +// re-pointed at it, so the two cannot end up naming different objects. +func TestSetARGVRepairsAReplacedConstant(t *testing.T) { + got := runHost(t, "p [ARGV, ARGV.equal?($*)]", func(vm *VM) { + vm.SetConst("ARGV", object.NewString("not an array")) + vm.SetARGV([]string{"a"}) + }) + if want := "[[\"a\"], true]\n"; got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +// TestSetWarningLevelIsProcWOption covers the host half of #709: the three states +// -W selects. Measured with `ruby -W0 / -W1 / -W2 / -w / -W3` on a program +// that redefines a constant (an rb_warn) and prints $VERBOSE and $-W. +// +// The redefinition warning is the discriminator, so each row asserts BOTH the +// value a program reads and whether the warning was emitted — a row that only +// read $VERBOSE would pass even if nothing were gated on it. +// +// A/B: make the `level == 1` arm write object.NilVal() — it compiles, and the +// level-1 row fails on both the value and the warning. +func TestSetWarningLevelIsProcWOption(t *testing.T) { + const src = "X = 1\nX = 2\np [$VERBOSE, $-W]\n" + for _, c := range []struct { + name string + level int + wantState string + wantWarn bool + }{ + {"-W0 is nil and silences rb_warn", 0, "[nil, 0]\n", false}, + {"-W1 is false and lets rb_warn speak", 1, "[false, 1]\n", true}, + {"-W2 is true", 2, "[true, 2]\n", true}, + {"-W3 saturates at true", 3, "[true, 2]\n", true}, + // scan_oct cannot produce a negative level; treating one as the silent end + // of the scale is the only defined answer, and it is asserted rather than + // left to whichever arm happens to catch it. + {"a negative level is the silent end", -1, "[nil, 0]\n", false}, + } { + t.Run(c.name, func(t *testing.T) { + level := c.level + got := runHost(t, src, func(vm *VM) { vm.SetWarningLevel(level) }) + warned := bytes.Contains([]byte(got), []byte("already initialized constant X")) + if warned != c.wantWarn { + t.Errorf("warned = %v, want %v (output %q)", warned, c.wantWarn, got) + } + // The state line is the last line; the warnings (when any) precede it. + lines := bytes.Split([]byte(got), []byte("\n")) + last := string(lines[len(lines)-2]) + "\n" + if last != c.wantState { + t.Errorf("[$VERBOSE, $-W] = %q, want %q", last, c.wantState) + } + }) + } +} + +// TestSetWarningLevelLeavesOneRepresentation guards the defect globals.go warns +// about: $VERBOSE must have exactly one representation. SetWarningLevel writes the +// slot, so what a Ruby program reads and what warnEnabled/warningEnabled read have +// to agree for all three states. +func TestSetWarningLevelLeavesOneRepresentation(t *testing.T) { + for _, c := range []struct { + level int + wantWarn, wantWarn2 bool + }{ + {0, false, false}, + {1, true, false}, + {2, true, true}, + } { + vm := New(&bytes.Buffer{}) + vm.SetWarningLevel(c.level) + if got := vm.warnEnabled(); got != c.wantWarn { + t.Errorf("level %d: warnEnabled = %v, want %v", c.level, got, c.wantWarn) + } + if got := vm.warningEnabled(); got != c.wantWarn2 { + t.Errorf("level %d: warningEnabled = %v, want %v", c.level, got, c.wantWarn2) + } + } +} From 67e773eccb17031954f43aa2b8f5e44f66fbaf76 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 27 Sep 2026 12:43:34 +0200 Subject: [PATCH 2/6] Assert the closed-binary and wasm argv end to end, and correct the wasm claim TestClosedBuildIntegration now builds a closed binary from a program that prints ARGV and exits with its length, runs it with `-W0 alpha beta`, and asserts all three arrived as data (not as a switch) and that the status is 3. $0 was the EMPTY string in a closed binary before this change; it is now the invoked binary. TestClosedWasmBuildIntegration passes two arguments to the node glue and asserts they reach the embedded program's ARGV. That last assertion corrected a claim this branch made a moment ago. "wasm has no argv" is wrong for the host CI actually uses: Go's own lib/wasm/wasm_exec_node.js passes process.argv.slice(2) through, so a js/wasm module run under node DOES get arguments -- measured, ARGV == ["alpha", "beta"]. A browser-loaded module has none, and wasip1 takes them from WASI args_get; the comment in closed_argv.go now says which of those three rows was measured and which was not. The closed test deliberately uses a program containing a string literal. On origin/main (87e418bc) `rbgo build --closed` FAILS for a program whose frozen bytecode holds no object value at all -- `p ARGV` is enough -- because aot.FreezeISeq emits the internal/object import unconditionally and the nested go build rejects it as unused. That defect is in internal/aot, not here, and is left for its own change. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/closed_argv.go | 22 ++++++++++++++++------ cmd/rbgo/closed_build_test.go | 33 +++++++++++++++++++++++++++++++++ cmd/rbgo/wasm_build_test.go | 15 +++++++++++++-- 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/cmd/rbgo/closed_argv.go b/cmd/rbgo/closed_argv.go index f96c367e..4f966532 100644 --- a/cmd/rbgo/closed_argv.go +++ b/cmd/rbgo/closed_argv.go @@ -23,12 +23,22 @@ import ( // would be lying about whose command line it is. // // It is shared by the native and the wasm closed mains, and it reads os.Args on -// both, so no build tag decides whether a closed binary can take arguments. What -// differs is what the host puts there: wasip1 fills os.Args from the WASI -// args_get call, so `wasmtime prog.wasm a b` arrives intact, while a js/wasm -// module loaded by a browser has no process arguments at all and os.Args carries -// only the program name — ARGV is then empty, which is the honest answer rather -// than a tag-selected one. +// both, so no BUILD TAG decides whether a closed binary can take arguments — +// only the host does, which is the honest answer and not a tag-selected one. +// +// What each host puts there was measured, not assumed: +// +// - native: the process's argv, as usual. +// - js/wasm under node: Go's own lib/wasm/wasm_exec_node.js glue passes +// process.argv.slice(2) through, so `node wasm_exec_node.js prog.wasm a b` +// arrives as os.Args == {"prog.wasm", "a", "b"} and ARGV == ["a", "b"]. +// Verified end to end by TestClosedWasmBuildIntegration. +// - js/wasm in a browser: there is no process argv at all; os.Args carries only +// "js", so ARGV is empty. Nothing here special-cases that. +// - wasip1: os.Args comes from the WASI args_get call, so a host that passes +// arguments delivers them. This machine has no wasip1 runtime installed, so +// that row is reasoned from the Go runtime's own wasip1 os.Args and is the +// one row below that was NOT measured. func seedProcessArgs(machine *vm.VM) { if len(os.Args) > 0 { machine.SetScriptName(os.Args[0]) diff --git a/cmd/rbgo/closed_build_test.go b/cmd/rbgo/closed_build_test.go index b715ff43..729bab8b 100644 --- a/cmd/rbgo/closed_build_test.go +++ b/cmd/rbgo/closed_build_test.go @@ -64,6 +64,39 @@ func TestClosedBuildIntegration(t *testing.T) { t.Errorf("closed binary (%d) is not smaller than open (%d)", cs, os_) } + // A standalone binary that cannot take arguments is not a usable program + // (#708). Its arguments are its OWN: there is no interpreter command line in + // front of them, so "-W0" here is a datum and not a switch, and the exit + // status proves the program saw all three. + // + // The program deliberately carries a string literal. On origin/main + // (87e418bc) `rbgo build --closed` fails outright for a program whose frozen + // bytecode holds NO object value — `p ARGV` is enough — because + // aot.FreezeISeq emits the internal/object import unconditionally and the + // nested go build then rejects it as unused. That is a separate defect, in + // internal/aot rather than here; this test must not depend on it. + argvRb := filepath.Join(dir, "argv.rb") + argvProg := "puts \"argv=#{ARGV.inspect} zero=#{$0.empty? ? \"(empty)\" : \"(set)\"}\"\n" + + "exit ARGV.length\n" + if err := os.WriteFile(argvRb, []byte(argvProg), 0o644); err != nil { + t.Fatal(err) + } + argvBin := filepath.Join(dir, "argv_closed") + rbgoBuild(t, root, "-o", argvBin, "--closed", argvRb) + + argvCmd := exec.Command(argvBin, "-W0", "alpha", "beta") + argvOut, argvErr := argvCmd.CombinedOutput() + // Measured: three arguments, so `exit 3`. + if code := argvCmd.ProcessState.ExitCode(); code != 3 { + t.Errorf("closed binary exit status = %d, want 3 (it saw %d arguments); err=%v out=%s", + code, 3, argvErr, argvOut) + } + // $0 was the EMPTY string in a closed binary before this: nothing set it. + const wantArgv = "argv=[\"-W0\", \"alpha\", \"beta\"] zero=(set)\n" + if string(argvOut) != wantArgv { + t.Errorf("closed binary with arguments printed %q, want %q", argvOut, wantArgv) + } + // eval in a closed binary raises NotImplementedError (front-end dropped). ev := filepath.Join(dir, "ev.rb") if err := os.WriteFile(ev, []byte("puts eval(\"1 + 1\")\n"), 0o644); err != nil { diff --git a/cmd/rbgo/wasm_build_test.go b/cmd/rbgo/wasm_build_test.go index d33e2755..9f92dda9 100644 --- a/cmd/rbgo/wasm_build_test.go +++ b/cmd/rbgo/wasm_build_test.go @@ -83,7 +83,14 @@ func TestClosedWasmBuildIntegration(t *testing.T) { app := filepath.Join(dir, "app.rb") program := "JS.log(\"closed wasm ruby ran\")\n" + - "JS.log(\"document is: \" + JS.document.to_s)\n" + "JS.log(\"document is: \" + JS.document.to_s)\n" + + // #708 on the wasm side: whether a closed binary can take arguments must + // not be decided by a build tag. seedProcessArgs reads os.Args on every + // target; what differs is what the HOST supplies. Under node, Go's own + // wasm_exec_node.js glue passes process.argv.slice(2) through, so the two + // arguments given below arrive — MEASURED here rather than assumed, because + // the assumption ("wasm has no argv") is wrong for this host. + "JS.log(\"argv=\" + ARGV.inspect)\n" if err := os.WriteFile(app, []byte(program), 0o644); err != nil { t.Fatal(err) } @@ -121,7 +128,7 @@ func TestClosedWasmBuildIntegration(t *testing.T) { if _, err := os.Stat(glue); err != nil { t.Fatalf("no wasm_exec_node.js in this toolchain (%s): %v", glue, err) } - run := exec.Command(node, glue, wasmOut) + run := exec.Command(node, glue, wasmOut, "alpha", "beta") out, err := run.CombinedOutput() // A non-zero exit is EXPECTED here, and only for one reason. closed_main_wasm.go // ends in select{} on purpose, to keep the runtime alive for the JS event and @@ -142,4 +149,8 @@ func TestClosedWasmBuildIntegration(t *testing.T) { if !strings.Contains(string(out), "document is: ") { t.Errorf("the JS bridge did not report a value for the absent `document` global; node said:\n%s", out) } + // The module's own arguments reached the embedded program's ARGV. + if want := `argv=["alpha", "beta"]`; !strings.Contains(string(out), want) { + t.Errorf("the wasm module's arguments did not reach ARGV (want %s); node said:\n%s", want, out) + } } From 8b17281f10ae92dbcaf510529c7a8ad12c50074a Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 27 Sep 2026 12:49:03 +0200 Subject: [PATCH 3/6] Report an unreadable script the way ruby does: LoadError, not *os.PathError Measured against MRI 4.0.5, three of fifty invocations still disagreed, all for one reason -- the wording when the script cannot be read: ruby: No such file or directory -- nosuch.rb (LoadError) rbgo: open nosuch.rb: no such file or directory The exit status was already 1 in both, so this was never visible as a failure; only the text was wrong. MRI raises it in open_load_file through rb_load_fail, whose message is " -- ". Go differs from C in two ways that both had to be undone: an *os.PathError wraps the errno with its own operation and path, and Go's errno strings are strerror's text with a lower-case first letter. errnoText strips the wrapper, capitalise puts the letter back, and Unwrap still reaches the *fs.PathError so a caller can ask errors.Is(err, fs.ErrNotExist). Measured after: No such file or directory, Is a directory and Permission denied all match ruby's line byte for byte, and the table is 50/50. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/main.go | 7 +++ cmd/rbgo/options.go | 51 +++++++++++++++++++++- cmd/rbgo/options_test.go | 94 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 151 insertions(+), 1 deletion(-) diff --git a/cmd/rbgo/main.go b/cmd/rbgo/main.go index c3bcc02d..622c1c2e 100644 --- a/cmd/rbgo/main.go +++ b/cmd/rbgo/main.go @@ -22,6 +22,7 @@ package main import ( + "errors" "fmt" "os" @@ -70,6 +71,12 @@ func runCmd(args []string) { } src, name, fromFile, err := o.load() if err != nil { + var le *loadError + if errors.As(err, &le) { + // MRI's own LoadError line for an unreadable script, not Go's + // *os.PathError wording. + fatal("%s", le) + } fatal("rbgo: %v", err) } diff --git a/cmd/rbgo/options.go b/cmd/rbgo/options.go index 3f9485cf..964ec759 100644 --- a/cmd/rbgo/options.go +++ b/cmd/rbgo/options.go @@ -7,8 +7,10 @@ package main import ( + "errors" "fmt" "io" + "io/fs" "os" "strings" ) @@ -287,7 +289,54 @@ func (o *options) load() (src, name string, fromFile bool, err error) { } b, rerr := os.ReadFile(o.script) if rerr != nil { - return "", "", false, rerr + return "", "", false, &loadError{path: o.script, err: rerr} } return string(b), o.script, true, nil } + +// loadError is a script that could not be read. MRI reports this from +// open_load_file through rb_load_fail, which raises LoadError with the message +// " -- "; uncaught, that reaches stderr as +// "ruby: No such file or directory -- nosuch.rb (LoadError)" with exit 1. +// +// Go's syscall errno strings are the same strerror text with a lowercase first +// letter ("no such file or directory"), and an *os.PathError wraps them with its +// own operation and path ("open nosuch.rb: no such file or directory"). Both +// differences are undone here, so the line is MRI's. +type loadError struct { + path string + err error +} + +func (e *loadError) Error() string { + return "rbgo: " + capitalise(errnoText(e.err)) + " -- " + e.path + " (LoadError)" +} + +func (e *loadError) Unwrap() error { return e.err } + +// errnoText strips an *os.PathError's operation and path, leaving the bare +// strerror text MRI names. An error that is not a PathError is used as it is, +// because inventing a message would be worse than an unfamiliar one. +func errnoText(err error) string { + var pe *fs.PathError + if errors.As(err, &pe) { + if pe.Err == nil { + // os.ReadFile never builds one of these, and (*PathError).Error() + // dereferences Err unconditionally, so say what IS known rather than + // panic on a malformed error from somewhere else. + return pe.Op + " " + pe.Path + } + return pe.Err.Error() + } + return err.Error() +} + +// capitalise upper-cases the first BYTE only when it is an ASCII lower-case +// letter. Every errno string Go carries is ASCII, and a multi-byte first rune +// (which cannot occur here) is left untouched rather than mangled. +func capitalise(s string) string { + if s == "" || s[0] < 'a' || s[0] > 'z' { + return s + } + return string(s[0]-'a'+'A') + s[1:] +} diff --git a/cmd/rbgo/options_test.go b/cmd/rbgo/options_test.go index 4a5d00b2..6d2982a5 100644 --- a/cmd/rbgo/options_test.go +++ b/cmd/rbgo/options_test.go @@ -7,7 +7,9 @@ package main import ( + "errors" "io" + "io/fs" "os" "path/filepath" "reflect" @@ -409,3 +411,95 @@ func captureRun(t *testing.T, fn func()) (stdout, stderr string) { os.Stdout, os.Stderr = savedOut, savedErr return <-outCh, <-errCh } + +// TestLoadErrorIsMRIsLoadErrorLine: a script rbgo cannot read must produce the +// line ruby produces. Measured, all four against MRI 4.0.5 with the same argv: +// +// ruby: No such file or directory -- nosuch.rb (LoadError) exit 1 +// ruby: Is a directory -- adir (LoadError) exit 1 +// ruby: Permission denied -- noperm.rb (LoadError) exit 1 +// +// Before this, rbgo printed Go's *os.PathError — "rbgo: open nosuch.rb: no such +// file or directory" — with the same exit status. The status was never the +// defect; the wording was. +// +// A/B: drop the capitalise() call and every row fails, because Go's errno strings +// are strerror's text with a lower-case first letter. +func TestLoadErrorIsMRIsLoadErrorLine(t *testing.T) { + dir := t.TempDir() + sub := filepath.Join(dir, "adir") + if err := os.Mkdir(sub, 0o755); err != nil { + t.Fatal(err) + } + noperm := filepath.Join(dir, "noperm.rb") + if err := os.WriteFile(noperm, []byte("p 1\n"), 0o000); err != nil { + t.Fatal(err) + } + missing := filepath.Join(dir, "nosuch.rb") + + for _, c := range []struct { + name, path, want string + }{ + {"missing", missing, "rbgo: No such file or directory -- " + missing + " (LoadError)"}, + {"a directory", sub, "rbgo: Is a directory -- " + sub + " (LoadError)"}, + {"unreadable", noperm, "rbgo: Permission denied -- " + noperm + " (LoadError)"}, + } { + t.Run(c.name, func(t *testing.T) { + if os.Geteuid() == 0 && c.name == "unreadable" { + t.Skip("root reads a 0000 file, so this row cannot be arranged") + } + o, err := parseOptions([]string{c.path}, io.Discard) + if err != nil { + t.Fatal(err) + } + _, _, _, lerr := o.load() + if lerr == nil { + t.Fatalf("load(%q) returned no error", c.path) + } + var le *loadError + if !errors.As(lerr, &le) { + t.Fatalf("load error is %T, want *loadError", lerr) + } + if got := le.Error(); got != c.want { + t.Errorf("Error() = %q, want %q", got, c.want) + } + // Unwrap must still reach the fs.PathError, so a caller can ask + // errors.Is(err, fs.ErrNotExist). + var pe *fs.PathError + if !errors.As(lerr, &pe) { + t.Errorf("the underlying *fs.PathError is not reachable through Unwrap") + } + }) + } +} + +// TestCapitaliseLeavesNonASCIIAlone pins the guard rather than the happy path: +// every errno string Go carries is ASCII, and a first rune that is not an ASCII +// lower-case letter must come back byte-identical rather than half-encoded. +func TestCapitaliseLeavesNonASCIIAlone(t *testing.T) { + for _, c := range []struct{ in, want string }{ + {"", ""}, + {"no such file", "No such file"}, + {"Already capital", "Already capital"}, + {"é accented", "é accented"}, + {"1 leading digit", "1 leading digit"}, + } { + if got := capitalise(c.in); got != c.want { + t.Errorf("capitalise(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +// TestErrnoTextFallsBackToTheWholeError: an error that is not a *fs.PathError has +// no errno to strip, and inventing a message would be worse than an unfamiliar +// one. +func TestErrnoTextFallsBackToTheWholeError(t *testing.T) { + if got := errnoText(errors.New("something else entirely")); got != "something else entirely" { + t.Errorf("errnoText = %q, want the error's own text", got) + } + // A PathError whose Err is nil has no errno at all, and (*PathError).Error() + // would panic on it. os.ReadFile never builds one; the answer is still defined. + if got := errnoText(&fs.PathError{Op: "open", Path: "x"}); got != "open x" { + t.Errorf("errnoText of a PathError with no Err = %q, want %q", got, "open x") + } +} From bdca37ae3ab11154ee3233692969c8390dc5699f Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 27 Sep 2026 12:50:39 +0200 Subject: [PATCH 4/6] Correct one stale sentence about how rbgo is invoked docs/PRODUCTION-FEASIBILITY.md said "bare `rbgo -e` is not the CLI form". It was, even before this branch: the shorthand arm of main() already accepted `-e code`. The sentence now states the contract the front end implements, and says so about its own previous claim rather than silently deleting it. Co-Authored-By: Claude Opus 5 --- docs/PRODUCTION-FEASIBILITY.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/PRODUCTION-FEASIBILITY.md b/docs/PRODUCTION-FEASIBILITY.md index 355712be..3f36438a 100644 --- a/docs/PRODUCTION-FEASIBILITY.md +++ b/docs/PRODUCTION-FEASIBILITY.md @@ -19,8 +19,11 @@ output. Where a blocker was tractable, it was stubbed/worked around to expose th | Puppet | `puppetlabs/puppet` @ `e227c27` (PUPPETVERSION `8.11.0`) | | Rack | `rack/rack` @ `1e62232` | -Invocation is `rbgo run ` or `rbgo run -e ''` (note the `run` -sub-command; bare `rbgo -e` is not the CLI form). +Invocation is `rbgo [arguments...]` or `rbgo -e '' [arguments...]`, +following `ruby [options] [--] [filepath] [arguments]`; the explicit `run` +sub-command (`rbgo run `) accepts the same command line. The parenthesis +that used to stand here said bare `rbgo -e` was not the CLI form, which was never +true. --- From 5e77e627657fa95a0c2b294316d9a4ef2b8ff772 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 27 Sep 2026 12:53:40 +0200 Subject: [PATCH 5/6] Measure the wasip1 row instead of reasoning about it There is no wasmtime on the machine this was developed on, so the wasip1 line in closed_argv.go said it was reasoned rather than measured. node's node:wasi speaks preview1, which is enough: the OPEN CLI built with GOOS=wasip1 GOARCH=wasm runs under it and the whole command line works -- rbgo.wasip1.wasm wp.rb alpha beta -> [["alpha", "beta"], ".../wp.rb", false] 0 rbgo.wasip1.wasm -W0 wq.rb a b -> [["a", "b"], nil] 0 rbgo.wasip1.wasm -w wq.rb a b -> [["a", "b"], true] 0 rbgo.wasip1.wasm -Z wq.rb -> 1 A wasip1 CLOSED binary stays unmeasured, and for a reason worth stating: `rbgo build`'s only wasm target is GOOS=js, so that combination cannot be produced at all. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/closed_argv.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/rbgo/closed_argv.go b/cmd/rbgo/closed_argv.go index 4f966532..1a798d60 100644 --- a/cmd/rbgo/closed_argv.go +++ b/cmd/rbgo/closed_argv.go @@ -35,10 +35,11 @@ import ( // Verified end to end by TestClosedWasmBuildIntegration. // - js/wasm in a browser: there is no process argv at all; os.Args carries only // "js", so ARGV is empty. Nothing here special-cases that. -// - wasip1: os.Args comes from the WASI args_get call, so a host that passes -// arguments delivers them. This machine has no wasip1 runtime installed, so -// that row is reasoned from the Go runtime's own wasip1 os.Args and is the -// one row below that was NOT measured. +// - wasip1: os.Args comes from the WASI args_get call. Measured with the OPEN +// CLI built for GOOS=wasip1 and run under node's node:wasi (preview1): the +// whole command line works there, ARGV included. A wasip1 CLOSED binary is not +// reachable through `rbgo build`, whose only wasm target is GOOS=js, so that +// exact combination is the one thing here that could not be run. func seedProcessArgs(machine *vm.VM) { if len(os.Args) > 0 { machine.SetScriptName(os.Args[0]) From 841de3c2ae68dfad0f608e55565a461bce795ba3 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Sun, 27 Sep 2026 14:22:15 +0200 Subject: [PATCH 6/6] Name the errno by predicate, not by the operating system's wording The windows CI lane failed three rows of TestLoadErrorIsMRIsLoadErrorLine, and it was right to: want: rbgo: No such file or directory -- ...\nosuch.rb (LoadError) got: rbgo: The system cannot find the file specified. -- ... want: rbgo: Is a directory -- ...\adir (LoadError) got: rbgo: Incorrect function. -- ... load(...\noperm.rb) returned no error The first two are a real defect in the CODE, not in the test. Go surfaces FormatMessage's wording on Windows while ruby.exe still prints strerror(ENOENT), so "The system cannot find the file specified." was never going to be MRI's line. errnoText now recognises the three cases rb_load_fail names through Go's OS-independent predicates -- errors.Is(err, fs.ErrNotExist) / fs.ErrPermission, and a Stat for EISDIR, which has no Windows equivalent Go surfaces -- and falls back to the OS text only for the rest. That fallback is exercised and measured: `ruby t.rb/x` prints "Not a directory -- t.rb/x (LoadError)" and so does rbgo, on a fourth row added for it. The third failure is the test's own precondition: chmod 0000 denies nothing on Windows (and nothing to root on POSIX), so EACCES cannot be arranged. That row now asks whether the file is really unreadable and skips only when it is not, so the skip is a measurement of the host and not an assumption about it. The ENOTDIR row is skipped on Windows and says plainly that the Windows behaviour was not measured rather than guessing at it. Re-measured after: the fifty-row rbgo-versus-ruby table is still 50/50, and the four unreadable-script rows match byte for byte. Co-Authored-By: Claude Opus 5 --- cmd/rbgo/options.go | 46 +++++++++++++++++++++++++++++++++----- cmd/rbgo/options_test.go | 48 ++++++++++++++++++++++++++++++++++------ 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/cmd/rbgo/options.go b/cmd/rbgo/options.go index 964ec759..28ea4935 100644 --- a/cmd/rbgo/options.go +++ b/cmd/rbgo/options.go @@ -309,15 +309,51 @@ type loadError struct { } func (e *loadError) Error() string { - return "rbgo: " + capitalise(errnoText(e.err)) + " -- " + e.path + " (LoadError)" + return "rbgo: " + errnoText(e.err, e.path) + " -- " + e.path + " (LoadError)" } func (e *loadError) Unwrap() error { return e.err } -// errnoText strips an *os.PathError's operation and path, leaving the bare -// strerror text MRI names. An error that is not a PathError is used as it is, -// because inventing a message would be worse than an unfamiliar one. -func errnoText(err error) string { +// errnoText names the errno the way strerror does, which is the text MRI's +// rb_load_fail interpolates. +// +// The three cases MRI names for a script it cannot open are recognised through +// Go's OS-INDEPENDENT predicates, not through the operating system's own message, +// because Windows does not agree with strerror about any of them: Go surfaces +// FormatMessage's wording there ("The system cannot find the file specified.", +// "Incorrect function." for a directory) while ruby.exe still prints +// strerror(ENOENT), i.e. "No such file or directory". Matching the predicate +// rather than the text is what makes the line the same on both platforms. +// +// Anything else falls back to the OS's own text, stripped of the *os.PathError +// wrapper and capitalised, because inventing a message would be worse than an +// unfamiliar one. That path is exercised: ENOTDIR (`ruby t.rb/x`) reaches it and +// MRI's "Not a directory -- t.rb/x" comes out of it unchanged. +func errnoText(err error, path string) string { + switch { + case errors.Is(err, fs.ErrNotExist): + return "No such file or directory" // ENOENT + case errors.Is(err, fs.ErrPermission): + return "Permission denied" // EACCES + case isDirectory(path): + // EISDIR. Asked of the filesystem rather than of the error, because + // syscall.EISDIR does not exist on Windows and the error there carries no + // portable marker for it. + return "Is a directory" + } + return capitalise(bareErrText(err)) +} + +// isDirectory reports whether path is a directory, answering false for anything +// it cannot determine — a name that has since gone, or a stat that is refused. +func isDirectory(path string) bool { + fi, err := os.Stat(path) + return err == nil && fi.IsDir() +} + +// bareErrText strips an *os.PathError's operation and path, leaving the OS's +// message alone. +func bareErrText(err error) string { var pe *fs.PathError if errors.As(err, &pe) { if pe.Err == nil { diff --git a/cmd/rbgo/options_test.go b/cmd/rbgo/options_test.go index 6d2982a5..bb73feda 100644 --- a/cmd/rbgo/options_test.go +++ b/cmd/rbgo/options_test.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "reflect" + "runtime" "strings" "testing" ) @@ -437,16 +438,34 @@ func TestLoadErrorIsMRIsLoadErrorLine(t *testing.T) { } missing := filepath.Join(dir, "nosuch.rb") + notdir := filepath.Join(noperm, "inner.rb") // a path THROUGH a regular file + for _, c := range []struct { name, path, want string }{ {"missing", missing, "rbgo: No such file or directory -- " + missing + " (LoadError)"}, {"a directory", sub, "rbgo: Is a directory -- " + sub + " (LoadError)"}, {"unreadable", noperm, "rbgo: Permission denied -- " + noperm + " (LoadError)"}, + // The fallback arm: no OS-independent predicate names ENOTDIR, so this one + // comes out of the operating system's own text. Measured: + // `ruby t.rb/x` prints "ruby: Not a directory -- t.rb/x (LoadError)". + {"a path through a file", notdir, "rbgo: Not a directory -- " + notdir + " (LoadError)"}, } { t.Run(c.name, func(t *testing.T) { - if os.Geteuid() == 0 && c.name == "unreadable" { - t.Skip("root reads a 0000 file, so this row cannot be arranged") + if c.name == "unreadable" && !readDenied(t, noperm) { + // Windows does not deny reads on a 0000 file (nor does root on + // POSIX), so the PRECONDITION cannot be arranged here. Skipping is + // reported only after checking that the file really is readable, so + // this is a measurement rather than an assumption about the host. + t.Skipf("%s is readable here, so EACCES cannot be arranged", noperm) + } + if c.name == "a path through a file" && runtime.GOOS == "windows" { + // ENOTDIR has no Windows equivalent that Go surfaces with this + // wording, and this row's "want" is MRI's POSIX strerror text. It is + // skipped rather than guessed at: the Windows behaviour was NOT + // measured (no Windows host here), and the CI lane that runs it is + // where such a claim would have to come from. + t.Skip("ENOTDIR's strerror text is POSIX; Windows behaviour not measured") } o, err := parseOptions([]string{c.path}, io.Discard) if err != nil { @@ -492,14 +511,29 @@ func TestCapitaliseLeavesNonASCIIAlone(t *testing.T) { // TestErrnoTextFallsBackToTheWholeError: an error that is not a *fs.PathError has // no errno to strip, and inventing a message would be worse than an unfamiliar -// one. +// one. The path argument names something that does not exist, so isDirectory +// cannot claim the case. func TestErrnoTextFallsBackToTheWholeError(t *testing.T) { - if got := errnoText(errors.New("something else entirely")); got != "something else entirely" { - t.Errorf("errnoText = %q, want the error's own text", got) + absent := filepath.Join(t.TempDir(), "absent") + if got := errnoText(errors.New("something else entirely"), absent); got != "Something else entirely" { + t.Errorf("errnoText = %q, want the error's own text, capitalised", got) } // A PathError whose Err is nil has no errno at all, and (*PathError).Error() // would panic on it. os.ReadFile never builds one; the answer is still defined. - if got := errnoText(&fs.PathError{Op: "open", Path: "x"}); got != "open x" { - t.Errorf("errnoText of a PathError with no Err = %q, want %q", got, "open x") + if got := errnoText(&fs.PathError{Op: "open", Path: "x"}, absent); got != "Open x" { + t.Errorf("errnoText of a PathError with no Err = %q, want %q", got, "Open x") + } +} + +// readDenied reports whether path really cannot be read, which is the precondition +// the EACCES row needs. It is asked rather than assumed: chmod 0000 denies nothing +// to root, and on Windows it denies nothing at all. +func readDenied(t *testing.T, path string) bool { + t.Helper() + f, err := os.Open(path) + if err == nil { + f.Close() + return false } + return errors.Is(err, fs.ErrPermission) }