diff --git a/.gitignore b/.gitignore index f2de7a720..480e4b267 100644 --- a/.gitignore +++ b/.gitignore @@ -24,7 +24,7 @@ rust-toolchain # Binaries for programs and plugins *.exe -*.exe~ +*.exe~* *.dll *.so *.dylib diff --git a/CHANGELOG.md b/CHANGELOG.md index bb6acfd4d..ced355be6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +## [v1.1.0](https://github.com/fastly/cli/releases/tag/v1.1.0) (2021-11-08) + +[Full Changelog](https://github.com/fastly/cli/compare/v1.0.1...v1.1.0) + +**Enhancements:** + +* Implement `--watch` flag for `compute serve` [#464](https://github.com/fastly/cli/pull/464) + ## [v1.0.1](https://github.com/fastly/cli/releases/tag/v1.0.1) (2021-11-08) [Full Changelog](https://github.com/fastly/cli/compare/v1.0.0...v1.0.1) diff --git a/Makefile b/Makefile index 3050977af..3848cb8c4 100644 --- a/Makefile +++ b/Makefile @@ -25,8 +25,8 @@ all: config tidy fmt vet staticcheck gosec test build install .PHONY: dependencies dependencies: - go get -v -u github.com/securego/gosec/cmd/gosec - go get -v -u honnef.co/go/tools/cmd/staticcheck + go install github.com/securego/gosec/cmd/gosec@latest + go install honnef.co/go/tools/cmd/staticcheck@latest .PHONY: tidy tidy: diff --git a/go.mod b/go.mod index 5bfa3c0cc..28d975242 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( github.com/Masterminds/semver/v3 v3.1.1 github.com/ajg/form v1.5.1 // indirect github.com/alecthomas/units v0.0.0-20210208195552-ff826a37aa15 // indirect + github.com/bep/debounce v1.2.0 github.com/blang/semver v3.5.1+incompatible github.com/davecgh/go-spew v1.1.1 // indirect github.com/dustinkirkland/golang-petname v0.0.0-20191129215211-8e5a1ed0cff0 @@ -13,6 +14,7 @@ require ( github.com/fastly/kingpin v2.1.12-0.20191105091915-95d230a53780+incompatible github.com/fatih/color v1.12.0 github.com/frankban/quicktest v1.13.1 // indirect + github.com/fsnotify/fsnotify v1.5.1 github.com/google/go-cmp v0.5.6 github.com/google/go-github/v38 v38.1.0 github.com/hashicorp/go-cleanhttp v0.5.2 // indirect @@ -27,7 +29,7 @@ require ( github.com/pierrec/lz4 v2.6.1+incompatible // indirect github.com/segmentio/textio v1.2.0 github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 - golang.org/x/sys v0.0.0-20210903071746-97244b99971b // indirect + golang.org/x/sys v0.0.0-20211103235746-7861aae1554b // indirect golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect gopkg.in/yaml.v2 v2.4.0 // indirect diff --git a/go.sum b/go.sum index ef69453a3..a5ab8ed10 100644 --- a/go.sum +++ b/go.sum @@ -9,6 +9,8 @@ github.com/alecthomas/units v0.0.0-20210208195552-ff826a37aa15/go.mod h1:OMCwj8V github.com/andybalholm/brotli v1.0.0/go.mod h1:loMXtMfwqflxFJPmdbJO0a3KNoPuLBgiu3qAvBg8x/Y= github.com/andybalholm/brotli v1.0.3 h1:fpcw+r1N1h0Poc1F/pHbW40cUm/lMEQslZtCkBQ0UnM= github.com/andybalholm/brotli v1.0.3/go.mod h1:fO7iG3H7G2nSZ7m0zPUDn85XEX2GTukHGRSepvi9Eig= +github.com/bep/debounce v1.2.0 h1:wXds8Kq8qRfwAOpAxHrJDbCXgC5aHSzgQb/0gKsHQqo= +github.com/bep/debounce v1.2.0/go.mod h1:H8yggRPQKLUhUoqrJC1bO2xNya7vanpDl7xR3ISbCJ0= github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ= github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= @@ -30,6 +32,8 @@ github.com/fatih/color v1.12.0 h1:mRhaKNwANqRgUBGKmnI5ZxEk7QXmjQeCcuYFMX2bfcc= github.com/fatih/color v1.12.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM= github.com/frankban/quicktest v1.13.1 h1:xVm/f9seEhZFL9+n5kv5XLrGwy6elc4V9v/XFY2vmd8= github.com/frankban/quicktest v1.13.1/go.mod h1:NeW+ay9A/U67EYXNFA1nPE8e/tnQv/09mUdL/ijj8og= +github.com/fsnotify/fsnotify v1.5.1 h1:mZcQUHVQUQWoPXXtuf9yuEXKudkV2sx1E06UadKWpgI= +github.com/fsnotify/fsnotify v1.5.1/go.mod h1:T3375wBYaZdLLcVNkcVbzGHY7f1l/uK5T5Ai1i3InKU= github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM= @@ -136,8 +140,8 @@ golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20210903071746-97244b99971b h1:3Dq0eVHn0uaQJmPO+/aYPI/fRMqdrVDbu7MQcku54gg= -golang.org/x/sys v0.0.0-20210903071746-97244b99971b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20211103235746-7861aae1554b h1:1VkfZQv42XQlA/jchYumAnv1UPo6RgF9rJFkTgZIxO4= +golang.org/x/sys v0.0.0-20211103235746-7861aae1554b/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1 h1:v+OssWQX+hTHEmOBgwxdZxK4zHq3yOs8F9J7mk0PY8E= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -163,3 +167,4 @@ gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= +# forcing CI step 'Install dependencies' to run as we updated how to install deps which results in current hash of go.sum to not be a cache miss and thus dep step isn't run diff --git a/pkg/app/run_test.go b/pkg/app/run_test.go index ed1eec8eb..65b11132e 100644 --- a/pkg/app/run_test.go +++ b/pkg/app/run_test.go @@ -573,9 +573,9 @@ COMMANDS compute build [] Build a Compute@Edge package locally - --name=NAME Package name - --language=LANGUAGE Language type --include-source Include source code in built package + --language=LANGUAGE Language type + --name=NAME Package name --skip-verification Skip verification steps and force build --timeout=TIMEOUT Timeout, in seconds, for the build compilation step @@ -645,6 +645,9 @@ COMMANDS --name=NAME Package name --skip-build Skip the build step --skip-verification Skip verification steps and force build + --timeout=TIMEOUT Timeout, in seconds, for the build compilation step + --watch Watch for file changes, then rebuild project and + restart local server compute update --version=VERSION --package=PACKAGE [] Update a package on a Fastly Compute@Edge service version diff --git a/pkg/commands/compute/build.go b/pkg/commands/compute/build.go index 67bd0e03b..ca604a699 100644 --- a/pkg/commands/compute/build.go +++ b/pkg/commands/compute/build.go @@ -38,12 +38,12 @@ type BuildCommand struct { // NOTE: these are public so that the "publish" composite command can set the // values appropriately before calling the Exec() function. - Force bool - IncludeSrc bool - Lang string - Manifest manifest.Data - PackageName string - Timeout int + IncludeSrc bool + Lang string + Manifest manifest.Data + PackageName string + SkipVerification bool + Timeout int } // NewBuildCommand returns a usable command registered under the parent. @@ -54,12 +54,12 @@ func NewBuildCommand(parent cmd.Registerer, client api.HTTPClient, globals *conf c.client = client c.CmdClause = parent.Command("build", "Build a Compute@Edge package locally") - // NOTE: when updating these flags, be sure to update the composite command: - // `compute publish`. - c.CmdClause.Flag("name", "Package name").StringVar(&c.PackageName) - c.CmdClause.Flag("language", "Language type").StringVar(&c.Lang) + // NOTE: when updating these flags, be sure to update the composite commands: + // `compute publish` and `compute serve`. c.CmdClause.Flag("include-source", "Include source code in built package").BoolVar(&c.IncludeSrc) - c.CmdClause.Flag("skip-verification", "Skip verification steps and force build").BoolVar(&c.Force) + c.CmdClause.Flag("language", "Language type").StringVar(&c.Lang) + c.CmdClause.Flag("name", "Package name").StringVar(&c.PackageName) + c.CmdClause.Flag("skip-verification", "Skip verification steps and force build").BoolVar(&c.SkipVerification) c.CmdClause.Flag("timeout", "Timeout, in seconds, for the build compilation step").IntVar(&c.Timeout) return &c @@ -139,7 +139,7 @@ func (c *BuildCommand) Exec(in io.Reader, out io.Writer) (err error) { return fmt.Errorf("unsupported language %s", lang) } - if !c.Force { + if !c.SkipVerification { progress.Step(fmt.Sprintf("Verifying local %s toolchain...", lang)) err = language.Verify(progress) diff --git a/pkg/commands/compute/compute_test.go b/pkg/commands/compute/compute_test.go index 810b0d334..a27973fdb 100644 --- a/pkg/commands/compute/compute_test.go +++ b/pkg/commands/compute/compute_test.go @@ -14,6 +14,7 @@ import ( "github.com/fastly/cli/pkg/api" "github.com/fastly/cli/pkg/commands/compute" "github.com/fastly/cli/pkg/commands/compute/manifest" + "github.com/fastly/cli/pkg/commands/update" "github.com/fastly/cli/pkg/config" "github.com/fastly/cli/pkg/testutil" "github.com/fastly/kingpin" @@ -59,10 +60,75 @@ func TestPublishFlagDivergence(t *testing.T) { } if !reflect.DeepEqual(expect, have) { - t.Fatalf("the flags between build/deploy and publish don't match") + t.Fatalf("the flags between build/deploy and publish don't match\n\nexpect: %+v\nhave: %+v\n\n", expect, have) } } +// TestServeFlagDivergence validates that the manually curated list of flags +// within the `compute serve` command doesn't fall out of sync with the +// `compute build` command as `compute serve` delegates to build. +func TestServeFlagDivergence(t *testing.T) { + var ( + cfg config.Data + data manifest.Data + ) + versioner := update.NewGitHub(update.GitHubOpts{ + Org: "fastly", + Repo: "viceroy", + Binary: "viceroy", + }) + acmd := kingpin.New("foo", "bar") + + rcmd := compute.NewRootCommand(acmd, &cfg) + bcmd := compute.NewBuildCommand(rcmd.CmdClause, client{}, &cfg, data) + scmd := compute.NewServeCommand(rcmd.CmdClause, &cfg, bcmd, versioner, data) + + buildFlags := getFlags(bcmd.CmdClause) + serveFlags := getFlags(scmd.CmdClause) + + var ( + expect = make(map[string]int) + have = make(map[string]int) + ) + + iter := buildFlags.MapRange() + for iter.Next() { + expect[iter.Key().String()] = 1 + } + + // Some flags on `compute serve` are unique to it. + // We only want to be sure serve contains all build flags. + ignoreServeFlags := []string{ + "addr", + "env", + "file", + "skip-build", + "watch", + } + + iter = serveFlags.MapRange() + for iter.Next() { + flag := iter.Key().String() + if !ignoreFlag(ignoreServeFlags, flag) { + have[flag] = 1 + } + } + + if !reflect.DeepEqual(expect, have) { + t.Fatalf("the flags between build and serve don't match\n\nexpect: %+v\nhave: %+v\n\n", expect, have) + } +} + +// ignoreFlag indicates if needle should be omitted from comparison. +func ignoreFlag(ignore []string, flag string) bool { + for _, i := range ignore { + if i == flag { + return true + } + } + return false +} + type client struct{} func (c client) Do(*http.Request) (*http.Response, error) { diff --git a/pkg/commands/compute/publish.go b/pkg/commands/compute/publish.go index 045b9d7f1..6c940fe80 100644 --- a/pkg/commands/compute/publish.go +++ b/pkg/commands/compute/publish.go @@ -17,11 +17,11 @@ type PublishCommand struct { deploy *DeployCommand // Build fields - name cmd.OptionalString - lang cmd.OptionalString - includeSrc cmd.OptionalBool - force cmd.OptionalBool - timeout cmd.OptionalInt + includeSrc cmd.OptionalBool + lang cmd.OptionalString + name cmd.OptionalString + skipVerification cmd.OptionalBool + timeout cmd.OptionalInt // Deploy fields acceptDefaults cmd.OptionalBool @@ -44,7 +44,7 @@ func NewPublishCommand(parent cmd.Registerer, globals *config.Data, build *Build c.CmdClause.Flag("name", "Package name").Action(c.name.Set).StringVar(&c.name.Value) c.CmdClause.Flag("language", "Language type").Action(c.lang.Set).StringVar(&c.lang.Value) c.CmdClause.Flag("include-source", "Include source code in built package").Action(c.includeSrc.Set).BoolVar(&c.includeSrc.Value) - c.CmdClause.Flag("skip-verification", "Skip verification steps and force build").Action(c.force.Set).BoolVar(&c.force.Value) + c.CmdClause.Flag("skip-verification", "Skip verification steps and force build").Action(c.skipVerification.Set).BoolVar(&c.skipVerification.Value) c.CmdClause.Flag("timeout", "Timeout, in seconds, for the build compilation step").Action(c.timeout.Set).IntVar(&c.timeout.Value) // Deploy flags @@ -71,17 +71,17 @@ func NewPublishCommand(parent cmd.Registerer, globals *config.Data, build *Build // the progress indicator. func (c *PublishCommand) Exec(in io.Reader, out io.Writer) (err error) { // Reset the fields on the BuildCommand based on PublishCommand values. - if c.name.WasSet { - c.build.PackageName = c.name.Value + if c.includeSrc.WasSet { + c.build.IncludeSrc = c.includeSrc.Value } if c.lang.WasSet { c.build.Lang = c.lang.Value } - if c.includeSrc.WasSet { - c.build.IncludeSrc = c.includeSrc.Value + if c.name.WasSet { + c.build.PackageName = c.name.Value } - if c.force.WasSet { - c.build.Force = c.force.Value + if c.skipVerification.WasSet { + c.build.SkipVerification = c.skipVerification.Value } if c.timeout.WasSet { c.build.Timeout = c.timeout.Value diff --git a/pkg/commands/compute/serve.go b/pkg/commands/compute/serve.go index 7e0068204..fa478272f 100644 --- a/pkg/commands/compute/serve.go +++ b/pkg/commands/compute/serve.go @@ -4,12 +4,17 @@ import ( "context" "fmt" "io" + "io/fs" + "log" "os" "os/exec" "path/filepath" "runtime" "strings" + "syscall" + "time" + "github.com/bep/debounce" "github.com/blang/semver" "github.com/fastly/cli/pkg/cmd" "github.com/fastly/cli/pkg/commands/compute/manifest" @@ -19,23 +24,29 @@ import ( fstexec "github.com/fastly/cli/pkg/exec" "github.com/fastly/cli/pkg/filesystem" "github.com/fastly/cli/pkg/text" + "github.com/fsnotify/fsnotify" ) // ServeCommand produces and runs an artifact from files on the local disk. type ServeCommand struct { cmd.Base - - addr string + manifest manifest.Data build *BuildCommand - env cmd.OptionalString - file string - force cmd.OptionalBool + viceroyVersioner update.Versioner + + // Build fields includeSrc cmd.OptionalBool lang cmd.OptionalString - manifest manifest.Data name cmd.OptionalString - skipBuild bool - viceroyVersioner update.Versioner + skipVerification cmd.OptionalBool + timeout cmd.OptionalInt + + // Serve fields + addr string + env cmd.OptionalString + file string + skipBuild bool + watch bool } // NewServeCommand returns a usable command registered under the parent. @@ -56,34 +67,24 @@ func NewServeCommand(parent cmd.Registerer, globals *config.Data, build *BuildCo c.CmdClause.Flag("language", "Language type").Action(c.lang.Set).StringVar(&c.lang.Value) c.CmdClause.Flag("name", "Package name").Action(c.name.Set).StringVar(&c.name.Value) c.CmdClause.Flag("skip-build", "Skip the build step").BoolVar(&c.skipBuild) - c.CmdClause.Flag("skip-verification", "Skip verification steps and force build").Action(c.force.Set).BoolVar(&c.force.Value) + c.CmdClause.Flag("skip-verification", "Skip verification steps and force build").Action(c.skipVerification.Set).BoolVar(&c.skipVerification.Value) + c.CmdClause.Flag("timeout", "Timeout, in seconds, for the build compilation step").Action(c.timeout.Set).IntVar(&c.timeout.Value) + c.CmdClause.Flag("watch", "Watch for file changes, then rebuild project and restart local server").BoolVar(&c.watch) return &c } // Exec implements the command interface. func (c *ServeCommand) Exec(in io.Reader, out io.Writer) (err error) { - if !c.skipBuild { - // Reset the fields on the BuildCommand based on ServeCommand values. - if c.name.WasSet { - c.build.PackageName = c.name.Value - } - if c.lang.WasSet { - c.build.Lang = c.lang.Value - } - if c.includeSrc.WasSet { - c.build.IncludeSrc = c.includeSrc.Value - } - if c.force.WasSet { - c.build.Force = c.force.Value - } + if c.skipBuild && c.watch { + return errors.ErrIncompatibleServeFlags + } - err = c.build.Exec(in, out) + if !c.skipBuild { + err = c.Build(in, out) if err != nil { return err } - - text.Break(out) } progress := text.NewProgress(out, c.Globals.Verbose()) @@ -96,16 +97,52 @@ func (c *ServeCommand) Exec(in io.Reader, out io.Writer) (err error) { progress.Step("Running local server...") progress.Done() - err = local(bin, c.file, progress, out, c.addr, c.env.Value, c.Globals.Verbose()) - if err != nil { - if err == errors.ErrSignalInterrupt || err == errors.ErrSignalKilled { - text.Break(out) - text.Info(out, "Local server stopped") - return nil + for { + err = local(bin, c.file, c.addr, c.env.Value, c.watch, c.Globals.Verbose(), progress, out) + if err != nil { + if err != errors.ErrViceroyRestart { + if err == errors.ErrSignalInterrupt || err == errors.ErrSignalKilled { + text.Info(out, "Local server stopped") + return nil + } + return err + } + + // Before restarting Viceroy we should rebuild. + err = c.Build(in, out) + if err != nil { + return err + } } + } +} + +// Build constructs and executes the build logic. +func (c *ServeCommand) Build(in io.Reader, out io.Writer) error { + // Reset the fields on the BuildCommand based on ServeCommand values. + if c.includeSrc.WasSet { + c.build.IncludeSrc = c.includeSrc.Value + } + if c.lang.WasSet { + c.build.Lang = c.lang.Value + } + if c.name.WasSet { + c.build.PackageName = c.name.Value + } + if c.skipVerification.WasSet { + c.build.SkipVerification = c.skipVerification.Value + } + if c.timeout.WasSet { + c.build.Timeout = c.timeout.Value + } + + err := c.build.Exec(in, out) + if err != nil { return err } + text.Break(out) + return nil } @@ -304,7 +341,7 @@ func setBinPerms(bin string) error { } // local spawns a subprocess that runs the compiled binary. -func local(bin string, file string, progress text.Progress, out io.Writer, addr string, env string, verbose bool) error { +func local(bin, file, addr, env string, watch, verbose bool, progress text.Progress, out io.Writer) error { if env != "" { env = "." + env } @@ -322,26 +359,160 @@ func local(bin string, file string, progress text.Progress, out io.Writer, addr text.Output(out, "Manifest: %s", manifest) } - cmd := fstexec.Streaming{ - Command: bin, - Args: args, - Env: os.Environ(), - Output: out, + cmd := &fstexec.Streaming{ + Args: args, + Command: bin, + Env: os.Environ(), + Output: out, + SignalCh: make(chan os.Signal, 1), } cmd.MonitorSignals() text.Break(out) + restart := make(chan bool) + if watch { + go watchFiles(cmd, out, restart) + } + + // NOTE: Once we run the viceroy executable, then it can be stopped by one of + // two separate mechanisms: + // + // 1. File modification + // 2. Explicit signal (SIGINT, SIGTERM etc). + // + // In the case of a signal (e.g. user presses Ctrl-c) the listener logic + // inside of (*fstexec.Streaming).MonitorSignals() will call + // (*fstexec.Streaming).Signal(signal os.Signal) to kill the process. + // + // In the case of a file modification the viceroy executable needs to first + // be killed (handled by the watchFiles() function) and then we can stop the + // signal listeners (handled below by sending a message to cmd.SignalCh). + // + // If we don't tell the signal listening channel to close, then the restart + // of the viceroy executable will cause the user to end up with N number of + // listeners. This will result in a "os: process already finished" error when + // we do finally come to stop the `serve` command (e.g. user presses Ctrl-c). + // How big an issue this is depends on how many file modifications a user + // makes, because having lots of signal listeners could exhaust resources. if err := cmd.Exec(); err != nil { e := strings.TrimSpace(err.Error()) if strings.Contains(e, "interrupt") { return errors.ErrSignalInterrupt } if strings.Contains(e, "killed") { - return errors.ErrSignalKilled + select { + case <-restart: + cmd.SignalCh <- syscall.SIGTERM + return errors.ErrViceroyRestart + case <-time.After(1 * time.Second): + return errors.ErrSignalKilled + } } return err } return nil } + +// watchFiles watches the 'src' directory and restarts the viceroy executable +// when changes are detected. +func watchFiles(cmd *fstexec.Streaming, out io.Writer, restart chan<- bool) { + watcher, err := fsnotify.NewWatcher() + if err != nil { + log.Fatal(err) + } + defer watcher.Close() + + done := make(chan bool) + debounced := debounce.New(1 * time.Second) + eventHandler := func(modifiedFile string, op fsnotify.Op) { + action := "modified" + switch op { + case fsnotify.Create: + action = "created" + case fsnotify.Remove: + action = "removed" + case fsnotify.Rename: + action = "renamed" + } + + text.Info(out, "Restarting: %s has been %s", modifiedFile, action) + text.Break(out) + + // NOTE: We force closing the watcher by pushing true into a done channel. + // We do this because if we didn't, then we'd get an error after one + // restart of the viceroy executable: "os: process already finished". + // + // This error happens happens because the compute.watchFiles() function is + // run in a goroutine and so it will keep running with a copy of the + // fstexec.Streaming command instance that wraps a process which has + // already been terminated. + done <- true + + // NOTE: To be able to force both the current viceroy process signal listener + // to close, and to restart the viceroy executable, we need to kill the + // process and also send 'true' to a restart channel. + // + // If we only sent a message to the restart channel, but didn't terminate + // the process, then we'd end up in a deadlock because we wouldn't be able + // to take a message from the restart channel inside the local() function + // because we need to have the process terminate first in order for us to + // execute the flushing of channel messages. + // + // When we stop the signal listener it will internally try to kill the + // process and discover it has already been killed and return an error: + // `os: process already finished`. This is why we don't do error handling + // within (*fstexec.Streaming).MonitorSignalsAsync() as the process could + // well be killed already when a user is doing local development with the + // --watch flag. The obvious downside to this logic flow is that if the + // user is running `compute serve` just to validate the program once, then + // there might be an unhandled error when they press Ctrl-c to stop the + // serve command from blocking their terminal. That said, this is unlikely + // and is a low risk concern. + err := cmd.Signal(os.Kill) + if err != nil { + log.Fatal(err) + } + + restart <- true + } + + go func() { + for { + select { + case event, ok := <-watcher.Events: + if !ok { + return + } + debounced(func() { + eventHandler(event.Name, event.Op) + }) + case err, ok := <-watcher.Errors: + if !ok { + return + } + text.Output(out, "error event while watching files: %v", err) + } + } + }() + + root := "src" + + filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { + if err != nil { + log.Fatal(err) + } + if entry.IsDir() { + err = watcher.Add(path) + if err != nil { + log.Fatal(err) + } + } + return nil + }) + + text.Info(out, "Watching ./%s for changes", root) + text.Break(out) + <-done +} diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index ded566765..b34770864 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -8,6 +8,19 @@ var ErrSignalInterrupt = fmt.Errorf("a SIGINT was received") // ErrSignalKilled means a SIGTERM was received. var ErrSignalKilled = fmt.Errorf("a SIGTERM was received") +// ErrViceroyRestart means the viceroy binary needs to be restarted due to a +// file modification noticed while running `compute serve --watch`. +var ErrViceroyRestart = fmt.Errorf("a RESTART was initiated") + +// ErrIncompatibleServeFlags means no --skip-build can't be used with --watch +// because it defeats the purpose of --watch which is designed to restart +// Viceroy whenever changes are detected (those changes would not be seen if we +// allowed --skip-build with --watch). +var ErrIncompatibleServeFlags = RemediationError{ + Inner: fmt.Errorf("--skip-build shouldn't be used with --watch"), + Remediation: ComputeServeRemediation, +} + // ErrNoToken means no --token has been provided. var ErrNoToken = RemediationError{ Inner: fmt.Errorf("no token provided"), diff --git a/pkg/errors/remediation_error.go b/pkg/errors/remediation_error.go index 5c042ede6..5b68a9c94 100644 --- a/pkg/errors/remediation_error.go +++ b/pkg/errors/remediation_error.go @@ -117,3 +117,10 @@ var ComputeInitRemediation = strings.Join([]string{ "Run `fastly compute init` to ensure a correctly configured manifest.", "See more at https://developer.fastly.com/reference/fastly-toml/", }, " ") + +// ComputeServeRemediation suggests re-running `compute serve` with one of the +// incompatible flags removed. +var ComputeServeRemediation = strings.Join([]string{ + "The --watch flag enables hot reloading of your project to support a faster feedback loop during local development, and subsequently conflicts with the --skip-build flag which avoids rebuilding your project altogether.", + "Remove one of the flags based on the outcome you require.", +}, " ") diff --git a/pkg/exec/exec.go b/pkg/exec/exec.go index 1d3c2896e..936a7ab9b 100644 --- a/pkg/exec/exec.go +++ b/pkg/exec/exec.go @@ -5,7 +5,6 @@ import ( "context" "fmt" "io" - "log" "os" "os/exec" "os/signal" @@ -19,43 +18,44 @@ import ( // compute commands can use this to standardize the flow control for each // compiler toolchain. type Streaming struct { - Command string - Args []string - Env []string - Output io.Writer - Timeout time.Duration - process *os.Process + Args []string + Command string + Env []string + Output io.Writer + Process *os.Process + SignalCh chan os.Signal + Timeout time.Duration } // MonitorSignals spawns a goroutine that configures signal handling so that // the long running subprocess can be killed using SIGINT/SIGTERM. func (s *Streaming) MonitorSignals() { - go monitorSignals(s) + go s.MonitorSignalsAsync() } -func monitorSignals(s *Streaming) { - sig := make(chan os.Signal, 1) - +func (s *Streaming) MonitorSignalsAsync() { signals := []os.Signal{ syscall.SIGINT, syscall.SIGTERM, } - signal.Notify(sig, signals...) + signal.Notify(s.SignalCh, signals...) - <-sig - signal.Stop(sig) + <-s.SignalCh + signal.Stop(s.SignalCh) - err := s.Signal(os.Kill) - if err != nil { - log.Fatal(err) - } + // NOTE: We don't do error handling here because the user might be doing local + // development with the --watch flag and that workflow will have already + // killed the process. The reason this line still exists is for users running + // their application locally without the --watch flag and who then execute + // Ctrl-C to kill the process. + s.Signal(os.Kill) } // Exec executes the compiler command and pipes the child process stdout and // stderr output to the supplied io.Writer, it waits for the command to exit // cleanly or returns an error. -func (s Streaming) Exec() error { +func (s *Streaming) Exec() error { // Construct the command with given arguments and environment. var cmd *exec.Cmd if s.Timeout > 0 { @@ -75,16 +75,21 @@ func (s Streaming) Exec() error { } cmd.Env = append(os.Environ(), s.Env...) - // Store off Process so it can be killed by signals - //lint:ignore SA4005 because it doesn't fail on macOS but does when run in CI. - s.process = cmd.Process - // Pipe the child process stdout and stderr to our own output writer. var stderrBuf bytes.Buffer cmd.Stdout = s.Output cmd.Stderr = io.MultiWriter(s.Output, &stderrBuf) - if err := cmd.Run(); err != nil { + if err := cmd.Start(); err != nil { + return err + } + + // Store off os.Process so it can be killed by signal listener. + // + // NOTE: cmd.Process is nil until exec.Start() returns successfully. + s.Process = cmd.Process + + if err := cmd.Wait(); err != nil { var ctx string if stderrBuf.Len() > 0 { ctx = fmt.Sprintf(":\n%s", strings.TrimSpace(stderrBuf.String())) @@ -98,9 +103,9 @@ func (s Streaming) Exec() error { } // Signal enables spawned subprocess to accept given signal. -func (s Streaming) Signal(signal os.Signal) error { - if s.process != nil { - err := s.process.Signal(signal) +func (s *Streaming) Signal(signal os.Signal) error { + if s.Process != nil { + err := s.Process.Signal(signal) if err != nil { return err }