diff --git a/go.mod b/go.mod index 8744ad212eb..535e7d7ac92 100644 --- a/go.mod +++ b/go.mod @@ -36,12 +36,14 @@ require ( github.com/spf13/cobra v1.7.0 github.com/spf13/pflag v1.0.5 github.com/stretchr/testify v1.8.4 + github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a github.com/wk8/go-ordered-map/v2 v2.1.8 github.com/zealic/go2node v0.1.0 go.jetpack.io/pkg v0.0.0-20231012130632-77dcd111db2e golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17 golang.org/x/mod v0.12.0 golang.org/x/sync v0.3.0 + golang.org/x/tools v0.6.0 gopkg.in/natefinch/lumberjack.v2 v2.2.1 gopkg.in/yaml.v3 v3.0.1 ) @@ -116,7 +118,6 @@ require ( golang.org/x/sys v0.12.0 // indirect golang.org/x/term v0.12.0 // indirect golang.org/x/text v0.13.0 // indirect - golang.org/x/tools v0.6.0 // indirect google.golang.org/appengine v1.6.8 // indirect google.golang.org/protobuf v1.31.0 // indirect ) diff --git a/go.sum b/go.sum index 3a51fb8560a..0857ff19719 100644 --- a/go.sum +++ b/go.sum @@ -323,6 +323,8 @@ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a h1:SJy1Pu0eH1C29XwJucQo73FrleVK6t4kYz4NVhp34Yw= +github.com/tailscale/hujson v0.0.0-20221223112325-20486734a56a/go.mod h1:DFSS3NAGHthKo1gTlmEcSBiZrRJXi28rLNd/1udP1c8= github.com/therootcompany/xz v1.0.1 h1:CmOtsn1CbtmyYiusbfmhmkpAAETj0wBIH6kCYaX+xzw= github.com/therootcompany/xz v1.0.1/go.mod h1:3K3UH1yCKgBneZYhuQUvJ9HPD19UEXEI0BWbMn8qNMY= github.com/ulikunitz/xz v0.5.6/go.mod h1:2bypXElzHzzJZwzH67Y6wb67pO62Rzfn7BSiF4ABRW8= diff --git a/internal/devconfig/ast.go b/internal/devconfig/ast.go new file mode 100644 index 00000000000..1da4a7f6996 --- /dev/null +++ b/internal/devconfig/ast.go @@ -0,0 +1,282 @@ +package devconfig + +import ( + "bytes" + "slices" + + "github.com/tailscale/hujson" +) + +// configAST is a hujson syntax tree that represents a devbox.json +// configuration. An AST allows the CLI to modify specific parts of a user's +// devbox.json instead of overwriting the entire file. This is important +// because a devbox.json can have user comments that must be preserved when +// saving changes. +// +// - Unmarshalling is still done with encoding/json. +// - Marshalling is done by calling configAST.root.Pack to encode the AST as +// hujson/JWCC. Therefore, any changes to a Config struct will NOT +// automatically be marshaled back to JSON. Support for modifying a part of +// the JSON must be explicitly implemented in configAST. +// - Validation with the AST is complex, so it doesn't do any. It will happily +// append duplicate object keys and panic on invalid types. The higher-level +// Config type is responsible for tracking state and making valid edits to +// the AST. +// +// Be aware that there are 4 ways of representing a package in devbox.json that +// the AST needs to handle: +// +// 1. ["name"] or ["name@version"] (versioned name array) +// 2. {"name": "version"} (packages object member with version string) +// 3. {"name": {"version": "1.2.3"}} (packages object member with package object) +// 4. {"github:F1bonacc1/process-compose/v0.40.2": {}} (packages object member with flakeref) +type configAST struct { + root hujson.Value +} + +// parseConfig parses the bytes of a devbox.json and returns a syntax tree. +func parseConfig(b []byte) (*configAST, error) { + root, err := hujson.Parse(b) + if err != nil { + return nil, err + } + return &configAST{root: root}, nil +} + +// packagesField gets the "packages" field, initializing it if necessary. The +// member value will either be an array of strings or an object. When it's an +// object, the keys will always be package names and the values will be a +// string or another object. Examples are: +// +// - {"packages": ["go", "hello"]} +// - {"packages": {"go": "1.20", "hello: {"platforms": ["aarch64-darwin"]}}} +// +// When migrate is true, the packages value will be migrated from the legacy +// array format to the object format. For example, the array: +// +// ["go@latest", "hello"] +// +// will become: +// +// { +// "go": "latest", +// "hello": "" +// } +func (c *configAST) packagesField(migrate bool) *hujson.ObjectMember { + rootObject := c.root.Value.(*hujson.Object) + i := c.memberIndex(rootObject, "packages") + if i != -1 { + switch rootObject.Members[i].Value.Value.Kind() { + case '[': + if migrate { + c.migratePackagesArray(&rootObject.Members[i].Value) + c.root.Format() + } + case 'n': + // Initialize a null packages field to an empty object. + rootObject.Members[i].Value.Value = &hujson.Object{ + AfterExtra: []byte{'\n'}, + } + c.root.Format() + } + return &rootObject.Members[i] + } + + // Add a packages field to the root config object and initialize it with + // an empty object. + rootObject.Members = append(rootObject.Members, hujson.ObjectMember{ + Name: hujson.Value{ + Value: hujson.String("packages"), + BeforeExtra: []byte{'\n'}, + }, + Value: hujson.Value{Value: &hujson.Object{}}, + }) + c.root.Format() + return &rootObject.Members[len(rootObject.Members)-1] +} + +// appendPackage appends a package to the packages field. +func (c *configAST) appendPackage(name, version string) { + pkgs := c.packagesField(false) + switch val := pkgs.Value.Value.(type) { + case *hujson.Object: + c.appendPackageToObject(val, name, version) + case *hujson.Array: + c.appendPackageToArray(val, joinNameVersion(name, version)) + default: + panic("packages field must be an object or array") + } + + // Ensure the packages field is on its own line. + if !slices.Contains(pkgs.Name.BeforeExtra, '\n') { + pkgs.Name.BeforeExtra = append(pkgs.Name.BeforeExtra, '\n') + } + c.root.Format() +} + +func (c *configAST) appendPackageToObject(pkgs *hujson.Object, name, version string) { + i := c.memberIndex(pkgs, name) + if i != -1 { + return + } + + // Add a new member to the packages object with the package name and + // version. + pkgs.Members = append(pkgs.Members, hujson.ObjectMember{ + Name: hujson.Value{Value: hujson.String(name), BeforeExtra: []byte{'\n'}}, + Value: hujson.Value{Value: hujson.String(version)}, + }) +} + +func (c *configAST) appendPackageToArray(arr *hujson.Array, versionedName string) { + var extra []byte + if len(arr.Elements) > 0 { + // Put each element on its own line if there + // will be more than 1. + extra = []byte{'\n'} + } + arr.Elements = append(arr.Elements, hujson.Value{ + BeforeExtra: extra, + Value: hujson.String(versionedName), + }) +} + +// removePackage removes a package from the packages field. +func (c *configAST) removePackage(name string) { + switch val := c.packagesField(false).Value.Value.(type) { + case *hujson.Object: + c.removePackageMember(val, name) + case *hujson.Array: + c.removePackageElement(val, name) + default: + panic("packages field must be an object or array") + } + c.root.Format() +} + +func (c *configAST) removePackageMember(pkgs *hujson.Object, name string) { + i := c.memberIndex(pkgs, name) + if i == -1 { + return + } + pkgs.Members = slices.Delete(pkgs.Members, i, i+1) +} + +func (c *configAST) removePackageElement(arr *hujson.Array, name string) { + i := c.packageElementIndex(arr, name) + if i == -1 { + return + } + arr.Elements = slices.Delete(arr.Elements, i, i+1) +} + +// appendPlatforms appends a platform to a package's "platforms" or +// "excluded_platforms" field. It automatically converts the package to an +// object if it isn't already. +func (c *configAST) appendPlatforms(name, fieldName string, platforms []string) { + if len(platforms) == 0 { + return + } + + pkgs := c.packagesField(true).Value.Value.(*hujson.Object) + i := c.memberIndex(pkgs, name) + if i == -1 { + return + } + + // We need to ensure that the package value is a full object + // (not a version string) before we can add a platform. + c.convertVersionToObject(&pkgs.Members[i].Value) + + pkgObject := pkgs.Members[i].Value.Value.(*hujson.Object) + var arr *hujson.Array + if i := c.memberIndex(pkgObject, fieldName); i == -1 { + arr = &hujson.Array{ + Elements: make([]hujson.Value, 0, len(platforms)), + } + pkgObject.Members = append(pkgObject.Members, hujson.ObjectMember{ + Name: hujson.Value{ + Value: hujson.String(fieldName), + BeforeExtra: []byte{'\n'}, + }, + Value: hujson.Value{Value: arr}, + }) + } else { + arr = pkgObject.Members[i].Value.Value.(*hujson.Array) + arr.Elements = slices.Grow(arr.Elements, len(platforms)) + } + + for _, p := range platforms { + arr.Elements = append(arr.Elements, hujson.Value{Value: hujson.String(p)}) + } + c.root.Format() +} + +// migratePackagesArray migrates a legacy array of package versionedNames to an +// object. See packagesField for details. +func (c *configAST) migratePackagesArray(pkgs *hujson.Value) { + arr := pkgs.Value.(*hujson.Array) + obj := &hujson.Object{Members: make([]hujson.ObjectMember, len(arr.Elements))} + for i, elem := range arr.Elements { + name, version := parseVersionedName(elem.Value.(hujson.Literal).String()) + + // Preserve any comments above the array elements. + var before []byte + if comment := bytes.TrimSpace(elem.BeforeExtra); len(comment) > 0 { + before = append([]byte{'\n'}, comment...) + } + before = append(before, '\n') + + obj.Members[i] = hujson.ObjectMember{ + Name: hujson.Value{ + Value: hujson.String(name), + BeforeExtra: before, + }, + Value: hujson.Value{Value: hujson.String(version)}, + } + } + pkgs.Value = obj +} + +// convertVersionToObject transforms a version string into an object with the +// version as a field. +func (c *configAST) convertVersionToObject(pkg *hujson.Value) { + if pkg.Value.Kind() == '{' { + return + } + + obj := &hujson.Object{} + if version, ok := pkg.Value.(hujson.Literal); ok && version.String() != "" { + obj.Members = append(obj.Members, hujson.ObjectMember{ + Name: hujson.Value{ + Value: hujson.String("version"), + BeforeExtra: []byte{'\n'}, + }, + Value: hujson.Value{Value: version}, + }) + } + pkg.Value = obj +} + +// memberIndex returns the index of an object member. +func (*configAST) memberIndex(obj *hujson.Object, name string) int { + return slices.IndexFunc(obj.Members, func(m hujson.ObjectMember) bool { + return m.Name.Value.(hujson.Literal).String() == name + }) +} + +// packageElementIndex returns the index of a package from an array of +// versionedName strings. +func (*configAST) packageElementIndex(arr *hujson.Array, name string) int { + return slices.IndexFunc(arr.Elements, func(v hujson.Value) bool { + elemName, _ := parseVersionedName(v.Value.(hujson.Literal).String()) + return elemName == name + }) +} + +func joinNameVersion(name, version string) string { + if version == "" { + return name + } + return name + "@" + version +} diff --git a/internal/devconfig/config.go b/internal/devconfig/config.go index a0ca4fba630..04bc6ef9a37 100644 --- a/internal/devconfig/config.go +++ b/internal/devconfig/config.go @@ -4,13 +4,17 @@ package devconfig import ( + "encoding/json" "io" "net/http" + "os" "path/filepath" "regexp" + "slices" "strings" "github.com/pkg/errors" + "github.com/tailscale/hujson" "go.jetpack.io/devbox/internal/boxcli/usererr" "go.jetpack.io/devbox/internal/cuecfg" "go.jetpack.io/devbox/internal/impl/shellcmd" @@ -42,6 +46,8 @@ type Config struct { // plugin: for built-in plugins // This is a similar format to nix inputs Include []string `json:"include,omitempty"` + + ast *configAST } type shellConfig struct { @@ -60,22 +66,28 @@ type Stage struct { } func DefaultConfig() *Config { - return &Config{ - // initialize to empty slice instead of nil for consistent marshalling - Packages: Packages{Collection: []Package{}}, - Shell: &shellConfig{ - Scripts: map[string]*shellcmd.Commands{ - "test": { - Cmds: []string{"echo \"Error: no test specified\" && exit 1"}, - }, - }, - InitHook: &shellcmd.Commands{ - Cmds: []string{ - "echo 'Welcome to devbox!' > /dev/null", - }, - }, - }, + cfg, err := loadBytes([]byte(`{ + "packages": [], + "shell": { + "init_hook": [ + "echo 'Welcome to devbox!' > /dev/null" + ], + "scripts": { + "test": [ + "echo \"Error: no test specified\" && exit 1" + ] + } + } +} +`)) + if err != nil { + panic("default devbox.json is invalid: " + err.Error()) } + return cfg +} + +func (c *Config) Bytes() []byte { + return c.ast.root.Pack() } func (c *Config) Hash() (string, error) { @@ -114,52 +126,50 @@ func (c *Config) InitHook() *shellcmd.Commands { // SaveTo writes the config to a file. func (c *Config) SaveTo(path string) error { - cfgPath := filepath.Join(path, DefaultName) - return cuecfg.WriteFile(cfgPath, c) -} - -func readConfig(path string) (*Config, error) { - cfg := &Config{} - return cfg, errors.WithStack(cuecfg.ParseFile(path, cfg)) + return os.WriteFile(filepath.Join(path, DefaultName), c.Bytes(), 0o644) } // Load reads a devbox config file, and validates it. func Load(path string) (*Config, error) { - cfg, err := readConfig(path) + b, err := os.ReadFile(path) if err != nil { return nil, err } - return cfg, validateConfig(cfg) + return loadBytes(b) } -func LoadConfigFromURL(url string) (*Config, error) { - res, err := http.Get(url) +func loadBytes(b []byte) (*Config, error) { + jsonb, err := hujson.Standardize(slices.Clone(b)) if err != nil { - return nil, errors.WithStack(err) + return nil, err } - defer res.Body.Close() - cfg := &Config{} - data, err := io.ReadAll(res.Body) + + ast, err := parseConfig(b) if err != nil { - return nil, errors.WithStack(err) + return nil, err } - ext := filepath.Ext(url) - if !cuecfg.IsSupportedExtension(ext) { - ext = ".json" + cfg := &Config{ + Packages: Packages{ast: ast}, + ast: ast, } - if err = cuecfg.Unmarshal(data, ext, cfg); err != nil { - return nil, errors.WithStack(err) + if err := json.Unmarshal(jsonb, cfg); err != nil { + return nil, err } return cfg, validateConfig(cfg) } -// WriteConfig saves a devbox config file. -func WriteConfig(path string, cfg *Config) error { - err := validateConfig(cfg) +func LoadConfigFromURL(url string) (*Config, error) { + res, err := http.Get(url) if err != nil { - return err + return nil, errors.WithStack(err) + } + defer res.Body.Close() + + data, err := io.ReadAll(res.Body) + if err != nil { + return nil, errors.WithStack(err) } - return cuecfg.WriteFile(path, cfg) + return loadBytes(data) } func validateConfig(cfg *Config) error { diff --git a/internal/devconfig/config_test.go b/internal/devconfig/config_test.go new file mode 100644 index 00000000000..7d537d6d048 --- /dev/null +++ b/internal/devconfig/config_test.go @@ -0,0 +1,620 @@ +//nolint:varnamelen +package devconfig + +import ( + "encoding/json" + "io" + "path/filepath" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tailscale/hujson" + "golang.org/x/tools/txtar" +) + +/* +The tests in this file use txtar to define test input and expected output. +This makes the JSON a lot easier to read vs. defining it in variables or structs +with weird indentation. + +Tests begin by defining their JSON with: + + in, want := parseConfigTxtarTest(t, `an optional comment that will be logged with t.Log + -- in -- + { } + -- want -- + { "packages": { "go": "latest" } }`) +*/ + +func parseConfigTxtarTest(t *testing.T, test string) (in *Config, want []byte) { + t.Helper() + + ar := txtar.Parse([]byte(test)) + if comment := strings.TrimSpace(string(ar.Comment)); comment != "" { + t.Log(comment) + } + for _, f := range ar.Files { + switch f.Name { + case "in": + var err error + in, err = loadBytes(f.Data) + if err != nil { + t.Fatalf("input devbox.json is invalid: %v\n%s", err, f.Data) + } + + case "want": + want = f.Data + } + } + return in, want +} + +func optBytesToStrings() cmp.Option { + return cmp.Transformer("bytesToStrings", func(b []byte) string { + return string(b) + }) +} + +func optParseHujson() cmp.Option { + f := func(b []byte) map[string]any { + gotMin, err := hujson.Minimize(b) + if err != nil { + return nil + } + var m map[string]any + if err := json.Unmarshal(gotMin, &m); err != nil { + return nil + } + return m + } + return cmp.Transformer("parseHujson", f) +} + +func TestNoChanges(t *testing.T) { + in, want := parseConfigTxtarTest(t, `a config that's loaded and saved without any changes should have unchanged json +-- in -- +{ "packages": { "go": "latest" } } +-- want -- +{ "packages": { "go": "latest" } }`) + + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageEmptyConfig(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{} +-- want -- +{ + "packages": { + "go": "latest" + } +}`) + + in.Packages.Add("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageEmptyConfigWhitespace(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + +} +-- want -- +{ + "packages": { + "go": "latest" + } +}`) + + in.Packages.Add("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageEmptyConfigComment(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +// Comment +{} +-- want -- +// Comment +{ + "packages": { + "go": "latest", + }, +}`) + + in.Packages.Add("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageNull(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ "packages": null } +-- want -- +{ + "packages": { + "go": "latest" + } +}`) + + in.Packages.Add("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageObject(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": { + "go": "latest" + } +} +-- want -- +{ + "packages": { + "go": "latest", + "python": "3.10" + } +}`) + + in.Packages.Add("python@3.10") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageObjectComment(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": { + // Package comment + "go": "latest" + } +} +-- want -- +{ + "packages": { + // Package comment + "go": "latest", + "python": "3.10", + }, +}`) + + in.Packages.Add("python@3.10") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageEmptyArray(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": [] +} +-- want -- +{ + "packages": ["go@latest"] +}`) + + in.Packages.Add("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageOneLineArray(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": ["go"] +} +-- want -- +{ + "packages": [ + "go", + "python@3.10" + ] +}`) + + in.Packages.Add("python@3.10") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageMultiLineArray(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": [ + "go" + ] +} +-- want -- +{ + "packages": [ + "go", + "python@3.10" + ] +}`) + + in.Packages.Add("python@3.10") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPackageArrayComments(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": [ + // Go package comment + "go", + + // Python package comment + "python@3.10" + ] +} +-- want -- +{ + "packages": [ + // Go package comment + "go", + + // Python package comment + "python@3.10", + "hello@latest", + ], +}`) + + in.Packages.Add("hello@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestRemovePackageObject(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": { + "go": "latest", + "python": "3.10" + } +} +-- want -- +{ + "packages": { + "python": "3.10" + } +}`) + + in.Packages.Remove("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestRemovePackageLastMember(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "env": {"NAME": "value"}, + "packages": { + "go": "latest" + } +} +-- want -- +{ + "env": {"NAME": "value"}, + "packages": {} +}`) + + in.Packages.Remove("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes(), optBytesToStrings()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestRemovePackageArray(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": ["go@latest", "python@3.10"] +} +-- want -- +{ + "packages": ["python@3.10"] +}`) + + in.Packages.Remove("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestRemovePackageLastElement(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": ["go@latest"], + "env": { + "NAME": "value" + } +} +-- want -- +{ + "packages": [], + "env": { + "NAME": "value" + } +}`) + + in.Packages.Remove("go@latest") + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPlatforms(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": { + "go": { + "version": "1.20" + }, + "python": { + "version": "3.10", + "platforms": [ + "x86_64-linux" + ] + }, + "hello": { + "version": "latest", + "platforms": ["x86_64-linux"] + }, + "vim": { + "version": "latest" + } + } +} +-- want -- +{ + "packages": { + "go": { + "version": "1.20", + "platforms": ["aarch64-darwin", "x86_64-darwin"] + }, + "python": { + "version": "3.10", + "platforms": [ + "x86_64-linux", + "x86_64-darwin" + ] + }, + "hello": { + "version": "latest", + "platforms": ["x86_64-linux", "x86_64-darwin"] + }, + "vim": { + "version": "latest" + } + } +}`) + + err := in.Packages.AddPlatforms(io.Discard, "go@1.20", []string{"aarch64-darwin", "x86_64-darwin"}) + if err != nil { + t.Error(err) + } + err = in.Packages.AddPlatforms(io.Discard, "python@3.10", []string{"x86_64-darwin"}) + if err != nil { + t.Error(err) + } + err = in.Packages.AddPlatforms(io.Discard, "hello@latest", []string{"x86_64-darwin"}) + if err != nil { + t.Error(err) + } + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPlatformsMigrateArray(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": ["go", "python@3.10", "hello"] +} +-- want -- +{ + "packages": { + "go": { + "platforms": ["aarch64-darwin"] + }, + "python": { + "version": "3.10", + "platforms": ["x86_64-darwin", "x86_64-linux"] + }, + "hello": "" + } +}`) + + err := in.Packages.AddPlatforms(io.Discard, "go", []string{"aarch64-darwin"}) + if err != nil { + t.Error(err) + } + err = in.Packages.AddPlatforms(io.Discard, "python@3.10", []string{"x86_64-darwin", "x86_64-linux"}) + if err != nil { + t.Error(err) + } + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestAddPlatformsMigrateArrayComments(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": [ + // Go comment + "go", + + // Python comment + "python@3.10" + ] +} +-- want -- +{ + "packages": { + // Go comment + "go": "", + // Python comment + "python": { + "version": "3.10", + "platforms": ["x86_64-darwin", "x86_64-linux"], + }, + }, +}`) + + err := in.Packages.AddPlatforms(io.Discard, "python@3.10", []string{"x86_64-darwin", "x86_64-linux"}) + if err != nil { + t.Error(err) + } + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestExcludePlatforms(t *testing.T) { + in, want := parseConfigTxtarTest(t, ` +-- in -- +{ + "packages": { + "go": { + "version": "1.20" + } + } +} +-- want -- +{ + "packages": { + "go": { + "version": "1.20", + "excluded_platforms": ["aarch64-darwin"] + } + } +}`) + + err := in.Packages.ExcludePlatforms(io.Discard, "go@1.20", []string{"aarch64-darwin"}) + if err != nil { + t.Error(err) + } + if diff := cmp.Diff(want, in.Bytes(), optParseHujson()); diff != "" { + t.Errorf("wrong parsed config json (-want +got):\n%s", diff) + } + if diff := cmp.Diff(want, in.Bytes()); diff != "" { + t.Errorf("wrong raw config hujson (-want +got):\n%s", diff) + } +} + +func TestDefault(t *testing.T) { + path := filepath.Join(t.TempDir()) + in := DefaultConfig() + inBytes := in.Bytes() + if _, err := hujson.Parse(inBytes); err != nil { + t.Fatalf("default config JSON is invalid: %v\n%s", err, inBytes) + } + err := in.SaveTo(path) + if err != nil { + t.Fatal("got save error:", err) + } + out, err := Load(filepath.Join(path, "devbox.json")) + if err != nil { + t.Fatal("got load error:", err) + } + if diff := cmp.Diff(in, out, cmpopts.IgnoreUnexported(Config{}, Packages{})); diff != "" { + t.Errorf("configs not equal (-in +out):\n%s", diff) + } + + outBytes := out.Bytes() + if _, err := hujson.Parse(outBytes); err != nil { + t.Fatalf("loaded default config JSON is invalid: %v\n%s", err, outBytes) + } + if string(inBytes) != string(outBytes) { + t.Errorf("got different JSON after load/save/load:\ninput:\n%s\noutput:\n%s", inBytes, outBytes) + } +} diff --git a/internal/devconfig/init.go b/internal/devconfig/init.go index c67fe05cc71..a058593b4f9 100644 --- a/internal/devconfig/init.go +++ b/internal/devconfig/init.go @@ -4,26 +4,28 @@ package devconfig import ( + "errors" "fmt" "io" + "os" "path/filepath" "strings" "github.com/fatih/color" - "go.jetpack.io/devbox/internal/cuecfg" "go.jetpack.io/devbox/internal/initrec" ) func Init(dir string, writer io.Writer) (created bool, err error) { - cfgPath := filepath.Join(dir, DefaultName) - - config := DefaultConfig() + created, err = initConfigFile(filepath.Join(dir, DefaultName)) + if err != nil || !created { + return created, err + } // package suggestion pkgsToSuggest, err := initrec.Get(dir) if err != nil { - return false, err + return created, err } if len(pkgsToSuggest) > 0 { s := fmt.Sprintf("devbox add %s", strings.Join(pkgsToSuggest, " ")) @@ -33,6 +35,30 @@ func Init(dir string, writer io.Writer) (created bool, err error) { color.HiYellowString(s), ) } + return created, err +} + +func initConfigFile(path string) (created bool, err error) { + file, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o644) + if errors.Is(err, os.ErrExist) { + return false, nil + } + if err != nil { + return false, err + } + defer func() { + if err != nil { + os.Remove(file.Name()) + } + }() - return cuecfg.InitFile(cfgPath, config) + _, err = file.Write(DefaultConfig().Bytes()) + if err != nil { + file.Close() + return false, err + } + if err := file.Close(); err != nil { + return false, err + } + return true, nil } diff --git a/internal/devconfig/packages.go b/internal/devconfig/packages.go index 008a43884b3..3a04aa4cabc 100644 --- a/internal/devconfig/packages.go +++ b/internal/devconfig/packages.go @@ -1,3 +1,4 @@ +//nolint:dupl // add/exclude platforms needs refactoring package devconfig import ( @@ -7,7 +8,6 @@ import ( "strings" "github.com/pkg/errors" - "github.com/samber/lo" orderedmap "github.com/wk8/go-ordered-map/v2" "go.jetpack.io/devbox/internal/boxcli/usererr" "go.jetpack.io/devbox/internal/nix" @@ -15,23 +15,11 @@ import ( "go.jetpack.io/devbox/internal/ux" ) -type jsonKind int - -const ( - // jsonList is the legacy format for packages - jsonList jsonKind = iota - // jsonMap is the new format for packages - jsonMap jsonKind = iota -) - type Packages struct { - jsonKind jsonKind - // Collection contains the set of package definitions - // We don't want this key to be serialized automatically, hence the "key" in json is "-" - // NOTE: this is not a pointer to make debugging failure cases easier - // (get dumps of the values, not memory addresses) - Collection []Package `json:"-,omitempty"` + Collection []Package + + ast *configAST } // VersionedNames returns a list of package names with versions. @@ -50,26 +38,32 @@ func (pkgs *Packages) VersionedNames() []string { // Get returns the package with the given versionedName func (pkgs *Packages) Get(versionedName string) (*Package, bool) { name, version := parseVersionedName(versionedName) - for _, pkg := range pkgs.Collection { - if pkg.name == name && pkg.Version == version { - return &pkg, true - } + i := pkgs.index(name, version) + if i == -1 { + return nil, false } - return nil, false + return &pkgs.Collection[i], true } // Add adds a package to the list of packages func (pkgs *Packages) Add(versionedName string) { name, version := parseVersionedName(versionedName) + if pkgs.index(name, version) != -1 { + return + } pkgs.Collection = append(pkgs.Collection, NewVersionOnlyPackage(name, version)) + pkgs.ast.appendPackage(name, version) } // Remove removes a package from the list of packages func (pkgs *Packages) Remove(versionedName string) { name, version := parseVersionedName(versionedName) - pkgs.Collection = slices.DeleteFunc(pkgs.Collection, func(pkg Package) bool { - return pkg.name == name && pkg.Version == version - }) + i := pkgs.index(name, version) + if i == -1 { + return + } + pkgs.Collection = slices.Delete(pkgs.Collection, i, i+1) + pkgs.ast.removePackage(name) } // AddPlatforms adds a platform to the list of platforms for a given package @@ -82,33 +76,37 @@ func (pkgs *Packages) AddPlatforms(writer io.Writer, versionedname string, platf } name, version := parseVersionedName(versionedname) - for idx, pkg := range pkgs.Collection { - if pkg.name == name && pkg.Version == version { - - // Append if the platform is not already present - pkg.Platforms = lo.Uniq(append(pkg.Platforms, platforms...)) - - // Adding any platform will restrict installation to it, so - // the ExcludedPlatforms are no longer needed - if len(pkg.ExcludedPlatforms) > 0 { - return usererr.New( - "cannot add any platform for package %s because it already has `excluded_platforms` defined. "+ - "Please delete the `excludedPlatforms` for this package from devbox.json and re-try.", - pkg.VersionedName(), - ) - } - ux.Finfo(writer, - "Added platform %s to package %s\n", strings.Join(platforms, ", "), - pkg.VersionedName(), - ) - - pkgs.jsonKind = jsonMap - pkg.kind = regular - pkgs.Collection[idx] = pkg - return nil + i := pkgs.index(name, version) + if i == -1 { + return errors.Errorf("package %s not found", versionedname) + } + + // Adding any platform will restrict installation to it, so + // the ExcludedPlatforms are no longer needed + pkg := &pkgs.Collection[i] + if len(pkg.ExcludedPlatforms) > 0 { + return usererr.New( + "cannot add any platform for package %s because it already has `excluded_platforms` defined. "+ + "Please delete the `excluded_platforms` for this package from devbox.json and retry.", + pkg.VersionedName(), + ) + } + + // Append if the platform is not already present + oldLen := len(pkg.Platforms) + for _, p := range platforms { + if !slices.Contains(pkg.Platforms, p) { + pkg.Platforms = append(pkg.Platforms, p) } } - return errors.Errorf("package %s not found", versionedname) + if len(pkg.Platforms) > oldLen { + pkgs.ast.appendPlatforms(pkg.name, "platforms", pkg.Platforms[oldLen:]) + ux.Finfo(writer, + "Added platform %s to package %s\n", strings.Join(platforms, ", "), + pkg.VersionedName(), + ) + } + return nil } // ExcludePlatforms adds a platform to the list of excluded platforms for a given package @@ -116,34 +114,37 @@ func (pkgs *Packages) ExcludePlatforms(writer io.Writer, versionedName string, p if len(platforms) == 0 { return nil } - for _, platform := range platforms { - if err := nix.EnsureValidPlatform(platform); err != nil { - return errors.WithStack(err) - } + if err := nix.EnsureValidPlatform(platforms...); err != nil { + return errors.WithStack(err) } name, version := parseVersionedName(versionedName) - for idx, pkg := range pkgs.Collection { - if pkg.name == name && pkg.Version == version { - - pkg.ExcludedPlatforms = lo.Uniq(append(pkg.ExcludedPlatforms, platforms...)) - if len(pkg.Platforms) > 0 { - return usererr.New( - "cannot exclude any platform for package %s because it already has `platforms` defined. "+ - "Please delete the `platforms` for this package from devbox.json and re-try.", - pkg.VersionedName(), - ) - } - ux.Finfo(writer, "Excluded platform %s for package %s\n", strings.Join(platforms, ", "), - pkg.VersionedName()) + i := pkgs.index(name, version) + if i == -1 { + return errors.Errorf("package %s not found", versionedName) + } - pkgs.jsonKind = jsonMap - pkg.kind = regular - pkgs.Collection[idx] = pkg - return nil + pkg := &pkgs.Collection[i] + if len(pkg.Platforms) > 0 { + return usererr.New( + "cannot exclude any platform for package %s because it already has `platforms` defined. "+ + "Please delete the `platforms` for this package from devbox.json and re-try.", + pkg.VersionedName(), + ) + } + + oldLen := len(pkg.ExcludedPlatforms) + for _, p := range platforms { + if !slices.Contains(pkg.ExcludedPlatforms, p) { + pkg.ExcludedPlatforms = append(pkg.ExcludedPlatforms, p) } } - return errors.Errorf("package %s not found", versionedName) + if len(pkg.ExcludedPlatforms) > oldLen { + pkgs.ast.appendPlatforms(pkg.name, "excluded_platforms", pkg.ExcludedPlatforms[oldLen:]) + ux.Finfo(writer, "Excluded platform %s for package %s\n", strings.Join(platforms, ", "), + pkg.VersionedName()) + } + return nil } func (pkgs *Packages) UnmarshalJSON(data []byte) error { @@ -151,7 +152,6 @@ func (pkgs *Packages) UnmarshalJSON(data []byte) error { var packages []string if err := json.Unmarshal(data, &packages); err == nil { pkgs.Collection = packagesFromLegacyList(packages) - pkgs.jsonKind = jsonList return nil } @@ -174,41 +174,16 @@ func (pkgs *Packages) UnmarshalJSON(data []byte) error { packagesList = append(packagesList, pkg) } pkgs.Collection = packagesList - pkgs.jsonKind = jsonMap return nil } -func (pkgs *Packages) MarshalJSON() ([]byte, error) { - if pkgs.jsonKind == jsonList { - packagesList := make([]string, 0, len(pkgs.Collection)) - for _, p := range pkgs.Collection { - - // Version may be empty for unversioned packages - packageToWrite := p.name - if p.Version != "" { - packageToWrite += "@" + p.Version - } - packagesList = append(packagesList, packageToWrite) - } - return json.Marshal(packagesList) - } - - orderedMap := orderedmap.New[string, Package]() - for _, p := range pkgs.Collection { - orderedMap.Set(p.name, p) - } - return json.Marshal(orderedMap) +func (pkgs *Packages) index(name, version string) int { + return slices.IndexFunc(pkgs.Collection, func(p Package) bool { + return p.name == name && p.Version == version + }) } -type packageKind int - -const ( - versionOnly packageKind = iota - regular packageKind = iota -) - type Package struct { - kind packageKind name string Version string `json:"version,omitempty"` @@ -218,7 +193,6 @@ type Package struct { func NewVersionOnlyPackage(name, version string) Package { return Package{ - kind: versionOnly, name: name, Version: version, } @@ -243,7 +217,6 @@ func NewPackage(name string, values map[string]any) Package { } return Package{ - kind: regular, name: name, Version: version.(string), Platforms: platforms, @@ -285,7 +258,6 @@ func (p *Package) UnmarshalJSON(data []byte) error { // First, attempt to unmarshal as a version-only string var version string if err := json.Unmarshal(data, &version); err == nil { - p.kind = versionOnly p.Version = version return nil } @@ -298,20 +270,9 @@ func (p *Package) UnmarshalJSON(data []byte) error { } *p = Package(*alias) - p.kind = regular return nil } -func (p Package) MarshalJSON() ([]byte, error) { - if p.kind == versionOnly { - return json.Marshal(p.Version) - } - - // If we have a regular package, we want to marshal the entire struct: - type packageAlias Package // Use an alias-type to avoid infinite recursion - return json.Marshal((packageAlias)(p)) -} - // parseVersionedName parses the name and version from package@version representation func parseVersionedName(versionedName string) (name, version string) { var found bool diff --git a/internal/devconfig/packages_test.go b/internal/devconfig/packages_test.go index ea40ffa5e2a..c8cfe2caf5f 100644 --- a/internal/devconfig/packages_test.go +++ b/internal/devconfig/packages_test.go @@ -1,12 +1,11 @@ package devconfig import ( - "bytes" - "encoding/json" - "reflect" "testing" - "go.jetpack.io/devbox/internal/cuecfg" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/tailscale/hujson" ) // TestJsonifyConfigPackages tests the jsonMarshal and jsonUnmarshal of the Config.Packages field @@ -20,7 +19,6 @@ func TestJsonifyConfigPackages(t *testing.T) { name: "empty-list", jsonConfig: `{"packages":[]}`, expected: Packages{ - jsonKind: jsonList, Collection: []Package{}, }, }, @@ -28,7 +26,6 @@ func TestJsonifyConfigPackages(t *testing.T) { name: "empty-map", jsonConfig: `{"packages":{}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{}, }, }, @@ -36,7 +33,6 @@ func TestJsonifyConfigPackages(t *testing.T) { name: "flat-list", jsonConfig: `{"packages":["python","hello@latest","go@1.20"]}`, expected: Packages{ - jsonKind: jsonList, Collection: packagesFromLegacyList([]string{"python", "hello@latest", "go@1.20"}), }, }, @@ -44,7 +40,6 @@ func TestJsonifyConfigPackages(t *testing.T) { name: "map-with-string-value", jsonConfig: `{"packages":{"python":"latest","go":"1.20"}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewVersionOnlyPackage("python", "latest"), NewVersionOnlyPackage("go", "1.20"), @@ -56,7 +51,6 @@ func TestJsonifyConfigPackages(t *testing.T) { name: "map-with-struct-value", jsonConfig: `{"packages":{"python":{"version":"latest"}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewPackage("python", map[string]any{"version": "latest"}), }, @@ -66,7 +60,6 @@ func TestJsonifyConfigPackages(t *testing.T) { name: "map-with-string-and-struct-values", jsonConfig: `{"packages":{"go":"1.20","emacs":"latest","python":{"version":"latest"}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewVersionOnlyPackage("go", "1.20"), NewVersionOnlyPackage("emacs", "latest"), @@ -79,7 +72,6 @@ func TestJsonifyConfigPackages(t *testing.T) { jsonConfig: `{"packages":{"python":{"version":"latest",` + `"platforms":["x86_64-darwin","aarch64-linux"]}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewPackage("python", map[string]any{ "version": "latest", @@ -93,7 +85,6 @@ func TestJsonifyConfigPackages(t *testing.T) { jsonConfig: `{"packages":{"python":{"version":"latest",` + `"excluded_platforms":["x86_64-linux"]}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewPackage("python", map[string]any{ "version": "latest", @@ -108,7 +99,6 @@ func TestJsonifyConfigPackages(t *testing.T) { `"platforms":["x86_64-darwin","aarch64-linux"],` + `"excluded_platforms":["x86_64-linux"]}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewPackage("python", map[string]any{ "version": "latest", @@ -124,7 +114,6 @@ func TestJsonifyConfigPackages(t *testing.T) { `"platforms":["x86_64-darwin","aarch64-linux"],` + `"excluded_platforms":["x86_64-linux"]}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewPackage("path:my-php-flake#hello", map[string]any{ "version": "latest", @@ -141,7 +130,6 @@ func TestJsonifyConfigPackages(t *testing.T) { `"platforms":["x86_64-darwin","aarch64-linux"],` + `"excluded_platforms":["x86_64-linux"]}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewPackage("github:F1bonacc1/process-compose/v0.43.1", map[string]any{ "version": "latest", @@ -158,7 +146,6 @@ func TestJsonifyConfigPackages(t *testing.T) { `"platforms":["x86_64-darwin","aarch64-linux"],` + `"excluded_platforms":["x86_64-linux"]}}}`, expected: Packages{ - jsonKind: jsonMap, Collection: []Package{ NewPackage("github:nixos/nixpkgs/5233fd2ba76a3accb5aaa999c00509a11fd0793c#hello", map[string]any{ "version": "latest", @@ -172,42 +159,31 @@ func TestJsonifyConfigPackages(t *testing.T) { for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - config := &Config{} - if err := json.Unmarshal([]byte(testCase.jsonConfig), config); err != nil { - t.Errorf("unexpected error: %v", err) - } - if !reflect.DeepEqual(config.Packages, testCase.expected) { - t.Errorf("expected: %v, got: %v", testCase.expected, config.Packages) - } - - marshalled, err := json.Marshal(config) + config, err := loadBytes([]byte(testCase.jsonConfig)) if err != nil { - t.Errorf("unexpected error: %v", err) + t.Errorf("load error: %v", err) } - if string(marshalled) != testCase.jsonConfig { - t.Errorf("expected: %v, got: %v", testCase.jsonConfig, string(marshalled)) + if diff := diffPackages(t, config.Packages, testCase.expected); diff != "" { + t.Errorf("got wrong packages (-want +got):\n%s", diff) } - // We also test cuecfg.Marshal because elsewhere in our code we rely on it. - // While in this PR it is now a simple wrapper over json.Marshal, we want to - // ensure that any future changes to that function don't break our code. - marshalled, err = cuecfg.Marshal(config, ".json") + got, err := hujson.Minimize(config.Bytes()) if err != nil { - t.Errorf("unexpected error: %v", err) - } - // We need to pretty-print the expected output because cuecfg.Marshal returns - // the json pretty-printed. - expected := &bytes.Buffer{} - if err := json.Indent(expected, []byte(testCase.jsonConfig), "", cuecfg.Indent); err != nil { - t.Errorf("unexpected error: %v", err) + t.Fatal(err) } - if string(marshalled) != expected.String() { - t.Errorf("expected: %v, got: %v", testCase.jsonConfig, string(marshalled)) + if string(got) != testCase.jsonConfig { + t.Errorf("expected: %v, got: %v", testCase.jsonConfig, string(got)) } }) } } +func diffPackages(t *testing.T, got, want Packages) string { + t.Helper() + + return cmp.Diff(want, got, cmpopts.IgnoreUnexported(Packages{}, Package{})) +} + func TestParseVersionedName(t *testing.T) { testCases := []struct { name string