[engine] Prioritize version warning over unknown fields
If `shac.textproto` specifies a `min_shac_version`, check that shac is
running at that version before validating the remaining fields of
`shac.textproto` in case one of the fields is unknown only because it
was introduced in a later version of shac.
Change-Id: I253468cf81e1a7d7a0488942a6e691ecbb8c9aa1
Reviewed-on: https://fuchsia-review.googlesource.com/c/shac-project/shac/+/921276
Reviewed-by: Marc-Antoine Ruel <maruel@google.com>
Fuchsia-Auto-Submit: Oliver Newman <olivernewman@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
diff --git a/internal/engine/config.go b/internal/engine/config.go
index d90956d..7320853 100644
--- a/internal/engine/config.go
+++ b/internal/engine/config.go
@@ -24,21 +24,29 @@
"strings"
)
+func (doc *Document) CheckVersion() error {
+ if doc.MinShacVersion == "" {
+ return nil
+ }
+ v := parseVersion(doc.MinShacVersion)
+ if v == nil || len(v) > len(Version) {
+ return errors.New("min_shac_version is invalid")
+ }
+ for i := range v {
+ if v[i] > Version[i] {
+ return fmt.Errorf("min_shac_version specifies unsupported version %q, running %s", doc.MinShacVersion, Version)
+ }
+ if v[i] < Version[i] {
+ break
+ }
+ }
+ return nil
+}
+
// Validate verifies a shac.textproto document is valid.
func (doc *Document) Validate() error {
- if doc.MinShacVersion != "" {
- v := parseVersion(doc.MinShacVersion)
- if v == nil || len(v) > len(Version) {
- return errors.New("min_shac_version is invalid")
- }
- for i := range v {
- if v[i] > Version[i] {
- return fmt.Errorf("min_shac_version specifies unsupported version %q, running %d.%d.%d", doc.MinShacVersion, Version[0], Version[1], Version[2])
- }
- if v[i] < Version[i] {
- break
- }
- }
+ if err := doc.CheckVersion(); err != nil {
+ return err
}
deps := map[string]string{}
aliases := map[string]struct{}{}
diff --git a/internal/engine/config_test.go b/internal/engine/config_test.go
index efedfdb..1ce8874 100644
--- a/internal/engine/config_test.go
+++ b/internal/engine/config_test.go
@@ -268,10 +268,8 @@
"min_shac_version: \"1000\"\n",
func() string {
return fmt.Sprintf(
- "min_shac_version specifies unsupported version \"1000\", running %d.%d.%d",
- Version[0],
- Version[1],
- Version[2],
+ "min_shac_version specifies unsupported version \"1000\", running %s",
+ Version,
)
}(),
},
diff --git a/internal/engine/run.go b/internal/engine/run.go
index 0851e2d..705c07c 100644
--- a/internal/engine/run.go
+++ b/internal/engine/run.go
@@ -222,7 +222,20 @@
var b []byte
doc := Document{}
if b, err = os.ReadFile(p); err == nil {
- if err = prototext.Unmarshal(b, &doc); err != nil {
+ // 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
+ // long as min_shac_version is set appropriately).
+ opts := prototext.UnmarshalOptions{DiscardUnknown: true}
+ if err = opts.Unmarshal(b, &doc); err != nil {
+ return err
+ }
+ if err = doc.CheckVersion(); err != nil {
+ return err
+ }
+ // Parse the config file again, failing on any unknown fields.
+ opts.DiscardUnknown = false
+ if err = opts.Unmarshal(b, &doc); err != nil {
return err
}
} else if !errors.Is(err, fs.ErrNotExist) {
@@ -694,15 +707,3 @@
}
return err
}
-
-func parseVersion(s string) []int {
- var out []int
- for _, x := range strings.Split(s, ".") {
- i, err := strconv.Atoi(x)
- if err != nil {
- return nil
- }
- out = append(out, i)
- }
- return out
-}
diff --git a/internal/engine/run_test.go b/internal/engine/run_test.go
index 5c09186..15146ee 100644
--- a/internal/engine/run_test.go
+++ b/internal/engine/run_test.go
@@ -80,8 +80,34 @@
Dir: root,
}
}(),
- // The encoding is not deterministic.
- "...: unknown field: bad",
+ //
+ "...unexpected EOF",
+ },
+ {
+ "config file with unknown field",
+ func() Options {
+ root := t.TempDir()
+ writeFile(t, root, "shac.star", ``)
+ writeFile(t, root, "shac.textproto", "unknown_field: true\n")
+ return Options{
+ Dir: root,
+ }
+ }(),
+ "...unknown field: unknown_field",
+ },
+ {
+ "config file with newer min_shac_version and unknown field",
+ func() Options {
+ root := t.TempDir()
+ writeFile(t, root, "shac.star", ``)
+ writeFile(t, root, "shac.textproto",
+ "min_shac_version: \"2000\"\n"+
+ "unknown_field: true\n")
+ return Options{
+ Dir: root,
+ }
+ }(),
+ fmt.Sprintf("min_shac_version specifies unsupported version \"2000\", running %s", Version),
},
{
"no shac.star file",
diff --git a/internal/engine/version.go b/internal/engine/version.go
index 9433863..e88d772 100644
--- a/internal/engine/version.go
+++ b/internal/engine/version.go
@@ -14,9 +14,33 @@
package engine
+import (
+ "fmt"
+ "strconv"
+ "strings"
+)
+
+type shacVersion [3]int
+
var (
// Version is the current tool version.
//
// TODO(maruel): Add proper version, preferably from git tag.
- Version = [...]int{0, 1, 7}
+ Version = shacVersion{0, 1, 8}
)
+
+func (v shacVersion) String() string {
+ return fmt.Sprintf("%d.%d.%d", v[0], v[1], v[2])
+}
+
+func parseVersion(s string) []int {
+ var out []int
+ for _, x := range strings.Split(s, ".") {
+ i, err := strconv.Atoi(x)
+ if err != nil {
+ return nil
+ }
+ out = append(out, i)
+ }
+ return out
+}
