From 19675ff11ba952426cb8a077f8cce72cd50f5f80 Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Mon, 12 Apr 2021 11:54:20 +0100 Subject: [PATCH 1/6] Warn users about non empty directories. --- pkg/compute/init.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pkg/compute/init.go b/pkg/compute/init.go index 970ff7f80..436ddfd7c 100644 --- a/pkg/compute/init.go +++ b/pkg/compute/init.go @@ -90,6 +90,16 @@ func (c *InitCommand) Exec(in io.Reader, out io.Writer) (err error) { text.Output(out, "Press ^C at any time to quit.") text.Break(out) + files, err := os.ReadDir(".") + if err != nil { + return err + } + + if len(files) > 0 { + text.Warning(out, "The current directory isn't empty. This might prevent the CLI from building your project if there are conflicting language configuration files.") + text.Break(out) + } + var progress text.Progress if c.Globals.Verbose() { progress = text.NewVerboseProgress(out) From f187554c43de727687b6cf38bfc2b6d6329cb85a Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Mon, 12 Apr 2021 13:18:39 +0100 Subject: [PATCH 2/6] Test. --- pkg/compute/compute_integration_test.go | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/pkg/compute/compute_integration_test.go b/pkg/compute/compute_integration_test.go index b41afe1ef..5343ca0d6 100644 --- a/pkg/compute/compute_integration_test.go +++ b/pkg/compute/compute_integration_test.go @@ -418,6 +418,37 @@ func TestInit(t *testing.T) { "Updating package manifest...", }, }, + { + name: "non empty directory", + args: []string{"compute", "init"}, + configFile: config.File{ + User: config.User{ + Token: "123", + Email: "test@example.com", + }, + StarterKits: config.StarterKitLanguages{ + Rust: []config.StarterKit{ + { + Name: "Default", + Path: "https://github.com/fastly/compute-starter-kit-rust-default.git", + Branch: "0.6.0", + }, + }, + }, + }, + api: mock.API{ + GetTokenSelfFn: tokenOK, + GetUserFn: getUserOk, + CreateServiceFn: createServiceOK, + CreateDomainFn: createDomainOK, + CreateBackendFn: createBackendOK, + }, + manifestIncludes: `authors = ["test@example.com"]`, + wantOutput: []string{ + "The current directory isn't empty.", + }, + manifest: `name = "test"`, + }, { name: "with default name inferred from directory", args: []string{"compute", "init"}, From 1fbea9ec89baad9c90d46e0fb6d1ee2a9f5bb3ed Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Mon, 12 Apr 2021 16:11:41 +0100 Subject: [PATCH 3/6] Implement an interactive prompt for non empty directory flows. --- pkg/compute/init.go | 45 ++++++++++++++++++++++++++++++--- pkg/errors/remediation_error.go | 6 +++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/pkg/compute/init.go b/pkg/compute/init.go index 436ddfd7c..2d32cae77 100644 --- a/pkg/compute/init.go +++ b/pkg/compute/init.go @@ -90,14 +90,16 @@ func (c *InitCommand) Exec(in io.Reader, out io.Writer) (err error) { text.Output(out, "Press ^C at any time to quit.") text.Break(out) - files, err := os.ReadDir(".") + cont, err := verifyDirectory(out, in) if err != nil { return err } - if len(files) > 0 { - text.Warning(out, "The current directory isn't empty. This might prevent the CLI from building your project if there are conflicting language configuration files.") - text.Break(out) + if !cont { + return errors.RemediationError{ + Inner: fmt.Errorf("project directory not empty"), + Remediation: errors.ExistingDirRemediation, + } } var progress text.Progress @@ -501,6 +503,41 @@ func (c *InitCommand) Exec(in io.Reader, out io.Writer) (err error) { return nil } +// verifyDirectory indicates if the user wants to continue with the execution +// flow when presented with a prompt that suggests the current directory isn't +// empty. +func verifyDirectory(out io.Writer, in io.Reader) (bool, error) { + files, err := os.ReadDir(".") + if err != nil { + return false, err + } + + if len(files) > 0 { + dir, err := os.Getwd() + if err != nil { + return false, err + } + + label := fmt.Sprintf("The current directory isn't empty. Are you sure you want to initialize a Compute@Edge project in %s? [y/n] ", dir) + cont, err := text.Input(out, label, in) + if err != nil { + return false, fmt.Errorf("error reading input %w", err) + } + + contl := strings.ToLower(cont) + + if contl == "n" || contl == "no" { + return false, nil + } + + if contl == "y" || contl == "yes" { + return true, nil + } + } + + return true, nil +} + func verifyDestination(path string, verbose io.Writer) (abspath string, err error) { abspath, err = filepath.Abs(path) if err != nil { diff --git a/pkg/errors/remediation_error.go b/pkg/errors/remediation_error.go index a77b54b23..6644bade8 100644 --- a/pkg/errors/remediation_error.go +++ b/pkg/errors/remediation_error.go @@ -80,3 +80,9 @@ var BugRemediation = strings.Join([]string{ var ServiceIDRemediation = strings.Join([]string{ "Please provide one via the --service-id flag or within your package manifest", }, " ") + +// ExistingDirRemediation suggests moving to another directory and retrying. +var ExistingDirRemediation = strings.Join([]string{ + "Please create a new directory and initialize a new project using:", + "`fastly compute init`.", +}, " ") From d532d8a8e2c5ff725342f7aab299ce920ea30382 Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Mon, 12 Apr 2021 16:32:07 +0100 Subject: [PATCH 4/6] Ensure non TTY can ignore non-empty project directory. --- pkg/compute/compute_integration_test.go | 9 +++------ pkg/compute/init.go | 26 ++++++++++++++----------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/pkg/compute/compute_integration_test.go b/pkg/compute/compute_integration_test.go index 5343ca0d6..e068d3fab 100644 --- a/pkg/compute/compute_integration_test.go +++ b/pkg/compute/compute_integration_test.go @@ -336,7 +336,7 @@ func TestInit(t *testing.T) { }, { name: "with existing package manifest", - args: []string{"compute", "init"}, + args: []string{"compute", "init", "--force"}, // --force will ignore that the directory isn't empty configFile: config.File{ User: config.User{ Token: "123", @@ -443,11 +443,8 @@ func TestInit(t *testing.T) { CreateDomainFn: createDomainOK, CreateBackendFn: createBackendOK, }, - manifestIncludes: `authors = ["test@example.com"]`, - wantOutput: []string{ - "The current directory isn't empty.", - }, - manifest: `name = "test"`, + wantError: "project directory not empty", + manifest: `name = "test"`, // causes a file to be created as part of test setup }, { name: "with default name inferred from directory", diff --git a/pkg/compute/init.go b/pkg/compute/init.go index 2d32cae77..76ebd11b9 100644 --- a/pkg/compute/init.go +++ b/pkg/compute/init.go @@ -41,16 +41,17 @@ var ( // InitCommand initializes a Compute@Edge project package on the local machine. type InitCommand struct { common.Base - client api.HTTPClient - manifest manifest.Data - language string - from string - branch string - tag string - path string - domain string - backend string - backendPort uint + client api.HTTPClient + manifest manifest.Data + language string + from string + branch string + tag string + path string + domain string + backend string + backendPort uint + forceNonEmpty bool } // NewInitCommand returns a usable command registered under the parent. @@ -73,6 +74,7 @@ func NewInitCommand(parent common.Registerer, client api.HTTPClient, globals *co c.CmdClause.Flag("domain", "The name of the domain associated to the package").StringVar(&c.domain) c.CmdClause.Flag("backend", "A hostname, IPv4, or IPv6 address for the package backend").StringVar(&c.backend) c.CmdClause.Flag("backend-port", "A port number for the package backend").UintVar(&c.backendPort) + c.CmdClause.Flag("force", "Whether a non-empty project directory should be ignored").BoolVar(&c.forceNonEmpty) return &c } @@ -95,7 +97,7 @@ func (c *InitCommand) Exec(in io.Reader, out io.Writer) (err error) { return err } - if !cont { + if !c.forceNonEmpty && !cont { return errors.RemediationError{ Inner: fmt.Errorf("project directory not empty"), Remediation: errors.ExistingDirRemediation, @@ -533,6 +535,8 @@ func verifyDirectory(out io.Writer, in io.Reader) (bool, error) { if contl == "y" || contl == "yes" { return true, nil } + + return false, nil } return true, nil From 63e1adf152af3de6e6b4f589baedba8406e3012f Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Mon, 12 Apr 2021 16:35:51 +0100 Subject: [PATCH 5/6] Clarify default behaviour. --- pkg/compute/init.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/compute/init.go b/pkg/compute/init.go index 76ebd11b9..96fecc9b8 100644 --- a/pkg/compute/init.go +++ b/pkg/compute/init.go @@ -536,6 +536,8 @@ func verifyDirectory(out io.Writer, in io.Reader) (bool, error) { return true, nil } + // NOTE: be defensive and default to short-circuiting the execution flow if + // the input is unrecognised. return false, nil } From 74f1535d82656cc2516e1645db616620f4673eb8 Mon Sep 17 00:00:00 2001 From: Mark McDonnell Date: Mon, 12 Apr 2021 16:47:08 +0100 Subject: [PATCH 6/6] Fix test for full help output. --- pkg/app/run_test.go | 2 ++ pkg/compute/init.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/app/run_test.go b/pkg/app/run_test.go index 5c94b972f..27b35b5d9 100644 --- a/pkg/app/run_test.go +++ b/pkg/app/run_test.go @@ -287,6 +287,8 @@ COMMANDS package backend --backend-port=BACKEND-PORT A port number for the package backend + --force Skip non-empty directory verification step + and force new project creation compute build [] Build a Compute@Edge package locally diff --git a/pkg/compute/init.go b/pkg/compute/init.go index 96fecc9b8..9a95469b8 100644 --- a/pkg/compute/init.go +++ b/pkg/compute/init.go @@ -74,7 +74,7 @@ func NewInitCommand(parent common.Registerer, client api.HTTPClient, globals *co c.CmdClause.Flag("domain", "The name of the domain associated to the package").StringVar(&c.domain) c.CmdClause.Flag("backend", "A hostname, IPv4, or IPv6 address for the package backend").StringVar(&c.backend) c.CmdClause.Flag("backend-port", "A port number for the package backend").UintVar(&c.backendPort) - c.CmdClause.Flag("force", "Whether a non-empty project directory should be ignored").BoolVar(&c.forceNonEmpty) + c.CmdClause.Flag("force", "Skip non-empty directory verification step and force new project creation").BoolVar(&c.forceNonEmpty) return &c }