[engine] Improve error messages when run in invalid directory

1. --var flags are not validated until after shac determines that the
   working directory is valid (i.e. contains a shac.star file).
2. If a shac.star file cannot be discovered via git, print the expected
   location of the shac.star file.
3. If a --var flag is specified but no shac.textproto file exists, use a
   slightly different error message to make it more clear that
   shac.textproto doesn't exist yet.

Change-Id: I6bed59f60b73e9ecff0caec97c152f2f7ea94fb7
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/926852
Reviewed-by: Anthony Fandrianto <[email protected]>
Fuchsia-Auto-Submit: Oliver Newman <[email protected]>
Commit-Queue: Auto-Submit <[email protected]>

internal/engine/run.go

@@ -224,7 +224,9 @@ func runInner(ctx context.Context, o *Options, tmpdir string) error {
 	}
 	var b []byte
 	doc := Document{}
+	configExists := false
 	if b, err = os.ReadFile(absConfig); err == nil {
+		configExists = true
 		// First parse the config file ignoring unknown fields and check only
 		// min_shac_version, so users get an "unsupported version" error if they
 		// set fields that are only available in a later version of shac (as
@@ -277,17 +279,6 @@ func runInner(ctx context.Context, o *Options, tmpdir string) error {
 		scm = &cachingSCM{scm: scm}
 	}
 
-	vars := make(map[string]string)
-	for _, v := range doc.Vars {
-		vars[v.Name] = v.Default
-	}
-	for name, value := range o.Vars {
-		if _, ok := vars[name]; !ok {
-			return fmt.Errorf("var not declared in %s: %s", config, name)
-		}
-		vars[name] = value
-	}
-
 	pkgMgr := NewPackageManager(tmpdir)
 	packages, err := pkgMgr.RetrievePackages(ctx, root, &doc)
 	if err != nil {
@@ -304,7 +295,28 @@ func runInner(ctx context.Context, o *Options, tmpdir string) error {
 		packages: packages,
 	}
 
-	newState := func(scm scmCheckout, subdir string, idx int) *shacState {
+	var vars map[string]string
+
+	newState := func(scm scmCheckout, subdir string, idx int) (*shacState, error) {
+		// Lazy-load vars only once a shac.star file is detected, so that errors
+		// about missing shac.star files are prioritized over var validation
+		// errors.
+		if vars == nil {
+			vars = make(map[string]string)
+			for _, v := range doc.Vars {
+				vars[v.Name] = v.Default
+			}
+			for name, value := range o.Vars {
+				if _, ok := vars[name]; !ok {
+					if configExists {
+						return nil, fmt.Errorf("var not declared in %s: %s", config, name)
+					}
+					return nil, fmt.Errorf("var must be declared in a %s file: %s", config, name)
+				}
+				vars[name] = value
+			}
+		}
+
 		if subdir != "" {
 			normalized := subdir + "/"
 			if subdir == "." {
@@ -327,7 +339,7 @@ func runInner(ctx context.Context, o *Options, tmpdir string) error {
 			writableRoot:   doc.WritableRoot,
 			vars:           vars,
 			passthroughEnv: doc.PassthroughEnv,
-		}
+		}, nil
 	}
 	var shacStates []*shacState
 	if o.Recurse {
@@ -363,13 +375,27 @@ func runInner(ctx context.Context, o *Options, tmpdir string) error {
 			}
 		}
 		if len(subdirs) == 0 {
-			return fmt.Errorf("no %s files found", entryPoint)
+			return fmt.Errorf("no %s files found in %s", entryPoint, root)
 		}
 		for i, s := range subdirs {
-			shacStates = append(shacStates, newState(scm, s, i))
+			state, err := newState(scm, s, i)
+			if err != nil {
+				return err
+			}
+			shacStates = append(shacStates, state)
 		}
 	} else {
-		shacStates = []*shacState{newState(scm, "", 0)}
+		if _, err := os.Stat(filepath.Join(root, entryPoint)); err != nil {
+			if errors.Is(err, fs.ErrNotExist) {
+				return fmt.Errorf("no %s file in repository root: %s", entryPoint, root)
+			}
+			return err
+		}
+		state, err := newState(scm, "", 0)
+		if err != nil {
+			return err
+		}
+		shacStates = []*shacState{state}
 	}
 
 	// Parse the starlark files. Run everything from our errgroup.

internal/engine/run_test.go

@@ -113,19 +113,32 @@ func TestRun_Fail(t *testing.T) {
 		{
 			"no shac.star file",
 			Options{
-				Dir: t.TempDir(),
+				Dir:        scratchDir,
+				EntryPoint: "entrypoint.star",
 			},
-			// TODO(olivernewman): This error should be more specific, like "no
-			// shac.star file found".
-			"shac.star not found",
+			fmt.Sprintf("no entrypoint.star file in repository root: %s", scratchDir),
+		},
+		{
+			"no shac.star file and invalid vars",
+			Options{
+				Dir:        scratchDir,
+				EntryPoint: "entrypoint.star",
+				Vars: map[string]string{
+					"this_is_an_invalid_var": "foo",
+				},
+			},
+			// Missing entrypoint file errors should be prioritized over invalid
+			// var errors.
+			fmt.Sprintf("no entrypoint.star file in repository root: %s", scratchDir),
 		},
 		{
 			"no shac.star files (recursive)",
 			Options{
-				Dir:     t.TempDir(),
-				Recurse: true,
+				Dir:        scratchDir,
+				Recurse:    true,
+				EntryPoint: "entrypoint.star",
 			},
-			"no shac.star files found",
+			fmt.Sprintf("no entrypoint.star files found in %s", scratchDir),
 		},
 		{
 			"nonexistent directory",
@@ -153,6 +166,20 @@ func TestRun_Fail(t *testing.T) {
 			},
 			"var not declared in shac.textproto: unknown_var",
 		},
+		{
+			"invalid var with no config file",
+			func() Options {
+				root := t.TempDir()
+				writeFile(t, root, "shac.star", ``)
+				return Options{
+					Dir: root,
+					Vars: map[string]string{
+						"unknown_var": "",
+					},
+				}
+			}(),
+			"var must be declared in a shac.textproto file: unknown_var",
+		},
 	}
 	for i := range data {
 		i := i

internal/engine/version.go

@@ -26,7 +26,7 @@ var (
 	// Version is the current tool version.
 	//
 	// TODO(maruel): Add proper version, preferably from git tag.
-	Version = shacVersion{0, 1, 10}
+	Version = shacVersion{0, 1, 11}
 )
 
 func (v shacVersion) String() string {