From 16a82ff1767d77e124b44f39021104e7900aa3e1 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 18:16:03 +0000 Subject: [PATCH] Move the CLI to urfave/cli v3 (closes #110) Ported per the library's v2-to-v3 migration guide: the app is a root cli.Command, actions take a context and the command, and flag environment variables become value sources. -v, -q and the version flag are local so, as before, only the commands defining them accept them. Every command stops reading flags at its first argument, as v2 did. ErrWriter is stdout so usage errors print with their help. The action's context reaches the manifest download in check, export and list. testify rises to v1.12.1, which v3 requires; the urfave_cli_no_docs build tag, which v3 lacks, is dropped. v3 accepts a flag given under two names, so -v --verbose gives debug output, and the test pinning the refusal becomes a TestVerboseCount case. Model: opus-5-5 --- Dockerfile | 2 +- go.mod | 12 +-- go.sum | 28 ++---- internal/cli/check.go | 43 ++++---- internal/cli/entry_test.go | 46 ++++----- internal/cli/errmsg_test.go | 64 ++++++------ internal/cli/export.go | 12 ++- internal/cli/fetch.go | 30 +++--- internal/cli/fetch_test.go | 17 +--- internal/cli/freshen.go | 41 ++++---- internal/cli/gen.go | 39 ++++---- internal/cli/list.go | 14 +-- internal/cli/manifest_loader.go | 16 +-- internal/cli/mfer.go | 169 ++++++++++++++++++-------------- script/build | 4 +- 15 files changed, 275 insertions(+), 262 deletions(-) diff --git a/Dockerfile b/Dockerfile index 1db5ba3..67d3104 100644 --- a/Dockerfile +++ b/Dockerfile @@ -64,7 +64,7 @@ RUN version="${VERSION:-$(git describe --tags --always)}"; \ exit 1; \ fi; \ cd cmd/mfer && \ - CGO_ENABLED=0 go build -tags urfave_cli_no_docs -ldflags "-X main.Gitrev=$version" -o /mfer . + CGO_ENABLED=0 go build -ldflags "-X main.Gitrev=$version" -o /mfer . # Fail unless /mfer is statically linked: scratch has no C library to run it. RUN ldd /mfer 2>&1 | grep -q 'not a dynamic executable' diff --git a/go.mod b/go.mod index e014702..74ad570 100644 --- a/go.mod +++ b/go.mod @@ -9,27 +9,21 @@ require ( github.com/klauspost/compress v1.18.2 github.com/multiformats/go-multihash v0.2.3 github.com/spf13/afero v1.8.0 - github.com/stretchr/testify v1.8.1 - github.com/urfave/cli/v2 v2.27.7 + github.com/stretchr/testify v1.12.1 + github.com/urfave/cli/v3 v3.14.0 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 google.golang.org/protobuf v1.28.1 ) require ( - github.com/cpuguy83/go-md2man/v2 v2.0.7 // indirect github.com/klauspost/cpuid/v2 v2.0.9 // indirect - github.com/kr/pretty v0.2.0 // indirect github.com/minio/sha256-simd v1.0.0 // indirect github.com/mr-tron/base58 v1.2.0 // indirect github.com/multiformats/go-varint v0.0.6 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/russross/blackfriday/v2 v2.1.0 // indirect github.com/spaolacci/murmur3 v1.1.0 // indirect - github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 // indirect + go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e // indirect golang.org/x/sys v0.1.0 // indirect golang.org/x/text v0.3.6 // indirect - gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect lukechampine.com/blake3 v1.1.6 // indirect ) diff --git a/go.sum b/go.sum index da66064..e8e0f47 100644 --- a/go.sum +++ b/go.sum @@ -46,8 +46,6 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cncf/udpa/go v0.0.0-20200629203442-efcf912fb354/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk= github.com/cncf/udpa/go v0.0.0-20201120205902-5459f2c99403/go.mod h1:WmhPx2Nbnhtbo57+VJT5O0JRkEi1Wbu0z5j0R8u5Hbk= -github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo= -github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -134,10 +132,7 @@ github.com/klauspost/cpuid/v2 v2.0.9 h1:lgaqFMSdTdQYdZ04uHyN2d/eKdOMyi2YLSvlQIBF github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= -github.com/kr/pretty v0.2.0 h1:s5hAObm+yFO5uHYt5dYjxi2rXrsnmRpJx4OYvIWUaQs= -github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/minio/sha256-simd v1.0.0 h1:v1ta+49hkWZyvaKwrQB8elexRqm6Y0aMLjCNsrYxo6g= github.com/minio/sha256-simd v1.0.0/go.mod h1:OuYzVNI5vcoYIAmbIvHPl3N3jUzVedXbKy5RFepssQM= @@ -149,30 +144,21 @@ github.com/multiformats/go-varint v0.0.6 h1:gk85QWKxh3TazbLxED/NlDVv8+q+ReFJk7Y2 github.com/multiformats/go-varint v0.0.6/go.mod h1:3Ls8CIEsrijN6+B7PbrXRPxHRPuXSrVKRY101jdMZYE= github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/sftp v1.13.1/go.mod h1:3HaPG6Dq1ILlpPZRO0HVMrsydcdLt6HRDccSgb87qRg= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= -github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk= -github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spaolacci/murmur3 v1.1.0 h1:7c1g84S4BPRrfL5Xrdp6fOJ206sU9y293DDHaoy0bLI= github.com/spaolacci/murmur3 v1.1.0/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA= github.com/spf13/afero v1.8.0 h1:5MmtuhAgYeU6qpa7w7bP0dv6MBYuup0vekhSpSkoq60= github.com/spf13/afero v1.8.0/go.mod h1:CtAatgMJh6bJEIs48Ay/FOnkljP3WeGUG0MC1RfAqwo= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.5.1/go.mod h1:5W2xD1RspED5o8YsWQXVCued0rvSQ+mT+I5cxcmMvtA= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= -github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/urfave/cli/v2 v2.27.7 h1:bH59vdhbjLv3LAvIu6gd0usJHgoTTPhCFib8qqOwXYU= -github.com/urfave/cli/v2 v2.27.7/go.mod h1:CyNAG/xg+iAOg0N4MPGZqVmv2rCoP267496AOXUZjA4= -github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1 h1:gEOO8jv9F4OT7lGCjxCBTO/36wtF6j2nSip77qHd4x4= -github.com/xrash/smetrics v0.0.0-20240521201337-686a1a2994c1/go.mod h1:Ohn+xnUBiLI6FVj/9LpzZWtj1/D6lUovWYBkxHVV3aM= +github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= +github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= +github.com/urfave/cli/v3 v3.14.0 h1:a8414NQlHJs0c/iBsulKLzlES0n/lEAskbL2LKpU4/s= +github.com/urfave/cli/v3 v3.14.0/go.mod h1:vXn6HxPNccJSzQr2QvwVncOKrgYGIHU0HY5h8B2nQj4= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= @@ -183,6 +169,8 @@ go.opencensus.io v0.22.2/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.3/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.4/go.mod h1:yxeiOL68Rb0Xd1ddK5vPZ/oVn4vY4Ynel7k9FzqtOIw= go.opencensus.io v0.22.5/go.mod h1:5pWMHQbX5EPX2/62yrJeAkowc+lfs/XD7Uxpq3pI6kk= +go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= +go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190510104115-cbcb75029529/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= @@ -470,13 +458,9 @@ google.golang.org/protobuf v1.28.1 h1:d0NfwRgPtno5B1Wa6L2DAG+KivqkdutMf1UhdNx175 google.golang.org/protobuf v1.28.1/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo= -gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= -gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= -gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190106161140-3f1c8253044a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/internal/cli/check.go b/internal/cli/check.go index c146dfd..655b443 100644 --- a/internal/cli/check.go +++ b/internal/cli/check.go @@ -16,7 +16,7 @@ import ( "github.com/dustin/go-humanize" "github.com/spf13/afero" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/internal/log" "sneak.berlin/go/mfer/mfer" ) @@ -96,8 +96,10 @@ func findManifest(fs afero.Fs, dir string) (string, error) { // fetchManifestToTemp downloads a manifest URL to a temporary file and // returns the temp file path. The caller is responsible for removing it. -func (mfa *CLIApp) fetchManifestToTemp(url string) (string, error) { - rc, fetchErr := mfa.openManifestReader(url) +func (mfa *CLIApp) fetchManifestToTemp( + ctx context.Context, url string, +) (string, error) { + rc, fetchErr := mfa.openManifestReader(ctx, url) if fetchErr != nil { return "", fetchErr } @@ -208,13 +210,15 @@ func countCheckFailures( // findExtraFiles reports files present on disk but absent from the // manifest, and anything the search cannot read: each is a failure under // --no-extra-files, otherwise a warning. -func findExtraFiles(ctx *cli.Context, chk *mfer.Checker, failures *int64) error { +func findExtraFiles( + ctx context.Context, cmd *cli.Command, chk *mfer.Checker, failures *int64, +) error { extraResults := make(chan mfer.Result, 1) extraDone := make(chan struct{}) go func() { for result := range extraResults { - if ctx.Bool("no-extra-files") { + if cmd.Bool("no-extra-files") { *failures++ log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message) @@ -226,7 +230,7 @@ func findExtraFiles(ctx *cli.Context, chk *mfer.Checker, failures *int64) error close(extraDone) }() - err := chk.FindExtraFiles(ctx.Context, extraResults) + err := chk.FindExtraFiles(ctx, extraResults) if err != nil { return fmt.Errorf("failed to check for extra files: %w", err) } @@ -238,7 +242,9 @@ func findExtraFiles(ctx *cli.Context, chk *mfer.Checker, failures *int64) error // runCheck runs the manifest check with progress and result reporting // and returns the number of failures. -func runCheck(ctx *cli.Context, chk *mfer.Checker, showProgress bool) (int64, error) { +func runCheck( + ctx context.Context, cmd *cli.Command, chk *mfer.Checker, showProgress bool, +) (int64, error) { // Set up results channel results := make(chan mfer.Result, 1) @@ -264,7 +270,7 @@ func runCheck(ctx *cli.Context, chk *mfer.Checker, showProgress bool) (int64, er go countCheckFailures(results, &failures, done) // Run check - err := chk.Check(ctx.Context, results, progress) + err := chk.Check(ctx, results, progress) progressWg.Wait() @@ -275,7 +281,7 @@ func runCheck(ctx *cli.Context, chk *mfer.Checker, showProgress bool) (int64, er // Wait for results processing to complete <-done - err = findExtraFiles(ctx, chk, &failures) + err = findExtraFiles(ctx, cmd, chk, &failures) if err != nil { return 0, err } @@ -283,17 +289,19 @@ func runCheck(ctx *cli.Context, chk *mfer.Checker, showProgress bool) (int64, er return failures, nil } -func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error { +func (mfa *CLIApp) checkManifestOperation( + ctx context.Context, cmd *cli.Command, +) error { log.Debug("checkManifestOperation()") - manifestPath, err := mfa.resolveManifestArg(ctx) + manifestPath, err := mfa.resolveManifestArg(cmd) if err != nil { return fmt.Errorf("check: %w", err) } // URL manifests need to be downloaded to a temp file for the checker if isHTTPURL(manifestPath) { - tmpPath, tmpErr := mfa.fetchManifestToTemp(manifestPath) + tmpPath, tmpErr := mfa.fetchManifestToTemp(ctx, manifestPath) if tmpErr != nil { return fmt.Errorf("check: %w", tmpErr) } @@ -303,12 +311,13 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error { manifestPath = tmpPath } - basePath := ctx.String("base") - showProgress := ctx.Bool("progress") + basePath := cmd.String("base") + showProgress := cmd.Bool("progress") log.Infof("checking manifest %s with base %s", manifestPath, basePath) // Create checker + //nolint:contextcheck // mfer loads a manifest without a context chk, err := mfer.NewChecker(&mfer.CheckerOptions{ ManifestPath: manifestPath, BasePath: basePath, @@ -319,9 +328,9 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error { } // Check signature requirement - requiredSigner := ctx.String(flagRequireSignature) + requiredSigner := cmd.String(flagRequireSignature) if requiredSigner != "" { - err = verifyRequiredSigner(ctx.Context, chk, requiredSigner) + err = verifyRequiredSigner(ctx, chk, requiredSigner) if err != nil { return err } @@ -330,7 +339,7 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error { log.Infof("manifest contains %d files, %s", chk.FileCount(), humanize.IBytes(safeUint64(int64(chk.TotalBytes())))) - failures, err := runCheck(ctx, chk, showProgress) + failures, err := runCheck(ctx, cmd, chk, showProgress) if err != nil { return err } diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index e5e8a31..a7ac2f6 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -18,7 +18,7 @@ import ( "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - urfcli "github.com/urfave/cli/v2" + urfcli "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/internal/log" "sneak.berlin/go/mfer/mfer" ) @@ -264,8 +264,9 @@ func commandsTakingVerbose() [][]string { } // TestVerboseCount asserts that one -v or --verbose gives verbose output and -// two -v give debug output (issue #125). urfave/cli before v2.25.5 counted a -// flag given by its alias twice, so one -v gave debug output. +// two -v, or -v and --verbose, give debug output (issue #125). urfave/cli +// before v2.25.5 counted a flag given by its alias twice, so one -v gave debug +// output. func TestVerboseCount(t *testing.T) { t.Parallel() @@ -276,6 +277,7 @@ func TestVerboseCount(t *testing.T) { {[]string{"-v"}, log.VerboseLevel}, {[]string{testFlagVerbose}, log.VerboseLevel}, {[]string{"-v", "-v"}, log.DebugLevel}, + {[]string{"-v", testFlagVerbose}, log.DebugLevel}, } for _, command := range commandsTakingVerbose() { @@ -313,28 +315,6 @@ func TestCombinedShortVerboseRefused(t *testing.T) { } } -// TestShortAndLongVerboseRefused asserts that -v and --verbose given together -// are refused (issue #125): urfave/cli v2 refuses a flag given under two of its -// names, and one flag with an alias keeps help and parsing simple. -func TestShortAndLongVerboseRefused(t *testing.T) { - t.Parallel() - - for _, command := range commandsTakingVerbose() { - args := slices.Concat(command, []string{"-v", testFlagVerbose}) - - t.Run(strings.Join(args, " "), func(t *testing.T) { - t.Parallel() - - opts := testOpts(args, afero.NewMemMapFs()) - exitCode, level := runCLIWithLevel(opts) - - assert.Equal(t, 1, exitCode) - assert.Contains(t, testStderr(t, opts), "Cannot use two forms of the same flag") - assert.Equal(t, log.InfoLevel, level) - }) - } -} - func TestHelpCommand(t *testing.T) { t.Parallel() @@ -1190,6 +1170,22 @@ func TestGenerateValidatesInputPaths(t *testing.T) { }) } +// TestFlagAfterArgumentIsArgument asserts that flags are read only before a +// command's first argument: after it, -v is a path, not the verbose flag. +func TestFlagAfterArgumentIsArgument(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "content") + + opts := testOpts([]string{testApp, cmdGenerate, testDir, "-v"}, fs) + exitCode := runCLI(opts) + + assert.Equal(t, 1, exitCode) + assert.Contains(t, testStderr(t, opts), "path does not exist: -v") +} + func TestCheckDetectsManifestCorruption(t *testing.T) { t.Parallel() diff --git a/internal/cli/errmsg_test.go b/internal/cli/errmsg_test.go index 6c642af..bfcd336 100644 --- a/internal/cli/errmsg_test.go +++ b/internal/cli/errmsg_test.go @@ -4,7 +4,6 @@ package cli import ( "bytes" "context" - "flag" "net/http" "net/http/httptest" "os" @@ -15,7 +14,7 @@ import ( "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - urfcli "github.com/urfave/cli/v2" + urfcli "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/mfer" ) @@ -195,13 +194,17 @@ func signedChecker(t *testing.T, manifest []byte) *mfer.Checker { func TestPathDoesNotExistMessage(t *testing.T) { t.Parallel() - set := flag.NewFlagSet("gen", flag.ContinueOnError) - require.NoError(t, set.Parse([]string{"nope"})) - mfa := &CLIApp{Fs: afero.NewMemMapFs()} - ctx := urfcli.NewContext(nil, set, nil) + cmd := &urfcli.Command{ + Name: cmdGenerate, + Action: func(_ context.Context, c *urfcli.Command) error { + _, err := mfa.collectInputPaths(c.Args()) - _, err := mfa.collectInputPaths(ctx.Args()) + return err + }, + } + + err := cmd.Run(context.Background(), []string{cmdGenerate, "nope"}) require.ErrorIs(t, err, errPathNotExist) assert.EqualError(t, err, "path does not exist: nope") } @@ -214,18 +217,22 @@ func TestOutputFileExistsMessage(t *testing.T) { require.NoError(t, afero.WriteFile(fs, "/d/f.txt", []byte("hi"), 0o644)) require.NoError(t, afero.WriteFile(fs, "/out.mf", []byte("old"), 0o644)) - set := flag.NewFlagSet("gen", flag.ContinueOnError) - set.String("output", "", "") - set.Bool("force", false, "") - require.NoError(t, set.Parse([]string{"/d"})) - require.NoError(t, set.Set("output", "/out.mf")) - mfa := &CLIApp{Fs: fs} - ctx := urfcli.NewContext(nil, set, nil) + cmd := &urfcli.Command{ + Name: cmdGenerate, + Flags: []urfcli.Flag{ + &urfcli.StringFlag{Name: "output"}, + &urfcli.BoolFlag{Name: "force"}, + }, + Action: mfa.generateManifestOperation, + } // generateManifestOperation writes to the process-global logger during // enumeration, so serialize with the other CLI runs. - err := runLocked(func() error { return mfa.generateManifestOperation(ctx) }) + err := runLocked(func() error { + return cmd.Run(context.Background(), + []string{cmdGenerate, "--output", "/out.mf", "/d"}) + }) require.ErrorIs(t, err, errOutputExists) assert.EqualError(t, err, "output file /out.mf already exists (use --force to overwrite)") @@ -250,7 +257,7 @@ func TestUnknownCommandMessage(t *testing.T) { err := runLocked(func() error { mfa.run([]string{testApp}) - return mfa.app.Run([]string{testApp, "bogus"}) + return mfa.app.Run(context.Background(), []string{testApp, "bogus"}) }) require.ErrorIs(t, err, errUnknownCommand) assert.EqualError(t, err, `unknown command "bogus"`) @@ -267,7 +274,7 @@ func TestManifestLoaderHTTPStatusMessage(t *testing.T) { mfa := &CLIApp{Fs: afero.NewMemMapFs()} - _, err := mfa.openManifestReader(server.URL + "/foo.mf") + _, err := mfa.openManifestReader(context.Background(), server.URL+"/foo.mf") require.ErrorIs(t, err, errHTTPStatus) assert.EqualError(t, err, "failed to fetch "+server.URL+"/foo.mf: HTTP 404") @@ -284,17 +291,13 @@ func TestFetchManifestHTTPStatusMessage(t *testing.T) { mfa := &CLIApp{Fs: afero.NewMemMapFs()} - set := flag.NewFlagSet(cmdFetch, flag.ContinueOnError) - for _, f := range mfa.fetchCommand().Flags { - require.NoError(t, f.Apply(set)) - } - - require.NoError(t, set.Parse([]string{server.URL})) - - ctx := urfcli.NewContext(nil, set, nil) + cmd := mfa.fetchCommand() + cmd.Action = mfa.fetchManifestOperation // fetchManifestOperation logs to the process-global logger. - err := runLocked(func() error { return mfa.fetchManifestOperation(ctx) }) + err := runLocked(func() error { + return cmd.Run(context.Background(), []string{cmdFetch, server.URL}) + }) require.ErrorIs(t, err, errHTTPStatus) assert.EqualError(t, err, "failed to fetch manifest: HTTP 404") } @@ -320,14 +323,13 @@ func TestFetchFileHTTPStatusMessage(t *testing.T) { func TestURLRequiredMessage(t *testing.T) { t.Parallel() - set := flag.NewFlagSet("fetch", flag.ContinueOnError) - require.NoError(t, set.Parse([]string{})) - mfa := &CLIApp{Fs: afero.NewMemMapFs()} - ctx := urfcli.NewContext(nil, set, nil) + cmd := &urfcli.Command{Name: cmdFetch, Action: mfa.fetchManifestOperation} // fetchManifestOperation logs to the process-global logger. - err := runLocked(func() error { return mfa.fetchManifestOperation(ctx) }) + err := runLocked(func() error { + return cmd.Run(context.Background(), []string{cmdFetch}) + }) require.ErrorIs(t, err, errURLRequired) assert.EqualError(t, err, "URL argument required") } diff --git a/internal/cli/export.go b/internal/cli/export.go index a557137..3ceda69 100644 --- a/internal/cli/export.go +++ b/internal/cli/export.go @@ -1,12 +1,13 @@ package cli import ( + "context" "encoding/hex" "encoding/json" "fmt" "time" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/mfer" ) @@ -19,19 +20,22 @@ type ExportEntry struct { Ctime *string `json:"ctime,omitempty"` } -func (mfa *CLIApp) exportManifestOperation(ctx *cli.Context) error { - pathOrURL, err := mfa.resolveManifestArg(ctx) +func (mfa *CLIApp) exportManifestOperation( + ctx context.Context, cmd *cli.Command, +) error { + pathOrURL, err := mfa.resolveManifestArg(cmd) if err != nil { return fmt.Errorf("export: %w", err) } - rc, err := mfa.openManifestReader(pathOrURL) + rc, err := mfa.openManifestReader(ctx, pathOrURL) if err != nil { return fmt.Errorf("export: %w", err) } defer func() { _ = rc.Close() }() + //nolint:contextcheck // mfer loads a manifest without a context manifest, err := mfer.NewManifestFromReader(rc) if err != nil { return fmt.Errorf("export: failed to parse manifest: %w", err) diff --git a/internal/cli/fetch.go b/internal/cli/fetch.go index ad1c0a2..966a771 100644 --- a/internal/cli/fetch.go +++ b/internal/cli/fetch.go @@ -20,7 +20,7 @@ import ( "github.com/dustin/go-humanize" "github.com/multiformats/go-multihash" "github.com/spf13/afero" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/internal/log" "sneak.berlin/go/mfer/mfer" ) @@ -328,19 +328,21 @@ func alreadyPresent(dest, localPath string, entry *mfer.MFFilePath) bool { return verifyDownloadedHash(h.Sum(nil), entry) == nil } -func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { +func (mfa *CLIApp) fetchManifestOperation( + ctx context.Context, cmd *cli.Command, +) error { log.Debug("fetchManifestOperation()") - if ctx.Args().Len() == 0 { + if cmd.Args().Len() == 0 { return errURLRequired } - timeout := ctx.Duration(flagTimeout) + timeout := cmd.Duration(flagTimeout) if timeout <= 0 { return errInvalidTimeout } - manifestURL, err := resolveManifestURL(ctx.Args().Get(0)) + manifestURL, err := resolveManifestURL(cmd.Args().Get(0)) if err != nil { return fmt.Errorf("invalid URL: %w", err) } @@ -350,7 +352,7 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { firstDelay: firstRetryDelay, } - manifestData, files, err := fetchManifest(ctx, client, manifestURL) + manifestData, files, err := fetchManifest(ctx, cmd, client, manifestURL) if err != nil { return err } @@ -361,7 +363,7 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { return err } - dest := ctx.String(flagDest) + dest := cmd.String(flagDest) err = os.MkdirAll(dest, dirPerms) if err != nil { @@ -379,7 +381,7 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { // Download each file downloaded, downloadedBytes, dlErr := downloadManifestFiles( - ctx.Context, client, baseURL, dest, files, progress) + ctx, client, baseURL, dest, files, progress) close(progress) <-done @@ -416,7 +418,7 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { // downloaded, to be saved once the files are in place, and the files it // lists. func fetchManifest( - ctx *cli.Context, client retryingClient, manifestURL string, + ctx context.Context, cmd *cli.Command, client retryingClient, manifestURL string, ) ([]byte, []*mfer.MFFilePath, error) { log.Infof("fetching manifest from %s", manifestURL) @@ -425,7 +427,7 @@ func fetchManifest( // manifest. var manifestData []byte - err := client.get(ctx.Context, manifestURL, func(resp *http.Response) error { + err := client.get(ctx, manifestURL, func(resp *http.Response) error { var readErr error manifestData, readErr = io.ReadAll(resp.Body) @@ -437,12 +439,13 @@ func fetchManifest( } // Parse manifest + //nolint:contextcheck // mfer loads a manifest without a context manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) if err != nil { return nil, nil, fmt.Errorf("failed to parse manifest: %w", err) } - requiredSigner := ctx.String(flagRequireSignature) + requiredSigner := cmd.String(flagRequireSignature) if requiredSigner != "" { err = verifyFetchedSigner(ctx, manifestData, requiredSigner) if err != nil { @@ -504,7 +507,7 @@ func checkNoNameClash(files []*mfer.MFFilePath) error { // its manifest from a file, so the manifest is handed to it as a file in // memory. func verifyFetchedSigner( - ctx *cli.Context, manifestData []byte, requiredSigner string, + ctx context.Context, manifestData []byte, requiredSigner string, ) error { memFs := afero.NewMemMapFs() manifestPath := "/" + defaultManifestName @@ -514,6 +517,7 @@ func verifyFetchedSigner( return err } + //nolint:contextcheck // mfer loads a manifest without a context chk, err := mfer.NewChecker(&mfer.CheckerOptions{ ManifestPath: manifestPath, BasePath: "/", @@ -523,7 +527,7 @@ func verifyFetchedSigner( return fmt.Errorf("failed to load manifest: %w", err) } - return verifyRequiredSigner(ctx.Context, chk, requiredSigner) + return verifyRequiredSigner(ctx, chk, requiredSigner) } // saveManifest writes the fetched manifest into dest under the default diff --git a/internal/cli/fetch_test.go b/internal/cli/fetch_test.go index b9859ec..10e263c 100644 --- a/internal/cli/fetch_test.go +++ b/internal/cli/fetch_test.go @@ -4,7 +4,6 @@ package cli import ( "bytes" "context" - "flag" "fmt" "io" "maps" @@ -23,7 +22,6 @@ import ( "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - urfcli "github.com/urfave/cli/v2" "sneak.berlin/go/mfer/mfer" ) @@ -1322,18 +1320,13 @@ func TestFetchTimeoutFlag(t *testing.T) { mfa := &CLIApp{Fs: afero.NewMemMapFs()} - set := flag.NewFlagSet(cmdFetch, flag.ContinueOnError) - for _, f := range mfa.fetchCommand().Flags { - require.NoError(t, f.Apply(set)) - } - - require.NoError(t, set.Parse([]string{"--" + flagTimeout, "100ms", server.URL})) - - cliCtx := urfcli.NewContext(nil, set, nil) - cliCtx.Context = ctx + cmd := mfa.fetchCommand() + cmd.Action = mfa.fetchManifestOperation // fetchManifestOperation logs to the process-global logger. - err := runLocked(func() error { return mfa.fetchManifestOperation(cliCtx) }) + err := runLocked(func() error { + return cmd.Run(ctx, []string{cmdFetch, "--" + flagTimeout, "100ms", server.URL}) + }) require.Error(t, err) } diff --git a/internal/cli/freshen.go b/internal/cli/freshen.go index e50d803..ee7ae37 100644 --- a/internal/cli/freshen.go +++ b/internal/cli/freshen.go @@ -14,7 +14,7 @@ import ( "github.com/dustin/go-humanize" "github.com/multiformats/go-multihash" "github.com/spf13/afero" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/internal/log" "sneak.berlin/go/mfer/mfer" ) @@ -202,12 +202,12 @@ func (s *freshenScanner) walk(path string, info fs.FileInfo, walkErr error) erro // resolveFreshenManifestPath determines the manifest path from the CLI // arguments, searching directories for a manifest where needed. -func (mfa *CLIApp) resolveFreshenManifestPath(ctx *cli.Context) (string, error) { - if ctx.Args().Len() == 0 { +func (mfa *CLIApp) resolveFreshenManifestPath(cmd *cli.Command) (string, error) { + if cmd.Args().Len() == 0 { return findManifest(mfa.Fs, ".") } - arg := ctx.Args().Get(0) + arg := cmd.Args().Get(0) info, statErr := mfa.Fs.Stat(arg) if statErr == nil && info.IsDir() { @@ -338,14 +338,14 @@ func writeFreshenedManifest( // newFreshenBuilder constructs the manifest builder configured from CLI // flags. -func newFreshenBuilder(ctx *cli.Context) *mfer.Builder { +func newFreshenBuilder(cmd *cli.Command) *mfer.Builder { builder := mfer.NewBuilder() - if ctx.Bool("include-timestamps") { + if cmd.Bool("include-timestamps") { builder.SetIncludeTimestamps(true) } // Set up signing options if sign-key is provided - if signKey := ctx.String("sign-key"); signKey != "" { + if signKey := cmd.String("sign-key"); signKey != "" { builder.SetSigningOptions(&mfer.SigningOptions{ KeyID: mfer.GPGKeyID(signKey), }) @@ -358,13 +358,13 @@ func newFreshenBuilder(ctx *cli.Context) *mfer.Builder { // freshenScan runs the scan phase against the loaded manifest entries // and returns the populated scanner and the count of removed files. func (mfa *CLIApp) freshenScan( - ctx *cli.Context, manifestPath, absBase string, + cmd *cli.Command, manifestPath, absBase string, existingByPath map[string]*mfer.MFFilePath, ) (*freshenScanner, int64, error) { log.Infof("scanning filesystem...") startScan := time.Now() - showProgress := ctx.Bool("progress") + showProgress := cmd.Bool("progress") // Leave out the manifest and a temp file left by an interrupted run, // as gen does. A path that cannot be stat'd, normally because no file @@ -382,8 +382,8 @@ func (mfa *CLIApp) freshenScan( fs: mfa.Fs, absBase: absBase, excluded: excluded, - includeDotfiles: ctx.Bool("include-dotfiles"), - followSymlinks: ctx.Bool("follow-symlinks"), + includeDotfiles: cmd.Bool("include-dotfiles"), + followSymlinks: cmd.Bool("follow-symlinks"), showProgress: showProgress, existingByPath: existingByPath, } @@ -433,7 +433,7 @@ func hashTotals(entries []*freshenEntry) (int64, int64) { // runFreshenHash processes every entry through the hasher, aborting if // the context is canceled. func runFreshenHash( - ctx *cli.Context, hasher *freshenHasher, entries []*freshenEntry, + ctx context.Context, hasher *freshenHasher, entries []*freshenEntry, ) error { for _, e := range entries { select { @@ -479,18 +479,21 @@ func (mfa *CLIApp) loadExistingEntries( return existingByPath, nil } -func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { +func (mfa *CLIApp) freshenManifestOperation( + ctx context.Context, cmd *cli.Command, +) error { log.Debug("freshenManifestOperation()") - basePath := ctx.String("base") - showProgress := ctx.Bool("progress") + basePath := cmd.String("base") + showProgress := cmd.Bool("progress") // Find manifest file - manifestPath, err := mfa.resolveFreshenManifestPath(ctx) + manifestPath, err := mfa.resolveFreshenManifestPath(cmd) if err != nil { return fmt.Errorf("freshen: %w", err) } + //nolint:contextcheck // mfer loads a manifest without a context existingByPath, err := mfa.loadExistingEntries(manifestPath) if err != nil { return err @@ -502,7 +505,7 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { } // Phase 1: Scan filesystem - scanner, removed, err := mfa.freshenScan(ctx, manifestPath, absBase, + scanner, removed, err := mfa.freshenScan(cmd, manifestPath, absBase, existingByPath) if err != nil { return err @@ -524,7 +527,7 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { totalHashBytes: totalHashBytes, filesToHash: filesToHash, startHash: time.Now(), - builder: newFreshenBuilder(ctx), + builder: newFreshenBuilder(cmd), } err = runFreshenHash(ctx, hasher, scanner.entries) @@ -548,7 +551,7 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { } // Write updated manifest atomically (write to temp, then rename) - err = writeFreshenedManifest(ctx.Context, mfa.Fs, hasher.builder, manifestPath) + err = writeFreshenedManifest(ctx, mfa.Fs, hasher.builder, manifestPath) if err != nil { return err } diff --git a/internal/cli/gen.go b/internal/cli/gen.go index 68c2292..5630b29 100644 --- a/internal/cli/gen.go +++ b/internal/cli/gen.go @@ -1,6 +1,7 @@ package cli import ( + "context" "errors" "fmt" "os" @@ -12,7 +13,7 @@ import ( "github.com/dustin/go-humanize" "github.com/spf13/afero" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/internal/log" "sneak.berlin/go/mfer/mfer" ) @@ -88,12 +89,12 @@ func (mfa *CLIApp) collectInputPaths(args cli.Args) ([]string, error) { } // buildScannerOptions constructs scanner options from the CLI flags. -func (mfa *CLIApp) buildScannerOptions(ctx *cli.Context) *mfer.ScannerOptions { - output := ctx.String("output") +func (mfa *CLIApp) buildScannerOptions(cmd *cli.Command) *mfer.ScannerOptions { + output := cmd.String("output") opts := &mfer.ScannerOptions{ - IncludeDotfiles: ctx.Bool("include-dotfiles"), - FollowSymLinks: ctx.Bool("follow-symlinks"), - IncludeTimestamps: ctx.Bool("include-timestamps"), + IncludeDotfiles: cmd.Bool("include-dotfiles"), + FollowSymLinks: cmd.Bool("follow-symlinks"), + IncludeTimestamps: cmd.Bool("include-timestamps"), Fs: mfa.Fs, // Neither a manifest being replaced nor a temp file left by an // interrupted run belongs in the new manifest. @@ -101,14 +102,14 @@ func (mfa *CLIApp) buildScannerOptions(ctx *cli.Context) *mfer.ScannerOptions { } // Set seed for deterministic UUID if provided - if seed := ctx.String("seed"); seed != "" { + if seed := cmd.String("seed"); seed != "" { opts.Seed = seed log.Infof("using deterministic seed for manifest UUID") } // Set up signing options if sign-key is provided - if signKey := ctx.String("sign-key"); signKey != "" { + if signKey := cmd.String("sign-key"); signKey != "" { opts.SigningOptions = &mfer.SigningOptions{ KeyID: mfer.GPGKeyID(signKey), } @@ -173,14 +174,14 @@ func (mfa *CLIApp) cleanupOnSignal(outFile afero.File, tmpPath string) chan os.S // runEnumeratePhase enumerates all input paths with optional progress // reporting and logs the totals. -func (mfa *CLIApp) runEnumeratePhase(ctx *cli.Context, s *mfer.Scanner) error { +func (mfa *CLIApp) runEnumeratePhase(cmd *cli.Command, s *mfer.Scanner) error { // Set up enumeration progress reporting var ( enumProgress chan mfer.EnumerateStatus enumWg sync.WaitGroup ) - if ctx.Bool("progress") { + if cmd.Bool("progress") { enumProgress = make(chan mfer.EnumerateStatus, 1) enumWg.Add(1) @@ -188,7 +189,7 @@ func (mfa *CLIApp) runEnumeratePhase(ctx *cli.Context, s *mfer.Scanner) error { go reportEnumProgress(enumProgress, &enumWg) } - err := mfa.enumerateInputs(s, ctx.Args(), enumProgress) + err := mfa.enumerateInputs(s, cmd.Args(), enumProgress) if err != nil { return err } @@ -201,22 +202,24 @@ func (mfa *CLIApp) runEnumeratePhase(ctx *cli.Context, s *mfer.Scanner) error { return nil } -func (mfa *CLIApp) generateManifestOperation(ctx *cli.Context) error { +func (mfa *CLIApp) generateManifestOperation( + ctx context.Context, cmd *cli.Command, +) error { log.Debug("generateManifestOperation()") - s := mfer.NewScannerWithOptions(mfa.buildScannerOptions(ctx)) + s := mfer.NewScannerWithOptions(mfa.buildScannerOptions(cmd)) // Phase 1: Enumeration - collect paths and stat files - err := mfa.runEnumeratePhase(ctx, s) + err := mfa.runEnumeratePhase(cmd, s) if err != nil { return err } - showProgress := ctx.Bool("progress") + showProgress := cmd.Bool("progress") // Check if output file exists - outputPath := ctx.String("output") - if exists, _ := afero.Exists(mfa.Fs, outputPath); exists && !ctx.Bool("force") { + outputPath := cmd.String("output") + if exists, _ := afero.Exists(mfa.Fs, outputPath); exists && !cmd.Bool("force") { return fmt.Errorf("output file %s %w", outputPath, errOutputExists) } @@ -259,7 +262,7 @@ func (mfa *CLIApp) generateManifestOperation(ctx *cli.Context) error { go reportScanProgress(scanProgress, &scanWg) } - err = s.ToManifest(ctx.Context, outFile, scanProgress) + err = s.ToManifest(ctx, outFile, scanProgress) scanWg.Wait() diff --git a/internal/cli/list.go b/internal/cli/list.go index dbf2096..8119f2f 100644 --- a/internal/cli/list.go +++ b/internal/cli/list.go @@ -1,33 +1,35 @@ package cli import ( + "context" "fmt" "time" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/internal/log" "sneak.berlin/go/mfer/mfer" ) -func (mfa *CLIApp) listManifestOperation(ctx *cli.Context) error { +func (mfa *CLIApp) listManifestOperation(ctx context.Context, cmd *cli.Command) error { // Default to ErrorLevel for clean output log.SetLevel(log.ErrorLevel) - longFormat := ctx.Bool("long") - print0 := ctx.Bool("print0") + longFormat := cmd.Bool("long") + print0 := cmd.Bool("print0") - pathOrURL, err := mfa.resolveManifestArg(ctx) + pathOrURL, err := mfa.resolveManifestArg(cmd) if err != nil { return fmt.Errorf("list: %w", err) } - rc, err := mfa.openManifestReader(pathOrURL) + rc, err := mfa.openManifestReader(ctx, pathOrURL) if err != nil { return fmt.Errorf("list: %w", err) } defer func() { _ = rc.Close() }() + //nolint:contextcheck // mfer loads a manifest without a context manifest, err := mfer.NewManifestFromReader(rc) if err != nil { return fmt.Errorf("list: failed to parse manifest: %w", err) diff --git a/internal/cli/manifest_loader.go b/internal/cli/manifest_loader.go index 46edcc4..583fba7 100644 --- a/internal/cli/manifest_loader.go +++ b/internal/cli/manifest_loader.go @@ -9,7 +9,7 @@ import ( "strings" "time" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" ) // manifestFetchTimeout bounds HTTP requests made to fetch a manifest. @@ -30,13 +30,13 @@ func isHTTPURL(s string) bool { // openManifestReader opens a manifest from a path or URL and returns a ReadCloser. // The caller must close the returned reader. -func (mfa *CLIApp) openManifestReader(pathOrURL string) (io.ReadCloser, error) { +func (mfa *CLIApp) openManifestReader( + ctx context.Context, pathOrURL string, +) (io.ReadCloser, error) { if isHTTPURL(pathOrURL) { client := &http.Client{Timeout: manifestFetchTimeout} - req, err := http.NewRequestWithContext( - context.Background(), http.MethodGet, pathOrURL, nil, - ) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, pathOrURL, nil) if err != nil { return nil, fmt.Errorf("failed to fetch %s: %w", pathOrURL, err) } @@ -67,9 +67,9 @@ func (mfa *CLIApp) openManifestReader(pathOrURL string) (io.ReadCloser, error) { // resolveManifestArg resolves the manifest path from CLI arguments. // HTTP(S) URLs are returned as-is. Directories are searched for index.mf. // If no argument is given, the current directory is searched. -func (mfa *CLIApp) resolveManifestArg(ctx *cli.Context) (string, error) { - if ctx.Args().Len() > 0 { - arg := ctx.Args().Get(0) +func (mfa *CLIApp) resolveManifestArg(cmd *cli.Command) (string, error) { + if cmd.Args().Len() > 0 { + arg := cmd.Args().Get(0) if isHTTPURL(arg) { return arg, nil } diff --git a/internal/cli/mfer.go b/internal/cli/mfer.go index 86516d3..708f8af 100644 --- a/internal/cli/mfer.go +++ b/internal/cli/mfer.go @@ -1,6 +1,7 @@ package cli import ( + "context" "errors" "fmt" "io" @@ -8,7 +9,7 @@ import ( "time" "github.com/spf13/afero" - "github.com/urfave/cli/v2" + "github.com/urfave/cli/v3" "sneak.berlin/go/mfer/internal/log" "sneak.berlin/go/mfer/mfer" ) @@ -54,7 +55,7 @@ type CLIApp struct { gitrev string startupTime time.Time exitCode int - app *cli.App + app *cli.Command Stdin io.Reader // Standard input stream Stdout io.Writer // Standard output stream for normal output @@ -105,15 +106,15 @@ func (mfa *CLIApp) printBanner() { // urfave/cli reads a flag from the nearest command that defines it, so each // command in the lineage is asked. The highest -v count wins rather than the // sum, because a subcommand without its own copies reads the root's. -func (mfa *CLIApp) setVerbosity(c *cli.Context) { +func (mfa *CLIApp) setVerbosity(cmd *cli.Command) { _, present := os.LookupEnv("MFER_DEBUG") verbosity := 0 quiet := false - for _, ctx := range c.Lineage() { - verbosity = max(verbosity, ctx.Count("verbose")) - quiet = quiet || ctx.Bool("quiet") + for _, c := range cmd.Lineage() { + verbosity = max(verbosity, c.Count("verbose")) + quiet = quiet || c.Bool("quiet") } switch { @@ -127,23 +128,34 @@ func (mfa *CLIApp) setVerbosity(c *cli.Context) { } // commonFlags returns the -v and -q flags taken by the root and by the -// generate, check, freshen and fetch subcommands. +// generate, check, freshen and fetch subcommands. They are local, so the +// root's copies are not inherited by the subcommands that do not take them. func commonFlags() []cli.Flag { return []cli.Flag{ &cli.BoolFlag{ Name: "verbose", Aliases: []string{"v"}, Usage: "Increase verbosity (-v for verbose, -v -v for debug)", - Count: new(int), + Local: true, }, &cli.BoolFlag{ Name: "quiet", Aliases: []string{"q"}, Usage: "Suppress output except errors", + Local: true, }, } } +// stopOnFirstArg returns the StopOnNthArg setting every command uses: flags +// are read only before the command's first argument, and everything after it +// is an argument, as with urfave/cli v2. So `mfer gen d -v` names a path "-v". +func stopOnFirstArg() *int { + n := 1 + + return &n +} + // requireSignatureFlag returns the --require-signature flag taken by the // check and fetch subcommands. func requireSignatureFlag() *cli.StringFlag { @@ -151,21 +163,22 @@ func requireSignatureFlag() *cli.StringFlag { Name: flagRequireSignature, Aliases: []string{"S"}, Usage: "Require manifest to be signed by the specified GPG key ID", - EnvVars: []string{"MFER_REQUIRE_SIGNATURE"}, + Sources: cli.EnvVars("MFER_REQUIRE_SIGNATURE"), } } func (mfa *CLIApp) generateCommand() *cli.Command { return &cli.Command{ - Name: cmdGenerate, - Aliases: []string{"gen"}, - Usage: "Generate manifest file", - ArgsUsage: "[path ...]", - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) + Name: cmdGenerate, + Aliases: []string{"gen"}, + Usage: "Generate manifest file", + ArgsUsage: "[path ...]", + StopOnNthArg: stopOnFirstArg(), + Action: func(ctx context.Context, cmd *cli.Command) error { + mfa.setVerbosity(cmd) mfa.printBanner() - return mfa.generateManifestOperation(c) + return mfa.generateManifestOperation(ctx, cmd) }, Flags: append(commonFlags(), &cli.BoolFlag{ @@ -199,12 +212,12 @@ func (mfa *CLIApp) generateCommand() *cli.Command { Name: "sign-key", Aliases: []string{"s"}, Usage: "GPG key ID to sign the manifest with", - EnvVars: []string{"MFER_SIGN_KEY"}, + Sources: cli.EnvVars("MFER_SIGN_KEY"), }, &cli.StringFlag{ Name: "seed", Usage: "Seed value for deterministic manifest UUID", - EnvVars: []string{"MFER_SEED"}, + Sources: cli.EnvVars("MFER_SEED"), }, &cli.BoolFlag{ Name: "include-timestamps", @@ -217,14 +230,15 @@ func (mfa *CLIApp) generateCommand() *cli.Command { func (mfa *CLIApp) checkCommand() *cli.Command { return &cli.Command{ - Name: cmdCheck, - Usage: "Validate files using manifest file", - ArgsUsage: manifestArgsUsage, - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) + Name: cmdCheck, + Usage: "Validate files using manifest file", + ArgsUsage: manifestArgsUsage, + StopOnNthArg: stopOnFirstArg(), + Action: func(ctx context.Context, cmd *cli.Command) error { + mfa.setVerbosity(cmd) mfa.printBanner() - return mfa.checkManifestOperation(c) + return mfa.checkManifestOperation(ctx, cmd) }, Flags: append(commonFlags(), &cli.StringFlag{ @@ -249,14 +263,15 @@ func (mfa *CLIApp) checkCommand() *cli.Command { func (mfa *CLIApp) freshenCommand() *cli.Command { return &cli.Command{ - Name: cmdFreshen, - Usage: "Update manifest with changed, new, and removed files", - ArgsUsage: manifestArgsUsage, - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) + Name: cmdFreshen, + Usage: "Update manifest with changed, new, and removed files", + ArgsUsage: manifestArgsUsage, + StopOnNthArg: stopOnFirstArg(), + Action: func(ctx context.Context, cmd *cli.Command) error { + mfa.setVerbosity(cmd) mfa.printBanner() - return mfa.freshenManifestOperation(c) + return mfa.freshenManifestOperation(ctx, cmd) }, Flags: append(commonFlags(), &cli.StringFlag{ @@ -285,7 +300,7 @@ func (mfa *CLIApp) freshenCommand() *cli.Command { Name: "sign-key", Aliases: []string{"s"}, Usage: "GPG key ID to sign the manifest with", - EnvVars: []string{"MFER_SIGN_KEY"}, + Sources: cli.EnvVars("MFER_SIGN_KEY"), }, &cli.BoolFlag{ Name: "include-timestamps", @@ -298,22 +313,24 @@ func (mfa *CLIApp) freshenCommand() *cli.Command { func (mfa *CLIApp) exportCommand() *cli.Command { return &cli.Command{ - Name: cmdExport, - Usage: "Export manifest contents as JSON", - ArgsUsage: "[manifest file or URL]", - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) + Name: cmdExport, + Usage: "Export manifest contents as JSON", + ArgsUsage: "[manifest file or URL]", + StopOnNthArg: stopOnFirstArg(), + Action: func(ctx context.Context, cmd *cli.Command) error { + mfa.setVerbosity(cmd) - return mfa.exportManifestOperation(c) + return mfa.exportManifestOperation(ctx, cmd) }, } } func (mfa *CLIApp) versionCommand() *cli.Command { return &cli.Command{ - Name: cmdVersion, - Usage: "Show version", - Action: func(_ *cli.Context) error { + Name: cmdVersion, + Usage: "Show version", + StopOnNthArg: stopOnFirstArg(), + Action: func(context.Context, *cli.Command) error { mfa.printVersion() return nil @@ -323,13 +340,12 @@ func (mfa *CLIApp) versionCommand() *cli.Command { func (mfa *CLIApp) listCommand() *cli.Command { return &cli.Command{ - Name: "list", - Aliases: []string{"ls"}, - Usage: "List files in manifest", - ArgsUsage: manifestArgsUsage, - Action: func(c *cli.Context) error { - return mfa.listManifestOperation(c) - }, + Name: "list", + Aliases: []string{"ls"}, + Usage: "List files in manifest", + ArgsUsage: manifestArgsUsage, + StopOnNthArg: stopOnFirstArg(), + Action: mfa.listManifestOperation, Flags: []cli.Flag{ &cli.BoolFlag{ Name: "long", @@ -346,14 +362,15 @@ func (mfa *CLIApp) listCommand() *cli.Command { func (mfa *CLIApp) fetchCommand() *cli.Command { return &cli.Command{ - Name: cmdFetch, - Usage: "fetch manifest and referenced files", - ArgsUsage: "URL", - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) + Name: cmdFetch, + Usage: "fetch manifest and referenced files", + ArgsUsage: "URL", + StopOnNthArg: stopOnFirstArg(), + Action: func(ctx context.Context, cmd *cli.Command) error { + mfa.setVerbosity(cmd) mfa.printBanner() - return mfa.fetchManifestOperation(c) + return mfa.fetchManifestOperation(ctx, cmd) }, Flags: append(commonFlags(), &cli.DurationFlag{ @@ -385,38 +402,42 @@ func (mfa *CLIApp) run(args []string) { log.SetOutput(mfa.Stdout, mfa.Stderr) log.Init() - // -v means verbose, not version. urfave/cli's built-in version flag - // claims -v by default, which made "mfer -v --version" fail to parse and - // gave -v a different meaning at the root than on the generate, check, - // freshen and fetch subcommands, where it means verbose. Verbose is the - // more common meaning of -v in tools that offer both, so -v means verbose - // at the root too and the version flag takes the capital -V. + // -v means verbose, not version, at the root as on the generate, check, + // freshen and fetch subcommands: verbose is the more common meaning of -v + // in tools that offer both. urfave/cli's built-in version flag claims -v + // by default, so the version flag takes the capital -V instead. // VersionFlag and VersionPrinter are urfave/cli package globals; run() is // serialized in tests, so assigning them here is safe. cli.VersionFlag = &cli.BoolFlag{ Name: cmdVersion, Aliases: []string{"V"}, Usage: "print the version", + Local: true, } - cli.VersionPrinter = func(_ *cli.Context) { mfa.printVersion() } + cli.VersionPrinter = func(*cli.Command) { mfa.printVersion() } - mfa.app = &cli.App{ - Name: mfa.appname, - Usage: "Manifest generator", - Version: mfa.VersionString(), - EnableBashCompletion: true, - Writer: mfa.Stdout, - ErrWriter: mfa.Stderr, - Flags: commonFlags(), - Action: func(c *cli.Context) error { - if c.Args().Len() > 0 { - return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First()) + mfa.app = &cli.Command{ + Name: mfa.appname, + Usage: "Manifest generator", + Version: mfa.VersionString(), + EnableShellCompletion: true, + Writer: mfa.Stdout, + // urfave/cli writes only its "Incorrect Usage" line to ErrWriter. It + // goes to stdout with the help printed after it; run logs the error + // itself to stderr. Tests rely on the logger being the only writer to + // a run's stderr. + ErrWriter: mfa.Stdout, + Flags: commonFlags(), + StopOnNthArg: stopOnFirstArg(), + Action: func(_ context.Context, cmd *cli.Command) error { + if cmd.Args().Len() > 0 { + return fmt.Errorf("%w %q", errUnknownCommand, cmd.Args().First()) } - mfa.setVerbosity(c) + mfa.setVerbosity(cmd) mfa.printBanner() - return cli.ShowAppHelp(c) + return cli.ShowRootCommandHelp(cmd) }, Commands: []*cli.Command{ mfa.generateCommand(), @@ -431,7 +452,7 @@ func (mfa *CLIApp) run(args []string) { mfa.app.HideVersion = false - err := mfa.app.Run(args) + err := mfa.app.Run(context.Background(), args) if err != nil { mfa.exitCode = 1 diff --git a/script/build b/script/build index 3e2bd02..019297c 100755 --- a/script/build +++ b/script/build @@ -12,9 +12,7 @@ main() { # empty constant. version="$(git describe --tags --always --dirty 2>/dev/null || true)" [ -n "$version" ] || version="unknown" - go build -tags urfave_cli_no_docs \ - -ldflags "-X main.Gitrev=$version" \ - -o bin/mfer ./cmd/mfer + go build -ldflags "-X main.Gitrev=$version" -o bin/mfer ./cmd/mfer } main "$@"