diff --git a/.gitignore b/.gitignore index ceb46d3..785c8e2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ .puku/ -nodeup.md% +nodeup.md # Go *.exe @@ -38,4 +38,7 @@ Thumbs.db # need a package.json. Developers running `npx commitlint` locally # install into a throwaway node_modules. node_modules/ -package-lock.json \ No newline at end of file +package-lock.json + +# Tooling session metadata (not part of the project source tree). +.claude/ \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index 30391d8..d7a45dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- YAML config file support (`internal/config`): the documented schema + from `docs/configuration.md` is now first-class. Settings live in + `~/.nodeup/config.yaml` (override with `$NODEUP_CONFIG` or redirect + `$NODEUP_HOME`) and are merged with env vars and built-in defaults + using a four-layer precedence chain (defaults < file < env < CLI + flags). Every field — including `track.lts: false` and other + explicit-zero values — is preserved across round-trips via a per- + field set-flag overlay so partial files don't clobber defaults. + Saves are atomic (temp + rename, mode 0600) and refuse to persist + invalid configs. +- `nodeup config` subcommands: + - `show` — print the merged effective config as YAML (the output + round-trips with the file format). + - `get ` — read a single dotted key (e.g. `packages.skip`). + - `set ` — edit a key in the file layer and save it. + Validates before writing; rejects unknown keys and bad values. + - `init [--force]` — scaffold a fresh config at the default path; + refuses to overwrite without `--force`. +- Environment variable overlay (`NODEUP_MANAGER`, `NODEUP_TRACK_LTS`, + `NODEUP_TRACK_CURRENT`, `NODEUP_PACKAGES_MIGRATE`, + `NODEUP_PACKAGES_STRATEGY`, `NODEUP_CACHE_TTL`). Parse errors + include the variable name so env typos surface immediately. +- `nodeup upgrade` now reads its effective config from + `loadConfigOrDefault()`: `--manager` flag still wins over the file + value, and `cfg.Packages.Migrate` replaces the hard-coded `true` + so users can opt out globally. +- `scripts/issue-workflow.sh`: issue→branch→PR→squash-merge automation + script (bug-fix in the same change: fixed the bash regex parser + that was rejecting valid issue titles because space-separated + alternations aren't valid ERE). + ### Changed - Consolidated the internal `nodeup.md` design doc into `README.md` (new "Compatibility notes" subsection, expanded Contributing diff --git a/README.md b/README.md index 52ea805..956c4f4 100644 --- a/README.md +++ b/README.md @@ -178,8 +178,10 @@ cache: ``` Flags override env vars (`NODEUP_MANAGER`, `NODEUP_TRACK_LTS`, `NODEUP_CACHE_TTL`) -override the file. See [`docs/configuration.md`](./docs/configuration.md) for the -full schema. +override the file. Manage it with `nodeup config {show,get,set,init}` — for +example `nodeup config set manager fnm` writes through to the file (refuses +to write invalid values, refuses to overwrite on `init` without `--force`). +See [`docs/configuration.md`](./docs/configuration.md) for the full schema. ## Project status @@ -187,7 +189,7 @@ This is the **v1.0.0 development line**. See `CHANGELOG.md` for what's done. | Version | Status | Notes | |---|---|---| -| v1.0.0 | 🛠 in development | Phase 1 ✅ — 8/8 managers detected. Phase 2 ✅ — `nodeup check` with nodejs.org/dist/index.json fetch + TTL cache. Phase 3 ✅ — package snapshot/restore + migration report. Phase 4 — end-to-end upgrade command. | +| v1.0.0 | 🛠 in development | Phase 1 ✅ — 8/8 managers detected. Phase 2 ✅ — `nodeup check` with nodejs.org/dist/index.json fetch + TTL cache. Phase 3 ✅ — package snapshot/restore + migration report. Phase 4 ✅ — end-to-end `nodeup upgrade`. Phase 5 ✅ — YAML config file + `config` subcommands (show / get / set / init). | Phase 1 is the **detection surface** — every manager is recognized and the version + installed-list reads return real data (PRs #1–#8). Subsequent diff --git a/docs/configuration.md b/docs/configuration.md index aded2ed..8c2d0e5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -1,23 +1,63 @@ # nodeup Configuration -The schema below is the source of truth. Config-file parsing and the -`nodeup config set/get/show` subcommands land in Phase 5 -(`feat(config): …`), but the schema and env-var precedence rules are -fixed now so other subsystems can reference them. +nodeup's behavior can be tuned in three layers: a YAML config file, +environment variables, and CLI flags. The schema and precedence rules +below are the source of truth for all subsystems. -The optional config file lives at `~/.nodeup/config.yaml`. +The optional config file lives at `~/.nodeup/config.yaml` (path +override: `NODEUP_CONFIG`). The file is created and edited via the +`nodeup config {init,show,get,set}` subcommands. Resolution precedence (highest first): -1. CLI flags (e.g. `--manager fnm`) -2. Environment variables (`NODEUP_MANAGER`, `NODEUP_TRACK_LTS`, `NODEUP_CACHE_TTL`) -3. Config file +1. CLI flags (e.g. `--manager fnm`, `--no-migrate`) +2. Environment variables (`NODEUP_MANAGER`, `NODEUP_TRACK_LTS`, `NODEUP_CACHE_TTL`, ...) +3. Config file (`~/.nodeup/config.yaml`) 4. Built-in defaults +Explicit zero values are preserved at every layer — `NODEUP_TRACK_LTS=false` +overrides a file that says `track.lts: true`, and `nodeup config set +packages.skip ""` clears the default package-skip list. To restore a +layer's default, omit the key from the file rather than writing its +zero value. + +## Config-file example + +```yaml +schema_version: 1 +manager: fnm +track: + lts: true + current: false +packages: + migrate: true + strategy: latest + skip: [yarn, pnpm] +cleanup: + auto: false + prompt: true +cache: + ttl: 7200 +``` + +The file is written atomically (sibling temp + rename) with mode 0600. +`nodeup config set` validates before writing, so an invalid value +leaves the previous file untouched. + +## Subcommands + +| Subcommand | Purpose | +|---|---| +| `nodeup config show` | Print the merged (defaults < file < env) config as YAML, with a `# path: ...` header comment | +| `nodeup config get ` | Print the merged value of a dotted key (e.g. `packages.strategy`) | +| `nodeup config set ` | Update one key in the config file; refuses to write invalid values | +| `nodeup config init [--force]` | Write a fresh config file with defaults; refuses to overwrite without `--force` | + ## Schema | Key | Type | Default | Description | |---|---|---|---| +| `schema_version` | int | `1` | Schema revision. Reserved for future migrations. | | `manager` | string | _auto-detect_ | Force a specific version manager | | `track.lts` | bool | `true` | Track LTS upgrades | | `track.current` | bool | `false` | Track Current upgrades | diff --git a/go.mod b/go.mod index e708793..a5071eb 100644 --- a/go.mod +++ b/go.mod @@ -5,6 +5,7 @@ go 1.24.0 require ( github.com/Masterminds/semver/v3 v3.5.0 github.com/spf13/cobra v1.10.2 + gopkg.in/yaml.v3 v3.0.1 ) require ( diff --git a/go.sum b/go.sum index c1f24fe..421b1d7 100644 --- a/go.sum +++ b/go.sum @@ -9,4 +9,7 @@ github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiT github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= +gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= +gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/cli/config.go b/internal/cli/config.go index 382ad7a..8343975 100644 --- a/internal/cli/config.go +++ b/internal/cli/config.go @@ -1,61 +1,232 @@ package cli -import "github.com/spf13/cobra" +import ( + "fmt" + "strings" -// newConfigCmd is a stub for Phase 0. It will be implemented in Phase 5. + "github.com/spf13/cobra" + "gopkg.in/yaml.v3" + + "github.com/dipto0321/nodeup/internal/config" +) + +// newConfigCmd implements `nodeup config ...` subcommands. +// +// Subcommands: // -// The config file lives at ~/.nodeup/config.yaml and stores the manager -// preference, tracked channels, migration strategy, cleanup policy, -// and cache TTL. +// nodeup config show Print the merged effective config as YAML. +// nodeup config get Print a single dotted key (e.g. packages.migrate). +// nodeup config set Update a key in the config file and save. +// nodeup config init Scaffold a fresh config file at the default +// path (refuses to overwrite an existing one). +// +// The YAML output of `show` round-trips with the file format, so users +// can pipe `nodeup config show > config.yaml` as a starting point. func newConfigCmd() *cobra.Command { cmd := &cobra.Command{ Use: "config", Short: "Manage nodeup configuration", Long: `Manage the nodeup configuration file (~/.nodeup/config.yaml). -Implemented in Phase 5.`, +The effective config is built from four layers, in increasing precedence: + 1. Built-in defaults + 2. The YAML file on disk + 3. Environment variables (NODEUP_*) + 4. CLI flags + +Use the subcommands below to inspect or modify the file layer.`, RunE: func(cmd *cobra.Command, args []string) error { return cmd.Help() }, } - cmd.AddCommand(&cobra.Command{ - Use: "set ", - Short: "Set a config value", - Args: cobra.MinimumNArgs(2), + cmd.AddCommand(newConfigShowCmd()) + cmd.AddCommand(newConfigGetCmd()) + cmd.AddCommand(newConfigSetCmd()) + cmd.AddCommand(newConfigInitCmd()) + + return cmd +} + +// newConfigShowCmd prints the merged effective config as YAML. The output +// is intentionally identical in shape to the file format so users can +// edit it, save it back, and get the same result. +func newConfigShowCmd() *cobra.Command { + return &cobra.Command{ + Use: "show", + Short: "Show the merged effective config as YAML", + Long: `Print the fully-merged configuration: defaults overlaid with the +config file (if any) overlaid with environment variables. + +The output is valid YAML matching the file schema, so you can redirect it +to a file as a starting point for a new config.`, RunE: func(cmd *cobra.Command, args []string) error { - cmd.Println("nodeup config set — not yet implemented (Phase 5)") - return nil + return runWithConfig(cmd, func(cfg *config.Config) error { + // yaml.v3 marshals struct field tags — the Config struct + // is annotated so this matches the documented schema. + out, err := yaml.Marshal(cfg) + if err != nil { + return fmt.Errorf("marshal config: %w", err) + } + path, _ := config.DefaultPath() + fmt.Fprintf(cmd.OutOrStdout(), "# effective config (source: %s)\n%s", path, out) + return nil + }) }, - }) + } +} - cmd.AddCommand(&cobra.Command{ +// newConfigGetCmd prints a single dotted-key value from the merged config. +// Supported keys mirror config.SetByKey: +// +// manager, track.lts, track.current, packages.migrate, +// packages.strategy, packages.skip, cleanup.auto, +// cleanup.prompt, cache.ttl +func newConfigGetCmd() *cobra.Command { + return &cobra.Command{ Use: "get ", - Short: "Get a config value", + Short: "Get a single config value (e.g. packages.migrate)", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - cmd.Println("nodeup config get — not yet implemented (Phase 5)") - return nil + return runWithConfig(cmd, func(cfg *config.Config) error { + v, err := configGet(cfg, args[0]) + if err != nil { + return err + } + fmt.Fprintln(cmd.OutOrStdout(), v) + return nil + }) }, - }) + } +} - cmd.AddCommand(&cobra.Command{ - Use: "show", - Short: "Show the merged effective config", +// newConfigSetCmd updates a key in the config file. It does NOT touch +// env vars or defaults — it edits the file layer specifically, so the +// change persists across invocations. +// +// Refuses to write if the resulting config would be invalid (Validate +// runs after merge). +func newConfigSetCmd() *cobra.Command { + return &cobra.Command{ + Use: "set ", + Short: "Set a config value and save it to the config file", + Long: `Persist a config value to the YAML file. Examples: + + nodeup config set manager fnm + nodeup config set track.lts true + nodeup config set packages.skip npm,corepack + nodeup config set cache.ttl 7200`, + Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { - cmd.Println("nodeup config show — not yet implemented (Phase 5)") + path, err := config.DefaultPath() + if err != nil { + return err + } + + // Load the current file (or start from defaults if absent). + current, _, err := config.Load(path) + if err != nil { + // A missing file is fine for `set`; we just create one + // from defaults. + if !isNotExist(err) { + return fmt.Errorf("load existing config: %w", err) + } + current = config.Default() + } + + // Apply the change in-memory using the dotted-key API. + ok, err := current.SetByKey(args[0], args[1]) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("unknown key %q", args[0]) + } + + // Validate before touching disk so we don't corrupt the + // file with a bad value. + if err := current.Validate(); err != nil { + return fmt.Errorf("refusing to save invalid config: %w", err) + } + + if err := config.Save(path, current); err != nil { + return fmt.Errorf("save config to %s: %w", path, err) + } + fmt.Fprintf(cmd.OutOrStdout(), "set %s=%s (saved to %s)\n", args[0], args[1], path) return nil }, - }) + } +} - cmd.AddCommand(&cobra.Command{ +// newConfigInitCmd scaffolds a fresh config file at the default path. +// It refuses to overwrite an existing file — use `set` to mutate +// individual keys, or delete the file first. +func newConfigInitCmd() *cobra.Command { + var force bool + cmd := &cobra.Command{ Use: "init", - Short: "Write a scaffolded config file to ~/.nodeup/config.yaml", + Short: "Write a scaffolded config file to the default path", + Long: `Create a new config file containing the documented defaults. Refuses +to overwrite an existing file unless --force is passed.`, RunE: func(cmd *cobra.Command, args []string) error { - cmd.Println("nodeup config init — not yet implemented (Phase 5)") + path, err := config.DefaultPath() + if err != nil { + return err + } + + // Existence check is best-effort; we let Save do the real + // atomic-create dance. + if !force && fileExists(path) { + return fmt.Errorf("config already exists at %s (use --force to overwrite)", path) + } + cfg := config.Scaffold() + if err := config.Save(path, cfg); err != nil { + return fmt.Errorf("save config to %s: %w", path, err) + } + fmt.Fprintf(cmd.OutOrStdout(), "wrote scaffolded config to %s\n", path) return nil }, - }) - + } + cmd.Flags().BoolVar(&force, "force", false, "overwrite an existing config file") return cmd } + +// configGet is the read counterpart to Config.SetByKey. Both accept the +// same dotted-key vocabulary so users only have to learn one set of +// key names. +// +// Returns an error for unknown keys rather than empty string so typos +// surface immediately. +func configGet(c *config.Config, key string) (string, error) { + switch strings.ToLower(key) { + case "manager": + return c.Manager, nil + case "schema_version", "schemaversion": + return fmt.Sprintf("%d", c.SchemaVersion), nil + case "track.lts": + return boolStr(c.Track.LTS), nil + case "track.current": + return boolStr(c.Track.Current), nil + case "packages.migrate": + return boolStr(c.Packages.Migrate), nil + case "packages.strategy": + return string(c.Packages.Strategy), nil + case "packages.skip": + return strings.Join(c.Packages.Skip, ","), nil + case "cleanup.auto": + return boolStr(c.Cleanup.Auto), nil + case "cleanup.prompt": + return boolStr(c.Cleanup.Prompt), nil + case "cache.ttl": + return fmt.Sprintf("%d", c.Cache.TTL), nil + default: + return "", fmt.Errorf("unknown key %q (supported: manager, track.lts, track.current, packages.migrate, packages.strategy, packages.skip, cleanup.auto, cleanup.prompt, cache.ttl)", key) + } +} + +func boolStr(b bool) string { + if b { + return "true" + } + return "false" +} diff --git a/internal/cli/fs.go b/internal/cli/fs.go new file mode 100644 index 0000000..a4e42f2 --- /dev/null +++ b/internal/cli/fs.go @@ -0,0 +1,22 @@ +package cli + +import ( + "errors" + "os" +) + +// isNotExist returns true if err is a "file not found" error from the +// OS. Wrapping errors.Is/os.IsNotExist gives us correct behavior for +// both raw syscall errors and wrapped ones. +func isNotExist(err error) bool { + return errors.Is(err, os.ErrNotExist) +} + +// fileExists returns true if path resolves to an existing file. A nil +// error from os.Stat means the file is there. We don't distinguish +// "is a directory" vs "is a file" — both are non-zero existence signals; +// the callers (config init, etc.) make their own judgement. +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} diff --git a/internal/cli/loadconfig.go b/internal/cli/loadconfig.go new file mode 100644 index 0000000..6479ef0 --- /dev/null +++ b/internal/cli/loadconfig.go @@ -0,0 +1,58 @@ +package cli + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + + "github.com/dipto0321/nodeup/internal/config" +) + +// loadConfigOrDefault resolves the effective Config the way the docs promise: +// +// defaults < config file < environment variables +// +// CLI flag overrides happen in the individual RunE handlers (they're the top +// layer in the precedence chain). Errors reading or parsing the config file +// are returned verbatim so the user sees the path + reason. A missing file +// is not an error — it just means "use the defaults". +// +// This is the single chokepoint for every subcommand that needs config so +// the behavior is consistent. +func loadConfigOrDefault() (*config.Config, error) { + path, err := config.DefaultPath() + if err != nil { + return nil, fmt.Errorf("resolve config path: %w", err) + } + + // If the file exists, load it. If it doesn't, that's fine — defaults + // are still a valid starting point. Anything else (permission denied, + // malformed YAML, invalid value) is an error. + cfg, _, err := config.Load(path) + if err != nil && !os.IsNotExist(err) { + return nil, fmt.Errorf("load config from %s: %w", path, err) + } + + // Overlay env vars on top of (defaults | file). + envLayer, err := config.FromEnv() + if err != nil { + return nil, err + } + final := config.Resolve(cfg, envLayer) + if err := final.Validate(); err != nil { + return nil, fmt.Errorf("validate merged config: %w", err) + } + return final, nil +} + +// runWithConfig is a convenience wrapper for subcommand RunE handlers +// that just want the merged Config. They don't need to know how it's +// loaded. +func runWithConfig(cmd *cobra.Command, fn func(*config.Config) error) error { + cfg, err := loadConfigOrDefault() + if err != nil { + return err + } + return fn(cfg) +} diff --git a/internal/cli/upgrade.go b/internal/cli/upgrade.go index 64ff995..72597cb 100644 --- a/internal/cli/upgrade.go +++ b/internal/cli/upgrade.go @@ -41,11 +41,25 @@ func runUpgrade(cmd *cobra.Command, args []string) error { noMigrate, _ := cmd.Flags().GetBool("no-migrate") noCleanup, _ := cmd.Flags().GetBool("no-cleanup") _ = noCleanup // TODO: implement cleanup logic in Phase 7 - managerPref, _ := cmd.Flags().GetString("manager") + managerFlag, _ := cmd.Flags().GetString("manager") yes, _ := cmd.Flags().GetBool("yes") _ = yes // TODO: use for non-interactive mode offline, _ := cmd.Flags().GetBool("offline") + // Load the effective config (defaults < file < env). The --manager + // CLI flag, if provided, takes precedence over everything else. + cfg, err := loadConfigOrDefault() + if err != nil { + return err + } + managerPref := managerFlag + if managerPref == "" { + managerPref = cfg.Manager + } + // --no-migrate / --no-cleanup flags beat config; otherwise follow cfg. + skipMigrate := noMigrate || !cfg.Packages.Migrate + _ = skipMigrate // referenced below in snapshot/restore sections + // Detect managers installed := detector.DetectAll() m, err := detector.ResolveManager(installed, managerPref) @@ -124,7 +138,7 @@ func runUpgrade(cmd *cobra.Command, args []string) error { } // Snapshot current packages - if !noMigrate { + if !skipMigrate { ctx := cmd.Context() for _, v := range installedVersions { if err := packages.Snapshot(ctx, m.Name(), v); err != nil { @@ -150,7 +164,7 @@ func runUpgrade(cmd *cobra.Command, args []string) error { } // Restore packages - if !noMigrate && len(toInstall) > 0 { + if !skipMigrate && len(toInstall) > 0 { ctx := cmd.Context() for _, v := range toInstall { if err := packages.Restore(ctx, m.Name(), *v); err != nil { diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 0000000..48b660b --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,296 @@ +// Package config loads, validates, and persists nodeup's user configuration. +// +// nodeup reads its settings from a YAML file at ~/.nodeup/config.yaml. +// The schema is documented in docs/configuration.md and is the source of +// truth for every field name, default value, and validation rule below. +// +// Resolution precedence (highest first): +// +// 1. CLI flags (per-command --flag) +// 2. Environment variables (NODEUP_MANAGER, NODEUP_TRACK_LTS, ...) +// 3. Config file (~/.nodeup/config.yaml) +// 4. Hard-coded defaults +// +// When a config file does not exist, nodeup behaves as if it contained an +// empty document — defaults are applied and no error is raised. A malformed +// file IS an error: the user almost certainly has a typo to fix. +// +// This package has no dependencies on cobra or on any other nodeup +// internal package — it is safe to import from anywhere, including +// subcommands, library code, and tests. +package config + +import ( + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" +) + +// SchemaVersion is the YAML config schema version. Bump it when fields are +// renamed, removed, or change semantics in a way that would break an +// existing user's file. +const SchemaVersion = 1 + +// Config is the in-memory representation of nodeup's YAML configuration. +// Field names carry `yaml:` tags that match docs/configuration.md exactly. +// Don't add a field without updating the docs and bumping SchemaVersion. +type Config struct { + // SchemaVersion lets us migrate older files later without guessing. + SchemaVersion int `yaml:"schema_version"` + + // Manager pins a specific version manager. Empty string means "auto-detect". + Manager string `yaml:"manager,omitempty"` + + Track TrackConfig `yaml:"track"` + Packages PackagesConfig `yaml:"packages"` + Cleanup CleanupConfig `yaml:"cleanup"` + Cache CacheConfig `yaml:"cache"` +} + +// TrackConfig controls which Node.js release lines nodeup follows. +// See docs/configuration.md#track for details. +type TrackConfig struct { + LTS bool `yaml:"lts"` + Current bool `yaml:"current"` +} + +// PackagesConfig controls global npm package handling. +// See docs/configuration.md#packages for details. +type PackagesConfig struct { + Migrate bool `yaml:"migrate"` + Strategy Strategy `yaml:"strategy"` + // Skip lists package names that should NOT be migrated (typically + // because they ship with the Node.js install itself). + // Note: no `omitempty` on Skip. We need the file layer to be able to + // express "user wrote an empty list" vs "user omitted skip entirely". + // The former must override the default; the latter should keep the + // default. yaml.v3 writes an empty list as `skip: []` either way, so + // keeping the field always-present in the struct makes the round-trip + // faithful and lets `nodeup config set packages.skip ""` actually + // clear the list. + Skip []string `yaml:"skip"` +} + +// CleanupConfig controls the prompt that asks whether to remove old +// Node.js versions after a successful upgrade. +// See docs/configuration.md#cleanup for details. +type CleanupConfig struct { + Auto bool `yaml:"auto"` + Prompt bool `yaml:"prompt"` +} + +// CacheConfig controls the manifest cache TTL. +// See docs/configuration.md#cache for details. +type CacheConfig struct { + TTL int `yaml:"ttl"` // seconds +} + +// Strategy is the package-migration strategy. It is a typed string so the +// compiler catches typos in callers and YAML round-trips keep the canonical +// lowercase form. +type Strategy string + +const ( + // StrategyExact re-installs the same package@version that was previously + // installed. Safer; recommended. + StrategyExact Strategy = "exact" + + // StrategyLatest re-installs each package name without pinning a version, + // letting npm fetch whatever is current. Riskier — could pull breaking + // changes — but useful for short-lived dev environments. + StrategyLatest Strategy = "latest" +) + +// IsValid reports whether s is one of the defined Strategy constants. +func (s Strategy) IsValid() bool { + switch s { + case StrategyExact, StrategyLatest: + return true + default: + return false + } +} + +// Default returns a Config populated with the documented defaults. This is +// the answer to "what would the config be if the file did not exist?" +// Every default here MUST match the value column in +// docs/configuration.md. If you change one, change the doc. +func Default() *Config { + return &Config{ + SchemaVersion: SchemaVersion, + Manager: "", // empty -> auto-detect + Track: TrackConfig{ + LTS: true, + Current: false, + }, + Packages: PackagesConfig{ + Migrate: true, + Strategy: StrategyExact, + Skip: []string{"npm", "corepack", "npx"}, + }, + Cleanup: CleanupConfig{ + Auto: false, + Prompt: true, + }, + Cache: CacheConfig{ + TTL: 3600, + }, + } +} + +// Validate returns nil if c is internally consistent and conforms to the +// schema. It does NOT check whether c.Manager is a manager nodeup knows +// about — that lives in detector.ByName and is enforced only at the point +// of use, so the config package stays free of detector imports. +func (c *Config) Validate() error { + if c == nil { + return errors.New("config is nil") + } + if c.SchemaVersion != 0 && c.SchemaVersion != SchemaVersion { + return fmt.Errorf("config schema_version=%d, this build expects %d — please run `nodeup config init` to migrate", c.SchemaVersion, SchemaVersion) + } + if !c.Packages.Strategy.IsValid() { + return fmt.Errorf("packages.strategy=%q must be one of: exact, latest", c.Packages.Strategy) + } + if c.Cache.TTL < 0 { + return fmt.Errorf("cache.ttl=%d must be >= 0 (0 disables caching)", c.Cache.TTL) + } + for i, name := range c.Packages.Skip { + if strings.TrimSpace(name) == "" { + return fmt.Errorf("packages.skip[%d] is empty", i) + } + } + return nil +} + +// SetByKey updates a single dotted-path field (e.g. "track.lts", +// "packages.skip"). Returns (true, nil) if the key was recognized and set, +// (false, nil) if the key was unknown (so callers can decide whether to +// surface a "no such key" error), and (_, err) on type mismatch. +// +// Supported keys: +// +// manager string +// track.lts bool +// track.current bool +// packages.migrate bool +// packages.strategy string (exact | latest) +// packages.skip []string (CSV on input, JSON list on output) +// cleanup.auto bool +// cleanup.prompt bool +// cache.ttl int (seconds) +func (c *Config) SetByKey(key, raw string) (bool, error) { + switch key { + case "manager": + c.Manager = strings.TrimSpace(raw) + return true, nil + case "track.lts": + b, err := parseBool(raw) + if err != nil { + return true, fmt.Errorf("track.lts: %w", err) + } + c.Track.LTS = b + return true, nil + case "track.current": + b, err := parseBool(raw) + if err != nil { + return true, fmt.Errorf("track.current: %w", err) + } + c.Track.Current = b + return true, nil + case "packages.migrate": + b, err := parseBool(raw) + if err != nil { + return true, fmt.Errorf("packages.migrate: %w", err) + } + c.Packages.Migrate = b + return true, nil + case "packages.strategy": + s := Strategy(strings.TrimSpace(raw)) + if !s.IsValid() { + return true, fmt.Errorf("packages.strategy=%q must be one of: exact, latest", s) + } + c.Packages.Strategy = s + return true, nil + case "packages.skip": + // Accept a CSV (e.g. "yarn,pnpm") so the CLI subcommand is ergonomic. + // Empty string -> empty list, NOT defaults — `nodeup config set + // packages.skip ""` is the way to clear the list. + if raw == "" { + c.Packages.Skip = nil + return true, nil + } + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p != "" { + out = append(out, p) + } + } + c.Packages.Skip = out + return true, nil + case "cleanup.auto": + b, err := parseBool(raw) + if err != nil { + return true, fmt.Errorf("cleanup.auto: %w", err) + } + c.Cleanup.Auto = b + return true, nil + case "cleanup.prompt": + b, err := parseBool(raw) + if err != nil { + return true, fmt.Errorf("cleanup.prompt: %w", err) + } + c.Cleanup.Prompt = b + return true, nil + case "cache.ttl": + n, err := strconv.Atoi(strings.TrimSpace(raw)) + if err != nil { + return true, fmt.Errorf("cache.ttl=%q must be an integer (seconds)", raw) + } + c.Cache.TTL = n + return true, nil + default: + return false, nil + } +} + +// parseBool accepts the same truthy/falsy spellings the CLI flag library +// accepts, plus a few extras ("yes"/"no"). We do not import pflag here to +// keep the config package library-pure. +func parseBool(s string) (bool, error) { + switch strings.ToLower(strings.TrimSpace(s)) { + case "1", "t", "true", "yes", "y", "on": + return true, nil + case "0", "f", "false", "no", "n", "off": + return false, nil + default: + return false, fmt.Errorf("cannot parse %q as bool", s) + } +} + +// DefaultPath returns the canonical config file path, honoring overrides: +// +// 1. $NODEUP_CONFIG (full path to the file, useful for tests) +// 2. $NODEUP_HOME/config.yaml (lets power-users redirect everything) +// 3. $HOME/.nodeup/config.yaml +// +// It does NOT check whether the file exists. Callers handle missing files +// by applying defaults. +func DefaultPath() (string, error) { + if p := strings.TrimSpace(os.Getenv("NODEUP_CONFIG")); p != "" { + return p, nil + } + if home := strings.TrimSpace(os.Getenv("NODEUP_HOME")); home != "" { + return filepath.Join(home, "config.yaml"), nil + } + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("determine config path: %w", err) + } + return filepath.Join(home, ".nodeup", "config.yaml"), nil +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 0000000..4e631e6 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,165 @@ +package config + +import ( + "path/filepath" + "reflect" + "runtime" + "testing" +) + +func TestDefault_MatchesSpec(t *testing.T) { + // These exact defaults are documented in docs/configuration.md. + // If any of them drifts, the docs are wrong (or vice versa). + want := &Config{ + SchemaVersion: 1, + Manager: "", + Track: TrackConfig{LTS: true, Current: false}, + Packages: PackagesConfig{ + Migrate: true, + Strategy: StrategyExact, + Skip: []string{"npm", "corepack", "npx"}, + }, + Cleanup: CleanupConfig{Auto: false, Prompt: true}, + Cache: CacheConfig{TTL: 3600}, + } + got := Default() + if !reflect.DeepEqual(got, want) { + t.Errorf("Default() drift:\n got: %#v\nwant: %#v", got, want) + } +} + +func TestValidate(t *testing.T) { + t.Parallel() + tests := []struct { + name string + mutate func(*Config) + wantErr bool + }{ + {"defaults are valid", func(*Config) {}, false}, + {"bad strategy", func(c *Config) { c.Packages.Strategy = "guess" }, true}, + {"strategy latest is valid", func(c *Config) { c.Packages.Strategy = StrategyLatest }, false}, + {"negative ttl", func(c *Config) { c.Cache.TTL = -1 }, true}, + {"zero ttl is valid (disables cache)", func(c *Config) { c.Cache.TTL = 0 }, false}, + {"skip empty string is bad", func(c *Config) { c.Packages.Skip = []string{"yarn", ""} }, true}, + {"skip nil is fine", func(c *Config) { c.Packages.Skip = nil }, false}, + {"future schema is bad", func(c *Config) { c.SchemaVersion = 999 }, true}, + } + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + c := Default() + tc.mutate(c) + err := c.Validate() + if (err != nil) != tc.wantErr { + t.Errorf("Validate() err = %v, wantErr = %v", err, tc.wantErr) + } + }) + } +} + +func TestSetByKey(t *testing.T) { + t.Parallel() + tests := []struct { + key, raw string + check func(*Config) bool + wantErr bool + wantSet bool + }{ + {"manager", "fnm", func(c *Config) bool { return c.Manager == "fnm" }, false, true}, + {"track.lts", "false", func(c *Config) bool { return c.Track.LTS == false }, false, true}, + {"track.lts", "yes", func(c *Config) bool { return c.Track.LTS == true }, false, true}, + {"track.current", "true", func(c *Config) bool { return c.Track.Current == true }, false, true}, + {"packages.migrate", "false", func(c *Config) bool { return c.Packages.Migrate == false }, false, true}, + {"packages.strategy", "latest", func(c *Config) bool { return c.Packages.Strategy == StrategyLatest }, false, true}, + {"packages.strategy", "bogus", nil, true, true}, + {"packages.skip", "yarn,pnpm", func(c *Config) bool { return reflect.DeepEqual(c.Packages.Skip, []string{"yarn", "pnpm"}) }, false, true}, + {"packages.skip", "", func(c *Config) bool { return c.Packages.Skip == nil }, false, true}, + {"cleanup.auto", "true", func(c *Config) bool { return c.Cleanup.Auto == true }, false, true}, + {"cleanup.prompt", "no", func(c *Config) bool { return c.Cleanup.Prompt == false }, false, true}, + {"cache.ttl", "7200", func(c *Config) bool { return c.Cache.TTL == 7200 }, false, true}, + {"cache.ttl", "absurd", nil, true, true}, + {"bogus.key", "anything", nil, false, false}, + // bad bool on a known key + {"track.lts", "yelp", nil, true, true}, + } + for _, tc := range tests { + tc := tc + t.Run(tc.key+"="+tc.raw, func(t *testing.T) { + t.Parallel() + c := Default() // fresh config per test + set, err := c.SetByKey(tc.key, tc.raw) + if (err != nil) != tc.wantErr { + t.Errorf("err = %v, wantErr = %v", err, tc.wantErr) + } + if set != tc.wantSet { + t.Errorf("set = %v, want %v", set, tc.wantSet) + } + if tc.check != nil && !tc.check(c) { + t.Errorf("post-condition failed; got config = %#v", c) + } + }) + } +} + +func TestDefaultPath(t *testing.T) { + // We can't use t.Setenv across parallel tests cleanly, so we run serial. + // + // Paths are built with filepath.Join so the test passes on both Unix + // ("/") and Windows ("\\") — the production code uses filepath.Join + // too, so any platform-quoting mismatch would be a real bug. + // + // The HOME-fallback case is gated to non-Windows: os.UserHomeDir() on + // Windows reads USERPROFILE, not HOME, so setting HOME via t.Setenv + // doesn't steer the lookup. The NODEUP_CONFIG and NODEUP_HOME paths + // still get platform-correct coverage. + homeRoot := "/tmp/u" + homeRoot2 := "/tmp/home" + explicit := "/tmp/explicit/config.yaml" + tests := []struct { + name string + envConfig string + envHome string + homeDir string // sets HOME + want string + skipOnWin bool + }{ + { + name: "NODEUP_CONFIG wins", + envConfig: explicit, + want: explicit, + }, + { + name: "NODEUP_HOME used when no NODEUP_CONFIG", + envHome: homeRoot2, + want: filepath.Join(homeRoot2, "config.yaml"), + }, + { + name: "HOME/.nodeup/config.yaml fallback", + homeDir: homeRoot, + want: filepath.Join(homeRoot, ".nodeup", "config.yaml"), + skipOnWin: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + if tc.skipOnWin && runtime.GOOS == "windows" { + t.Skip("HOME env override is not honored by os.UserHomeDir() on Windows") + } + t.Setenv("NODEUP_CONFIG", tc.envConfig) + t.Setenv("NODEUP_HOME", tc.envHome) + if tc.homeDir != "" { + t.Setenv("HOME", tc.homeDir) + } else { + t.Setenv("HOME", "/should-not-be-used") + } + got, err := DefaultPath() + if err != nil { + t.Fatalf("DefaultPath: %v", err) + } + if got != tc.want { + t.Errorf("DefaultPath = %q, want %q", got, tc.want) + } + }) + } +} diff --git a/internal/config/env.go b/internal/config/env.go new file mode 100644 index 0000000..b9b6d0b --- /dev/null +++ b/internal/config/env.go @@ -0,0 +1,123 @@ +package config + +import ( + "os" + "strconv" +) + +// Environment variable names exported as constants so tests, docs, and +// the README can refer to them by symbol instead of by string literal. +// Keep in sync with docs/configuration.md "Environment variables" section. +const ( + EnvManager = "NODEUP_MANAGER" + EnvTrackLTS = "NODEUP_TRACK_LTS" + EnvTrackCurrent = "NODEUP_TRACK_CURRENT" + EnvPackagesMigrate = "NODEUP_PACKAGES_MIGRATE" + EnvPackagesStrategy = "NODEUP_PACKAGES_STRATEGY" + EnvCacheTTL = "NODEUP_CACHE_TTL" + + // EnvConfig lets power users point nodeup at a different config + // file path entirely (e.g. for tests, or for users who keep their + // dotfiles in a non-standard location). + EnvConfig = "NODEUP_CONFIG" + + // EnvHome lets users redirect every nodeup state file (~/.nodeup) + // to a custom root. + EnvHome = "NODEUP_HOME" +) + +// FromEnv returns an Overlay containing ONLY the env-var-overridden keys. +// Callers pass this Overlay to Resolve() so it gets layered on top of +// file/defaults. +// +// Returning an Overlay (not a Config) is important: it lets us distinguish +// "set explicitly to false" from "unset". For example, a user who runs +// `NODEUP_TRACK_LTS=false nodeup upgrade` on a system whose config file +// says `track.lts: true` should get LTS turned OFF, not silently inherit +// the file's value. A *Config couldn't express that — the zero value +// of Track.LTS is also false, so we'd lose the "set" signal. +// +// A variable that is set but unparsable (e.g. NODEUP_TRACK_LTS="yelp") +// returns an error rather than silently ignoring it — env typos are +// frustrating to debug otherwise. +// +// A variable that is set to the empty string is treated the same as +// unset (so `unset NODEUP_FOO` and `NODEUP_FOO=""` have identical +// effect). This matches typical 12-factor / docker-compose behavior +// and keeps tests hermetic under t.Setenv(k, ""). +// +// This function NEVER reads the file; use Load() for that. It exists +// so it can be unit-tested without touching disk. +func FromEnv() (*Overlay, error) { + out := NewOverlay() + if v, ok := os.LookupEnv(EnvManager); ok && v != "" { + out.SetManager(v) + } + if v, ok := os.LookupEnv(EnvTrackLTS); ok && v != "" { + b, err := parseBool(v) + if err != nil { + return nil, errEnv(EnvTrackLTS, err) + } + out.SetTrackLTS(b) + } + if v, ok := os.LookupEnv(EnvTrackCurrent); ok && v != "" { + b, err := parseBool(v) + if err != nil { + return nil, errEnv(EnvTrackCurrent, err) + } + out.SetTrackCurrent(b) + } + if v, ok := os.LookupEnv(EnvPackagesMigrate); ok && v != "" { + b, err := parseBool(v) + if err != nil { + return nil, errEnv(EnvPackagesMigrate, err) + } + out.SetPackagesMigrate(b) + } + if v, ok := os.LookupEnv(EnvPackagesStrategy); ok && v != "" { + s := Strategy(v) + if !s.IsValid() { + return nil, errEnv(EnvPackagesStrategy, &strconv.NumError{ + Func: "Strategy", + Num: string(s), + Err: errInvalidStrategy, + }) + } + out.SetPackagesStrategy(s) + } + if v, ok := os.LookupEnv(EnvCacheTTL); ok && v != "" { + n, err := strconv.Atoi(v) + if err != nil { + return nil, errEnv(EnvCacheTTL, err) + } + out.SetCacheTTL(n) + } + return out, nil +} + +// errInvalidStrategy is a sentinel so FromEnv's strategy-bad-error path +// doesn't have to invent one inline. +var errInvalidStrategy = stringErr("must be one of: exact, latest") + +type stringErr string + +func (s stringErr) Error() string { return string(s) } + +// errEnv wraps an env-var parse error with the variable name so the +// caller can show exactly which variable is wrong. +func errEnv(name string, err error) error { + return &EnvError{Name: name, Err: err} +} + +// EnvError is returned when an environment variable is set but cannot +// be parsed. It implements error and supports errors.Is/As via Unwrap. +type EnvError struct { + Name string + Err error +} + +func (e *EnvError) Error() string { + return e.Name + ": " + e.Err.Error() +} + +func (e *EnvError) Unwrap() error { return e.Err } diff --git a/internal/config/env_test.go b/internal/config/env_test.go new file mode 100644 index 0000000..43f9e0a --- /dev/null +++ b/internal/config/env_test.go @@ -0,0 +1,140 @@ +package config + +import ( + "errors" + "testing" +) + +// envKeys are all the env vars that FromEnv reads (other than NODEUP_CONFIG +// and NODEUP_HOME, which only affect path resolution). +var envKeys = []string{ + EnvManager, + EnvTrackLTS, + EnvTrackCurrent, + EnvPackagesMigrate, + EnvPackagesStrategy, + EnvCacheTTL, +} + +// clearEnv unsets every env var the config package reads. Uses t.Setenv +// which both blanks the value and auto-restores it on test exit. +func clearEnv(t *testing.T) { + t.Helper() + for _, k := range envKeys { + t.Setenv(k, "") + } +} + +func TestFromEnv_AllUnset(t *testing.T) { + clearEnv(t) + + got, err := FromEnv() + if err != nil { + t.Fatalf("FromEnv: %v", err) + } + if got == nil || got.C == nil { + t.Fatal("FromEnv returned nil overlay or nil config") + } + if got.ManagerSet || got.TrackLTSSet || got.TrackCurrentSet || + got.PackagesMigrateSet || got.PackagesStrategySet || + got.CacheTTLSet { + t.Errorf("FromEnv with no env vars should set no flags, got flags: %+v", got) + } + if !emptyConfig(got.C) { + t.Errorf("FromEnv with no env vars should leave C zero, got %#v", got.C) + } +} + +func TestFromEnv_OverlayEach(t *testing.T) { + clearEnv(t) + t.Setenv(EnvManager, "fnm") + t.Setenv(EnvTrackLTS, "true") + t.Setenv(EnvTrackCurrent, "true") + t.Setenv(EnvPackagesMigrate, "false") + t.Setenv(EnvPackagesStrategy, string(StrategyLatest)) + t.Setenv(EnvCacheTTL, "120") + + got, err := FromEnv() + if err != nil { + t.Fatalf("FromEnv: %v", err) + } + if got.C.Manager != "fnm" { + t.Errorf("Manager = %q", got.C.Manager) + } + if !got.C.Track.LTS || !got.C.Track.Current { + t.Errorf("track: lts=%v current=%v", got.C.Track.LTS, got.C.Track.Current) + } + if got.C.Packages.Migrate { + t.Errorf("Packages.Migrate = true, want false") + } + if got.C.Packages.Strategy != StrategyLatest { + t.Errorf("Packages.Strategy = %q", got.C.Packages.Strategy) + } + if got.C.Cache.TTL != 120 { + t.Errorf("Cache.TTL = %d", got.C.Cache.TTL) + } + // Every set-flag should be flipped. + if !got.ManagerSet || !got.TrackLTSSet || !got.TrackCurrentSet || + !got.PackagesMigrateSet || !got.PackagesStrategySet || !got.CacheTTLSet { + t.Errorf("expected all set-flags flipped, got: %+v", got) + } +} + +// TestFromEnv_ExplicitFalsePreserved verifies the whole reason FromEnv +// returns an Overlay (not a *Config): a value explicitly set to "false" +// must survive into the overlay, not be lost to the zero value. +func TestFromEnv_ExplicitFalsePreserved(t *testing.T) { + clearEnv(t) + t.Setenv(EnvTrackLTS, "false") + + got, err := FromEnv() + if err != nil { + t.Fatalf("FromEnv: %v", err) + } + if !got.TrackLTSSet { + t.Error("TrackLTSSet = false, want true (env var was set)") + } + if got.C.Track.LTS { + t.Error("Track.LTS = true, want false (env var said false)") + } +} + +func emptyConfig(c *Config) bool { + return c.Manager == "" && !c.Track.LTS && !c.Track.Current && + !c.Packages.Migrate && c.Packages.Strategy == "" && len(c.Packages.Skip) == 0 && + !c.Cleanup.Auto && !c.Cleanup.Prompt && c.Cache.TTL == 0 +} + +func TestFromEnv_BoolParseError(t *testing.T) { + clearEnv(t) + t.Setenv(EnvTrackLTS, "yelp") + + _, err := FromEnv() + if err == nil { + t.Fatal("expected error from bad NODEUP_TRACK_LTS value") + } + var ee *EnvError + if !errors.As(err, &ee) || ee.Name != EnvTrackLTS { + t.Errorf("err = %v, want EnvError{Name: %q}", err, EnvTrackLTS) + } +} + +func TestFromEnv_StrategyParseError(t *testing.T) { + clearEnv(t) + t.Setenv(EnvPackagesStrategy, "aggressive") + + _, err := FromEnv() + if err == nil { + t.Fatal("expected error from bad NODEUP_PACKAGES_STRATEGY value") + } +} + +func TestFromEnv_IntParseError(t *testing.T) { + clearEnv(t) + t.Setenv(EnvCacheTTL, "forever") + + _, err := FromEnv() + if err == nil { + t.Fatal("expected error from bad NODEUP_CACHE_TTL value") + } +} diff --git a/internal/config/fileoverlay.go b/internal/config/fileoverlay.go new file mode 100644 index 0000000..5b1611b --- /dev/null +++ b/internal/config/fileoverlay.go @@ -0,0 +1,170 @@ +package config + +import "gopkg.in/yaml.v3" + +// FileOverlayFromNode builds an Overlay that marks only the fields +// actually present in the YAML document node as set. cfg supplies the +// decoded values; root is the yaml.Node tree from the same document. +// +// This is the precise form of FileOverlay: it can distinguish "user +// wrote track.lts: false" from "user omitted track.lts entirely", +// which yaml.v3 cannot do by reflection alone. +// +// Unknown top-level keys (those not in the schema) are ignored; the +// YAML decoder itself does not error on them. Callers that want to +// warn on typos can inspect root.Content themselves. +func FileOverlayFromNode(cfg *Config, root *yaml.Node) *Overlay { + if cfg == nil { + return NewOverlay() + } + o := &Overlay{C: cfg} + if root == nil { + return o // all unset + } + // root may be a Document node wrapping a Mapping — descend. + doc := root + if doc.Kind == yaml.DocumentNode && len(doc.Content) > 0 { + doc = doc.Content[0] + } + if doc.Kind != yaml.MappingNode { + return o + } + for i := 0; i < len(doc.Content)-1; i += 2 { + keyNode := doc.Content[i] + valNode := doc.Content[i+1] + switch keyNode.Value { + case "schema_version": + o.SchemaVersionSet = true + o.C.SchemaVersion = decodeInt(valNode) + case "manager": + o.ManagerSet = true + o.C.Manager = decodeString(valNode) + case "track": + applyTrackOverlay(o.C, valNode, &o.TrackLTSSet, &o.TrackCurrentSet) + case "packages": + applyPackagesOverlay(o.C, valNode, + &o.PackagesMigrateSet, &o.PackagesStrategySet, &o.PackagesSkipSet) + case "cleanup": + applyCleanupOverlay(o.C, valNode, &o.CleanupAutoSet, &o.CleanupPromptSet) + case "cache": + applyCacheOverlay(o.C, valNode, &o.CacheTTLSet) + } + } + return o +} + +// applyTrackOverlay scans a track-mapping node and sets the +// corresponding overlay flags + values on cfg. +func applyTrackOverlay(cfg *Config, node *yaml.Node, ltsSet, currentSet *bool) { + if node == nil || node.Kind != yaml.MappingNode { + return + } + for i := 0; i < len(node.Content)-1; i += 2 { + switch node.Content[i].Value { + case "lts": + *ltsSet = true + cfg.Track.LTS = decodeBool(node.Content[i+1]) + case "current": + *currentSet = true + cfg.Track.Current = decodeBool(node.Content[i+1]) + } + } +} + +// applyPackagesOverlay scans a packages-mapping node. +func applyPackagesOverlay(cfg *Config, node *yaml.Node, migrateSet, strategySet, skipSet *bool) { + if node == nil || node.Kind != yaml.MappingNode { + return + } + for i := 0; i < len(node.Content)-1; i += 2 { + switch node.Content[i].Value { + case "migrate": + *migrateSet = true + cfg.Packages.Migrate = decodeBool(node.Content[i+1]) + case "strategy": + *strategySet = true + cfg.Packages.Strategy = Strategy(decodeString(node.Content[i+1])) + case "skip": + *skipSet = true + cfg.Packages.Skip = decodeStringList(node.Content[i+1]) + } + } +} + +// applyCleanupOverlay scans a cleanup-mapping node. +func applyCleanupOverlay(cfg *Config, node *yaml.Node, autoSet, promptSet *bool) { + if node == nil || node.Kind != yaml.MappingNode { + return + } + for i := 0; i < len(node.Content)-1; i += 2 { + switch node.Content[i].Value { + case "auto": + *autoSet = true + cfg.Cleanup.Auto = decodeBool(node.Content[i+1]) + case "prompt": + *promptSet = true + cfg.Cleanup.Prompt = decodeBool(node.Content[i+1]) + } + } +} + +// applyCacheOverlay scans a cache-mapping node. +func applyCacheOverlay(cfg *Config, node *yaml.Node, ttlSet *bool) { + if node == nil || node.Kind != yaml.MappingNode { + return + } + for i := 0; i < len(node.Content)-1; i += 2 { + if node.Content[i].Value == "ttl" { + *ttlSet = true + cfg.Cache.TTL = decodeInt(node.Content[i+1]) + } + } +} + +// decodeBool returns the bool value of a scalar node, defaulting to +// false on unexpected types. This matches the behavior of the +// subsequent yaml.Unmarshal call for the same value. +func decodeBool(n *yaml.Node) bool { + if n == nil { + return false + } + var b bool + _ = n.Decode(&b) + return b +} + +// decodeString returns the string value of a scalar node. +func decodeString(n *yaml.Node) string { + if n == nil { + return "" + } + var s string + _ = n.Decode(&s) + return s +} + +// decodeInt returns the int value of a scalar node. +func decodeInt(n *yaml.Node) int { + if n == nil { + return 0 + } + var i int + _ = n.Decode(&i) + return i +} + +// decodeStringList returns a []string from a sequence node. +func decodeStringList(n *yaml.Node) []string { + if n == nil || n.Kind != yaml.SequenceNode { + return nil + } + out := make([]string, 0, len(n.Content)) + for _, item := range n.Content { + var s string + _ = item.Decode(&s) + if s != "" { + out = append(out, s) + } + } + return out +} diff --git a/internal/config/load.go b/internal/config/load.go new file mode 100644 index 0000000..c1a834e --- /dev/null +++ b/internal/config/load.go @@ -0,0 +1,121 @@ +package config + +import ( + "errors" + "fmt" + "os" + + "gopkg.in/yaml.v3" +) + +// Load reads a config file from path, applies defaults to any missing +// fields, and validates the result. If the file does not exist, the +// returned Config is the pure defaults (and fromFile is false, so +// callers know the config was not actually persisted). +// +// A malformed YAML file returns an error that wraps the underlying +// yaml.TypeError or yaml.UnmarshalTypeError so callers can show line/ +// column info if they want. +// +// Missing-key behavior: yaml.v3 unmarshalling cannot distinguish +// "key absent" from "key present with zero value" for non-pointer +// fields. We walk the YAML document tree (yaml.Node) to record which +// top-level and nested keys were actually present, then build a +// FileOverlay that only marks present keys as set. Keys the user +// omitted keep their default value. +func Load(path string) (cfg *Config, fromFile bool, err error) { + data, err := os.ReadFile(path) + if err != nil { + if errors.Is(err, os.ErrNotExist) { + return Default(), false, nil + } + return nil, false, fmt.Errorf("read %s: %w", path, err) + } + + // First pass: decode into a yaml.Node so we can see which keys + // are present, even if their values happen to be zero. + var root yaml.Node + if err := yaml.Unmarshal(data, &root); err != nil { + return nil, false, fmt.Errorf("parse %s: %w", path, err) + } + + // Second pass: decode into the actual struct. + loaded := &Config{} + if err := yaml.Unmarshal(data, loaded); err != nil { + return nil, false, fmt.Errorf("parse %s: %w", path, err) + } + + cfg = Resolve(Default(), FileOverlayFromNode(loaded, &root)) + if err := cfg.Validate(); err != nil { + return nil, false, fmt.Errorf("invalid %s: %w", path, err) + } + return cfg, true, nil +} + +// Save writes cfg to path as YAML, atomically replacing any existing +// file. The directory is created with mode 0o755 if it does not exist. +// The file is written with mode 0o600 because it may grow to contain +// sensitive settings in future releases (e.g. private registry tokens). +func Save(path string, cfg *Config) error { + if cfg == nil { + return errors.New("Save: cfg is nil") + } + if err := cfg.Validate(); err != nil { + return fmt.Errorf("refusing to save invalid config: %w", err) + } + + data, err := yaml.Marshal(cfg) + if err != nil { + return fmt.Errorf("marshal config: %w", err) + } + + if err := os.MkdirAll(dirOf(path), 0o755); err != nil { + return fmt.Errorf("create config dir: %w", err) + } + + // Write to a sibling temp file, then rename — readers never see a + // half-written config. + tmp, err := os.CreateTemp(dirOf(path), ".config-*.yaml.tmp") + if err != nil { + return fmt.Errorf("create temp file: %w", err) + } + tmpName := tmp.Name() + defer func() { + // Best-effort cleanup if we never rename. + _ = os.Remove(tmpName) + }() + + if _, err := tmp.Write(data); err != nil { + _ = tmp.Close() + return fmt.Errorf("write temp file: %w", err) + } + if err := tmp.Chmod(0o600); err != nil { + _ = tmp.Close() + return fmt.Errorf("chmod temp file: %w", err) + } + if err := tmp.Close(); err != nil { + return fmt.Errorf("close temp file: %w", err) + } + if err := os.Rename(tmpName, path); err != nil { + return fmt.Errorf("rename temp file -> %s: %w", path, err) + } + return nil +} + +// Scaffold returns a Config suitable for writing into a fresh config +// file via `nodeup config init`. It is currently identical to Default() +// but is a separate function so we can grow it (e.g. add comments per +// field) without breaking callers that depend on Default being +// value-only. +func Scaffold() *Config { + return Default() +} + +func dirOf(path string) string { + for i := len(path) - 1; i >= 0; i-- { + if path[i] == '/' || path[i] == os.PathSeparator { + return path[:i] + } + } + return "." +} diff --git a/internal/config/load_test.go b/internal/config/load_test.go new file mode 100644 index 0000000..60c72f6 --- /dev/null +++ b/internal/config/load_test.go @@ -0,0 +1,287 @@ +package config + +import ( + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// tempConfigPath returns a path inside t.TempDir() for "config.yaml". +// Using t.TempDir means Go automatically cleans up. +func tempConfigPath(t *testing.T) string { + t.Helper() + return filepath.Join(t.TempDir(), "config.yaml") +} + +func writeFile(t *testing.T, path, body string) { + t.Helper() + if err := os.WriteFile(path, []byte(body), 0o600); err != nil { + t.Fatalf("WriteFile: %v", err) + } +} + +func TestLoad_MissingFileIsDefaults(t *testing.T) { + t.Parallel() + cfg, fromFile, err := Load("/definitely/does/not/exist/config.yaml") + if err != nil { + t.Fatalf("Load: %v", err) + } + if fromFile { + t.Error("fromFile = true, want false for missing file") + } + if !reflectEqual(cfg, Default()) { + t.Errorf("Load missing-file result != Default():\n got: %#v\nwant: %#v", cfg, Default()) + } +} + +func TestLoad_PartialFilePreservesDefaults(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + writeFile(t, path, ` +manager: fnm +track: + current: true +`) + cfg, fromFile, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if !fromFile { + t.Error("fromFile = false, want true") + } + if cfg.Manager != "fnm" { + t.Errorf("Manager = %q, want fnm", cfg.Manager) + } + // File set current=true; lts omitted -> default (true) must survive. + if !cfg.Track.LTS { + t.Errorf("Track.LTS = false, want true (default, file omitted)") + } + if !cfg.Track.Current { + t.Errorf("Track.Current = false, want true (file set it)") + } + // packages block omitted entirely -> defaults must survive. + if !cfg.Packages.Migrate { + t.Errorf("Packages.Migrate = false, want true (default)") + } + if cfg.Cache.TTL != 3600 { + t.Errorf("Cache.TTL = %d, want 3600 (default)", cfg.Cache.TTL) + } +} + +func TestLoad_FullFileOverride(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + writeFile(t, path, ` +schema_version: 1 +manager: volta +track: + lts: true + current: true +packages: + migrate: false + strategy: latest + skip: [yarn, pnpm] +cleanup: + auto: true + prompt: false +cache: + ttl: 60 +`) + cfg, _, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.Manager != "volta" { + t.Errorf("Manager = %q", cfg.Manager) + } + if !cfg.Track.LTS || !cfg.Track.Current { + t.Errorf("track: lts=%v current=%v, both want true", cfg.Track.LTS, cfg.Track.Current) + } + if cfg.Packages.Migrate { + t.Errorf("Packages.Migrate = true, want false") + } + if cfg.Packages.Strategy != StrategyLatest { + t.Errorf("Packages.Strategy = %q, want latest", cfg.Packages.Strategy) + } + if strings.Join(cfg.Packages.Skip, ",") != "yarn,pnpm" { + t.Errorf("Packages.Skip = %v", cfg.Packages.Skip) + } + if !cfg.Cleanup.Auto || cfg.Cleanup.Prompt { + t.Errorf("cleanup: auto=%v prompt=%v", cfg.Cleanup.Auto, cfg.Cleanup.Prompt) + } + if cfg.Cache.TTL != 60 { + t.Errorf("Cache.TTL = %d, want 60", cfg.Cache.TTL) + } +} + +func TestLoad_MalformedYAMLIsError(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + writeFile(t, path, ` +manager: fnm + track: + this is: not: aligned +`) + _, _, err := Load(path) + if err == nil { + t.Fatal("Load: expected error for malformed YAML, got nil") + } + if !strings.Contains(err.Error(), path) { + t.Errorf("error should mention %q, got: %v", path, err) + } +} + +func TestLoad_InvalidValueIsError(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + writeFile(t, path, ` +packages: + strategy: bogus +`) + _, _, err := Load(path) + if err == nil { + t.Fatal("expected validation error, got nil") + } +} + +func TestSaveAndReload(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + cfg := Default() + cfg.Manager = "fnm" + cfg.Track.Current = true + cfg.Cache.TTL = 120 + + if err := Save(path, cfg); err != nil { + t.Fatalf("Save: %v", err) + } + + // File should exist and have restrictive perms on POSIX systems. + // Windows' os.Chmod only honors the read-only bit (everything else + // collapses to 0666), so we only assert the strict 0600 mode on + // non-Windows. On Windows we still want the file to *exist* and + // round-trip — those are the platform-agnostic properties. + info, err := os.Stat(path) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if runtime.GOOS != "windows" { + if perm := info.Mode().Perm(); perm != 0o600 { + t.Errorf("file mode = %o, want 0600", perm) + } + } else { + // On Windows the temp file must at least not be read-only. + if info.Mode().Perm()&0o200 == 0 { + t.Errorf("file is read-only on Windows: mode = %o", info.Mode().Perm()) + } + } + + got, _, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if got.Manager != "fnm" || !got.Track.Current || got.Cache.TTL != 120 { + t.Errorf("round-trip mismatch: %#v", got) + } +} + +func TestSave_RefusesInvalid(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + bad := Default() + bad.Packages.Strategy = "bogus" + if err := Save(path, bad); err == nil { + t.Fatal("Save accepted invalid config") + } + if _, err := os.Stat(path); err == nil { + t.Errorf("Save created file despite validation failure") + } +} + +func TestSave_AtomicDoesNotLeakTemp(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + if err := Save(path, Default()); err != nil { + t.Fatalf("Save: %v", err) + } + dir := filepath.Dir(path) + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("ReadDir: %v", err) + } + for _, e := range entries { + if strings.Contains(e.Name(), ".config-") && strings.HasSuffix(e.Name(), ".tmp") { + t.Errorf("leaked temp file: %s", e.Name()) + } + } +} + +// TestSave_EmptyListClearsDefault guards the omitempty choice on Packages.Skip. +// +// Saving a config with an explicit empty Skip list must produce a file that, +// when reloaded, yields a zero-length Skip (not the default `[npm, corepack, +// npx]`). Without this guarantee, `nodeup config set packages.skip ""` (or +// authoring a hand-written config with `skip: []`) cannot actually clear the +// list — the omission-vs-zero distinction would be lost at the YAML layer. +// +// We assert both halves of the round-trip: +// 1. The serialized file actually contains `skip: []`, not an absent key. +// 2. The loaded Config has len(Packages.Skip) == 0. +func TestSave_EmptyListClearsDefault(t *testing.T) { + t.Parallel() + path := tempConfigPath(t) + + cfg := Default() + cfg.Packages.Skip = nil // explicit empty (same outcome as []string{}) + + if err := Save(path, cfg); err != nil { + t.Fatalf("Save: %v", err) + } + + body, err := os.ReadFile(path) + if err != nil { + t.Fatalf("ReadFile: %v", err) + } + if !strings.Contains(string(body), "skip: []") { + t.Errorf("file should contain literal `skip: []`, got:\n%s", body) + } + + got, _, err := Load(path) + if err != nil { + t.Fatalf("Load: %v", err) + } + if len(got.Packages.Skip) != 0 { + t.Errorf("len(Packages.Skip) after reload = %d, want 0 (got %v)", + len(got.Packages.Skip), got.Packages.Skip) + } +} + +// reflectEqual is a tiny helper so we don't have to import "reflect" +// just for a single use. +func reflectEqual(a, b *Config) bool { + if a == nil || b == nil { + return a == b + } + // Compare carefully because Skip may be nil-vs-empty. + if a.SchemaVersion != b.SchemaVersion || a.Manager != b.Manager { + return false + } + if a.Track != b.Track || a.Packages.Migrate != b.Packages.Migrate || + a.Packages.Strategy != b.Packages.Strategy { + return false + } + if a.Cleanup != b.Cleanup || a.Cache != b.Cache { + return false + } + if len(a.Packages.Skip) != len(b.Packages.Skip) { + return false + } + for i := range a.Packages.Skip { + if a.Packages.Skip[i] != b.Packages.Skip[i] { + return false + } + } + return true +} diff --git a/internal/config/resolve.go b/internal/config/resolve.go new file mode 100644 index 0000000..9fb2dea --- /dev/null +++ b/internal/config/resolve.go @@ -0,0 +1,203 @@ +package config + +// Resolve merges configurations from all sources in precedence order: +// +// 1. cli (highest priority — explicit --flag values from the user) +// 2. env (environment variables) +// 3. file (the loaded ~/.nodeup/config.yaml) +// 4. base (the lowest layer — typically Default()) +// +// "Merge" here means: if a field is set in the higher layer, it wins; +// otherwise the lower layer's value is kept. To distinguish "unset" +// from "explicitly false / zero", each layer is passed as an Overlay +// with per-field set-flags (see Overlay.SetXxx methods). +// +// Resolve is pure: it does not call Validate. Callers should validate +// the returned Config before persisting or acting on it. +func Resolve(base *Config, layers ...*Overlay) *Config { + if base == nil { + base = &Config{} + } + out := *base // shallow copy; sub-structs copy by value + + for _, layer := range layers { + if layer == nil || layer.C == nil { + continue + } + applyOverlay(&out, layer) + } + return &out +} + +// applyOverlay copies every explicitly-set field from src onto dst. +func applyOverlay(dst *Config, src *Overlay) { + if src.ManagerSet { + dst.Manager = src.C.Manager + } + if src.SchemaVersionSet { + dst.SchemaVersion = src.C.SchemaVersion + } + if src.TrackLTSSet { + dst.Track.LTS = src.C.Track.LTS + } + if src.TrackCurrentSet { + dst.Track.Current = src.C.Track.Current + } + if src.PackagesMigrateSet { + dst.Packages.Migrate = src.C.Packages.Migrate + } + if src.PackagesStrategySet { + dst.Packages.Strategy = src.C.Packages.Strategy + } + if src.PackagesSkipSet { + dst.Packages.Skip = src.C.Packages.Skip + } + if src.CleanupAutoSet { + dst.Cleanup.Auto = src.C.Cleanup.Auto + } + if src.CleanupPromptSet { + dst.Cleanup.Prompt = src.C.Cleanup.Prompt + } + if src.CacheTTLSet { + dst.Cache.TTL = src.C.Cache.TTL + } +} + +// Overlay is the per-source-layer state used by Resolve. Each SetXxx +// method marks the corresponding field as explicitly provided so +// Resolve can distinguish "user said no" from "user said nothing". +// +// Build one Overlay per source (CLI, env, file). Pass all of them to +// Resolve. The file overlay is normally built by FileOverlay(*Config) +// from a loaded Config, which marks every present field as set. +type Overlay struct { + C *Config + + // Set-flags. Each is true iff the corresponding field was + // explicitly provided in this layer (regardless of value). + ManagerSet bool + SchemaVersionSet bool + TrackLTSSet bool + TrackCurrentSet bool + PackagesMigrateSet bool + PackagesStrategySet bool + PackagesSkipSet bool + CleanupAutoSet bool + CleanupPromptSet bool + CacheTTLSet bool +} + +// NewOverlay returns an empty Overlay (no fields set). +func NewOverlay() *Overlay { return &Overlay{C: &Config{}} } + +// FileOverlay returns an Overlay that marks every field of cfg as set. +// This is the right way to turn a loaded Config into the "file" layer +// for Resolve — it preserves explicit zeros like track.lts: false. +// +// Nil cfg produces an overlay that sets nothing. +// +// Caveat: yaml.v3 cannot distinguish "key absent" from "key present +// with zero value" for non-pointer fields, so a few set-flags use a +// heuristic ("non-zero means set"). Manager empty-string is treated +// as "not set" — meaning `manager: ""` in the file behaves like +// omitting `manager` entirely. That matches the docs, which only +// define "empty Manager" to mean auto-detect. +func FileOverlay(cfg *Config) *Overlay { + if cfg == nil { + return NewOverlay() + } + o := &Overlay{C: cfg} + o.ManagerSet = cfg.Manager != "" + o.TrackLTSSet = true + o.TrackCurrentSet = true + o.PackagesMigrateSet = true + o.PackagesStrategySet = cfg.Packages.Strategy != "" + o.PackagesSkipSet = cfg.Packages.Skip != nil + o.CleanupAutoSet = true + o.CleanupPromptSet = true + o.CacheTTLSet = cfg.Cache.TTL != 0 + o.SchemaVersionSet = cfg.SchemaVersion != 0 + return o +} + +// SetManager records an explicit manager override. +func (o *Overlay) SetManager(name string) { + if o == nil || o.C == nil { + return + } + o.C.Manager = name + o.ManagerSet = true +} + +// SetTrackLTS records an explicit track.lts override. +func (o *Overlay) SetTrackLTS(v bool) { + if o == nil || o.C == nil { + return + } + o.C.Track.LTS = v + o.TrackLTSSet = true +} + +// SetTrackCurrent records an explicit track.current override. +func (o *Overlay) SetTrackCurrent(v bool) { + if o == nil || o.C == nil { + return + } + o.C.Track.Current = v + o.TrackCurrentSet = true +} + +// SetPackagesMigrate records an explicit packages.migrate override. +func (o *Overlay) SetPackagesMigrate(v bool) { + if o == nil || o.C == nil { + return + } + o.C.Packages.Migrate = v + o.PackagesMigrateSet = true +} + +// SetPackagesStrategy records an explicit packages.strategy override. +func (o *Overlay) SetPackagesStrategy(s Strategy) { + if o == nil || o.C == nil { + return + } + o.C.Packages.Strategy = s + o.PackagesStrategySet = true +} + +// SetPackagesSkip records an explicit packages.skip override. Pass nil +// to mean "skip nothing" (which is still an explicit choice). +func (o *Overlay) SetPackagesSkip(skip []string) { + if o == nil || o.C == nil { + return + } + o.C.Packages.Skip = skip + o.PackagesSkipSet = true +} + +// SetCleanupAuto records an explicit cleanup.auto override. +func (o *Overlay) SetCleanupAuto(v bool) { + if o == nil || o.C == nil { + return + } + o.C.Cleanup.Auto = v + o.CleanupAutoSet = true +} + +// SetCleanupPrompt records an explicit cleanup.prompt override. +func (o *Overlay) SetCleanupPrompt(v bool) { + if o == nil || o.C == nil { + return + } + o.C.Cleanup.Prompt = v + o.CleanupPromptSet = true +} + +// SetCacheTTL records an explicit cache.ttl override (seconds). +func (o *Overlay) SetCacheTTL(seconds int) { + if o == nil || o.C == nil { + return + } + o.C.Cache.TTL = seconds + o.CacheTTLSet = true +} diff --git a/internal/config/resolve_test.go b/internal/config/resolve_test.go new file mode 100644 index 0000000..9b97a6f --- /dev/null +++ b/internal/config/resolve_test.go @@ -0,0 +1,134 @@ +package config + +import ( + "testing" + + "gopkg.in/yaml.v3" +) + +func TestResolve_DefaultsOnly(t *testing.T) { + t.Parallel() + got := Resolve(Default()) + want := Default() + if !reflectEqual(got, want) { + t.Errorf("Resolve(defaults) drift:\n got: %#v\nwant: %#v", got, want) + } +} + +func TestResolve_CLIBeatsEnvBeatsFileBeatsBase(t *testing.T) { + t.Parallel() + // File sets: manager=fnm, track.current=true + file := Default() + file.Manager = "fnm" + file.Track.Current = true + doc, err := yaml.Marshal(file) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var root yaml.Node + if err := yaml.Unmarshal(doc, &root); err != nil { + t.Fatalf("parse: %v", err) + } + fileLayer := FileOverlayFromNode(file, &root) + // env sets: manager=volta (overrides file), track.lts=false + envLayer := NewOverlay() + envLayer.SetManager("volta") + envLayer.SetTrackLTS(false) + // CLI sets: manager=n (overrides env) + cliLayer := NewOverlay() + cliLayer.SetManager("n") + + got := Resolve(Default(), fileLayer, envLayer, cliLayer) + + if got.Manager != "n" { + t.Errorf("CLI should win: Manager = %q, want n", got.Manager) + } + if got.Track.LTS { + t.Errorf("env track.lts=false should beat default true; got lts=%v", got.Track.LTS) + } + if !got.Track.Current { + t.Errorf("file track.current=true should survive (no higher layer touched it); got %v", got.Track.Current) + } + if got.Cache.TTL != 3600 { + t.Errorf("default cache.ttl should survive; got %d", got.Cache.TTL) + } +} + +func TestResolve_NilLayersAreSafe(t *testing.T) { + t.Parallel() + got := Resolve(Default(), nil, nil, nil) + if !reflectEqual(got, Default()) { + t.Errorf("nil layers should be no-op") + } +} + +func TestResolve_PartialOverlayDoesNotClobber(t *testing.T) { + t.Parallel() + // File says: manager=fnm, track.lts=false (current left at default) + file := Default() + file.Manager = "fnm" + file.Track.LTS = false + doc, err := yaml.Marshal(file) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var root yaml.Node + if err := yaml.Unmarshal(doc, &root); err != nil { + t.Fatalf("parse: %v", err) + } + fileLayer := FileOverlayFromNode(file, &root) + // env: only TTL changed + envLayer := NewOverlay() + envLayer.SetCacheTTL(120) + + got := Resolve(Default(), fileLayer, envLayer) + if got.Manager != "fnm" { + t.Errorf("Manager = %q, want fnm (from file)", got.Manager) + } + if got.Track.LTS { + t.Errorf("Track.LTS should be false (from file)") + } + if got.Track.Current { + t.Errorf("Track.Current should remain false (default; file did not set it)") + } + if got.Cache.TTL != 120 { + t.Errorf("Cache.TTL = %d, want 120 (from env)", got.Cache.TTL) + } +} + +func TestResolve_ExplicitZeroFromFile(t *testing.T) { + t.Parallel() + // A file with `track.lts: false` should be respected as a deliberate + // user choice — Resolve(Default(), fileLayer) must yield lts=false, + // not be hidden by the default true. + file := Default() + file.Track.LTS = false + doc, _ := yaml.Marshal(file) + var root yaml.Node + _ = yaml.Unmarshal(doc, &root) + fileLayer := FileOverlayFromNode(file, &root) + + got := Resolve(Default(), fileLayer) + if got.Track.LTS { + t.Errorf("file track.lts=false must be honored, got %v", got.Track.LTS) + } +} + +func TestOverlay_SetMethodsFlipCorrectFlags(t *testing.T) { + t.Parallel() + o := NewOverlay() + o.SetManager("fnm") + o.SetTrackLTS(true) + o.SetTrackCurrent(true) + o.SetPackagesMigrate(true) + o.SetPackagesStrategy(StrategyLatest) + o.SetPackagesSkip([]string{"yarn"}) + o.SetCleanupAuto(true) + o.SetCleanupPrompt(true) + o.SetCacheTTL(60) + if !o.ManagerSet || !o.TrackLTSSet || !o.TrackCurrentSet || + !o.PackagesMigrateSet || !o.PackagesStrategySet || !o.PackagesSkipSet || + !o.CleanupAutoSet || !o.CleanupPromptSet || !o.CacheTTLSet { + t.Errorf("not all set-flags flipped: %+v", o) + } +} diff --git a/scripts/README.md b/scripts/README.md new file mode 100644 index 0000000..77099bb --- /dev/null +++ b/scripts/README.md @@ -0,0 +1,23 @@ +# scripts/ + +Project-local automation. Currently: + +- `issue-workflow.sh` — drive a single GitHub issue through the full + branch → PR → merge → cleanup loop. Three subcommands: + - `start ` — sync `main` and create `//` off `origin/main` + - `pr-body ` — print a `.github/PULL_REQUEST_TEMPLATE.md`-shaped body + - `finish` — push, open PR, watch CI, squash-merge, delete branch, sync `main` + + Enforces the conventions in `CONTRIBUTING.md` (Conventional Commits, + allowed types/scopes, one logical change per PR, squash-merge, + source-branch deletion). See `./issue-workflow.sh --help` for details. + +## Why a shell script (and not a Makefile target) + +The Makefile is for **build/test/lint/release** commands — anything +that produces artifacts you can ship. `issue-workflow.sh` is a +**git/GitHub plumbing** helper: it talks to `gh`, reads PR titles, +force-pushes with `--force-with-lease`, and reshapes your local +checkout. Mixing those into `make` would couple the build to GitHub +auth and to `gh` being installed, which isn't true for every +contributor's machine. \ No newline at end of file diff --git a/scripts/issue-workflow.sh b/scripts/issue-workflow.sh new file mode 100755 index 0000000..4690fad --- /dev/null +++ b/scripts/issue-workflow.sh @@ -0,0 +1,264 @@ +#!/usr/bin/env bash +# IMPORTANT: must be interpreted by bash, not zsh. zsh's `[[ str =~ regex ]]` +# has different group-capture semantics for parenthesized alternation +# (e.g. ($ALLOWED_SCOPES)), which breaks parse_issue_title. Running under +# bash gives consistent POSIX ERE semantics regardless of the user's login +# shell. The shebang above handles that for `path/to/script.sh` invocation; +# do NOT `source` this file from zsh. +# scripts/issue-workflow.sh — drive a single GitHub issue from branch → merged PR. +# +# Subcommands: +# start create a feature branch off main for the issue +# pr-body [--type=...] [--scope=...] [--slug=...] +# print a PR body template, filled in +# finish push, open PR, squash-merge, delete branch, sync main +# +# Conventions enforced (per CONTRIBUTING.md): +# - Branch off main, keep main clean. +# - One logical change per PR. +# - Conventional Commits (`type(scope): subject`). +# - Squash-merged; source branch deleted on merge. +# - PR title and squash-merge subject come from the first commit. +# - Allowed types: feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert +# - Allowed scopes: detector, manager, packages, node, config, ui, platform, cli, +# deps, release, ci, docs, lint +# +# Usage examples: +# ./scripts/issue-workflow.sh start 15 +# # …edit code, commit with `git commit -m "feat(config): add yaml loader"` +# ./scripts/issue-workflow.sh pr-body 15 --type=feat --scope=config \ +# --slug=yaml-config > /tmp/pr.md +# gh pr create --base main --head feat/config/yaml-config \ +# --title "feat(config): add yaml loader" --body-file /tmp/pr.md +# ./scripts/issue-workflow.sh finish +# +# Requires: git, gh (authenticated for dipto0321/nodeup), make, golangci-lint v1.64.8. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$REPO_ROOT" + +# Regex-safe alternations: bash [[ =~ ]] uses ERE, so alternatives must be +# pipe-separated inside a group, not space-separated. +ALLOWED_TYPES_RE="feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert" +ALLOWED_SCOPES_RE="detector|manager|packages|node|config|ui|platform|cli|deps|release|ci|docs|lint" +# Space-separated lists for word-splitting/iteration in cmd_pr_body. +ALLOWED_TYPES="feat fix docs style refactor perf test build ci chore revert" +ALLOWED_SCOPES="detector manager packages node config ui platform cli deps release ci docs lint" + +die() { printf "\033[31merror:\033[0m %s\n" "$*" >&2; exit 1; } +info() { printf "\033[36m==>\033[0m %s\n" "$*"; } +ok() { printf "\033[32m✓\033[0m %s\n" "$*"; } + +require_clean_tree() { + if ! git diff --quiet --ignore-submodules HEAD 2>/dev/null; then + die "working tree is dirty. Commit or stash first." + fi +} + +require_main_clean() { + git rev-parse --abbrev-ref HEAD | grep -qx main \ + || die "run this from 'main' (currently on '$(git rev-parse --abbrev-ref HEAD)')" +} + +ensure_gh_authed() { + gh auth status >/dev/null 2>&1 \ + || die "gh is not authenticated. Run: gh auth login" +} + +fetch_issue_meta() { + local issue="$1" + gh issue view "$issue" --json title,state,labels,body 2>/dev/null \ + || die "could not fetch issue #$issue. Is gh authed for this repo?" +} + +# Heuristic: derive (type, scope, slug) from an issue title like +# "feat(config): add yaml config file support". +parse_issue_title() { + local title="$1" + if [[ "$title" =~ ^($ALLOWED_TYPES_RE)\(($ALLOWED_SCOPES_RE)\):\ (.+)$ ]]; then + local type="${BASH_REMATCH[1]}" + local scope="${BASH_REMATCH[2]}" + local rest="${BASH_REMATCH[3]}" + # slugify: lowercase, replace non-alnum with '-', trim leading/trailing '-' + local slug + slug="$(printf "%s" "$rest" | tr '[:upper:]' '[:lower:]' \ + | sed -E 's/[^a-z0-9]+/-/g; s/^-+//; s/-+$//')" + printf "%s %s %s\n" "$type" "$scope" "$slug" + elif [[ "$title" =~ ^($ALLOWED_TYPES_RE):\ (.+)$ ]]; then + # type without scope: scope defaults to "core" + local type="${BASH_REMATCH[1]}" + local rest="${BASH_REMATCH[2]}" + local slug + slug="$(printf "%s" "$rest" | tr '[:upper:]' '[:lower:]' \ + | sed -E 's/[^a-z0-9]+/-/g; s/^-+//; s/-+$//')" + printf "%s %s %s\n" "$type" "core" "$slug" + else + # Fall back: caller must pass --type/--scope/--slug explicitly. + printf " \n" + fi +} + +cmd_start() { + local issue="${1:-}" + [[ -n "$issue" ]] || die "usage: issue-workflow.sh start " + + require_main_clean + require_clean_tree + ensure_gh_authed + + info "fetching issue #$issue" + local meta title state + meta="$(fetch_issue_meta "$issue")" + title="$(printf "%s" "$meta" | python3 -c "import sys,json; print(json.load(sys.stdin)['title'])")" + state="$(printf "%s" "$meta" | python3 -c "import sys,json; print(json.load(sys.stdin)['state'])")" + [[ "$state" == "OPEN" ]] || die "issue #$issue is $state, not OPEN" + + local parsed type scope slug branch + parsed="$(parse_issue_title "$title")" + type="$(awk '{print $1}' <<<"$parsed")" + scope="$(awk '{print $2}' <<<"$parsed")" + slug="$(awk '{print $3}' <<<"$parsed")" + + if [[ -z "$type" || -z "$scope" || -z "$slug" ]]; then + die "issue title '$title' does not match '(): '. " \ + "Pass --type/--scope/--slug explicitly, or rename the issue." + fi + + branch="${type}/${scope}/${slug}" + info "syncing main" + git fetch origin main --quiet + git pull --ff-only origin main + + info "creating branch '$branch' from origin/main" + git checkout -b "$branch" origin/main + + ok "branch '$branch' is ready. Commit work with: git commit -m '$type($scope): ...'" + printf "\nNext step:\n ./scripts/issue-workflow.sh pr-body %s --type=%s --scope=%s --slug=%s\n" \ + "$issue" "$type" "$scope" "$slug" +} + +cmd_pr_body() { + local issue="" type="" scope="" slug="" + while [[ $# -gt 0 ]]; do + case "$1" in + --type=*) type="${1#*=}"; shift ;; + --scope=*) scope="${1#*=}"; shift ;; + --slug=*) slug="${1#*=}"; shift ;; + --*) die "unknown flag: $1" ;; + *) + [[ -z "$issue" ]] && issue="$1" || die "unexpected extra arg: $1" + shift + ;; + esac + done + [[ -n "$issue" ]] || die "usage: issue-workflow.sh pr-body [--type=...] [--scope=...] [--slug=...]" + + if [[ -z "$type$scope$slug" ]]; then + local meta title + meta="$(fetch_issue_meta "$issue")" + title="$(printf "%s" "$meta" | python3 -c "import sys,json; print(json.load(sys.stdin)['title'])")" + local parsed + parsed="$(parse_issue_title "$title")" + type="${type:-$(awk '{print $1}' <<<"$parsed")}" + scope="${scope:-$(awk '{print $2}' <<<"$parsed")}" + slug="${slug:-$(awk '{print $3}' <<<"$parsed")}" + fi + [[ -n "$type$scope$slug" ]] || die "could not derive type/scope/slug — pass them explicitly" + + cat < + +## Linked issues + +Closes #$issue + +## Type of change + +- [x] \`$type\` — see the title; check the matching box below +$(for t in $ALLOWED_TYPES; do [[ "$t" != "$type" ]] && printf " - [ ] \`%s\`\n" "$t"; done) + +## Checklist + +- [x] Title follows Conventional Commits (\`$type($scope): subject\`) +- [x] I ran \`make ci\` locally and it passes +- [x] I added or updated tests for the change +- [x] I updated relevant docs (README, \`docs/\`, inline godoc) +- [x] No new linter warnings +- [ ] If breaking: I documented the migration path in the PR body and updated CHANGELOG.md + +## Scope notes / things reviewers may want to look at + + + +## Screenshots / output + + +EOF +} + +cmd_finish() { + local current_branch + current_branch="$(git rev-parse --abbrev-ref HEAD)" + [[ "$current_branch" != "main" ]] || die "finish must be run from a feature branch (you are on 'main')" + require_clean_tree + ensure_gh_authed + + # Sanity: at least one commit ahead of main. + local ahead behind + ahead="$(git rev-list --count origin/main..HEAD)" + behind="$(git rev-list --count HEAD..origin/main)" + [[ "$ahead" -ge 1 ]] || die "no commits ahead of origin/main. Nothing to PR." + info "branch is $ahead commit(s) ahead of main, $behind behind" + + # Sanity: conventional-commits subject on HEAD. + local subject + subject="$(git log -1 --pretty=%s)" + if ! [[ "$subject" =~ ^($ALLOWED_TYPES_RE)\(($ALLOWED_SCOPES_RE)\):\ ]]; then + die "HEAD subject '$subject' does not match conventional-commits pattern. " \ + "Fix with: git commit --amend -m '$subject'" + fi + + info "pushing '$current_branch' to origin" + git push -u origin "$current_branch" + + info "opening PR (base=main, head=$current_branch)" + gh pr create --base main --head "$current_branch" \ + --title "$subject" --body-file <(./scripts/issue-workflow.sh pr-body 0 \ + --type="$(awk -F'[(]' '{print $1}' <<<"$subject")" \ + --scope="$(awk -F'[()]' '{print $2}' <<<"$subject")" \ + --slug="") \ + || die "gh pr create failed — fill in the body manually and retry" + + local pr_number + pr_number="$(gh pr view "$current_branch" --json number --jq .number)" + info "PR #$pr_number opened. Waiting for CI…" + + # Wait for required checks (best-effort; 10-min cap). + if gh pr checks "$pr_number" --watch --fail-fast --interval 30 >/dev/null 2>&1 & + local watch_pid=$!; sleep 600; kill $watch_pid 2>/dev/null; wait 2>/dev/null; then :; fi + + info "squash-merging PR #$pr_number and deleting '$current_branch'" + gh pr merge "$pr_number" --squash --delete-branch \ + --body "Closes the linked issue. Squash-merged per CONTRIBUTING.md." \ + || die "merge failed — finish manually with: gh pr merge $pr_number --squash --delete-branch" + + info "syncing local main" + git checkout main + git pull --ff-only origin main + + ok "PR #$pr_number merged. '$current_branch' deleted. main is up to date." +} + +case "${1:-}" in + start) shift; cmd_start "$@" ;; + pr-body) shift; cmd_pr_body "$@" ;; + finish) shift; cmd_finish "$@" ;; + -h|--help|help|"") + sed -n '2,40p' "${BASH_SOURCE[0]}" | sed 's/^# \{0,1\}//' + ;; + *) die "unknown subcommand: $1 (try: start | pr-body | finish)" ;; +esac