Skip to content

Commit

Permalink
[cleanup] Remove reflike and tyson (#1867)
Browse files Browse the repository at this point in the history
## Summary

Stacked on #1850

* Remove RefLike
* Remove experimental tyson support (v2 plugins are a reasonable
substitute)

## How was it tested?

CICD
  • Loading branch information
mikeland73 authored Mar 5, 2024
1 parent 3e8115a commit 9f11a98
Show file tree
Hide file tree
Showing 15 changed files with 93 additions and 189 deletions.
3 changes: 0 additions & 3 deletions internal/boxcli/featureflag/tyson.go

This file was deleted.

2 changes: 1 addition & 1 deletion internal/boxcli/multi/multi.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func Open(opts *devopt.Opts) ([]*devbox.Devbox, error) {
return err
}

if !dirEntry.IsDir() && configfile.IsConfigName(filepath.Base(path)) {
if !dirEntry.IsDir() && filepath.Base(path) == configfile.DefaultName {
optsCopy := *opts
optsCopy.Dir = path
box, err := devbox.Open(&optsCopy)
Expand Down
7 changes: 1 addition & 6 deletions internal/devbox/dir.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,5 @@ func missingConfigError(path string, didCheckParents bool) error {
}

func configExistsIn(path string) bool {
for _, name := range configfile.ValidConfigNames() {
if fileutil.Exists(filepath.Join(path, name)) {
return true
}
}
return false
return fileutil.Exists(filepath.Join(path, configfile.DefaultName))
}
8 changes: 4 additions & 4 deletions internal/devbox/dir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestFindProjectDirFromParentDirSearch(t *testing.T) {
err = os.MkdirAll(filepath.Join(root, testCase.allDirs), 0o777)
assert.NoError(err)

absProjectPath, err := filepath.Abs(filepath.Join(root, testCase.projectDir, configfile.ValidConfigNames()[0]))
absProjectPath, err := filepath.Abs(filepath.Join(root, testCase.projectDir, configfile.DefaultName))
assert.NoError(err)
err = os.WriteFile(absProjectPath, []byte("{}"), 0o666)
assert.NoError(err)
Expand Down Expand Up @@ -97,14 +97,14 @@ func TestFindParentDirAtPath(t *testing.T) {
name: "flag_path_is_file_has_config",
allDirs: "a/b/c",
projectDir: "a/b",
flagPath: "a/b/" + configfile.ValidConfigNames()[0],
flagPath: "a/b/" + configfile.DefaultName,
expectError: false,
},
{
name: "flag_path_is_file_missing_config",
allDirs: "a/b/c",
projectDir: "", // missing config
flagPath: "a/b/" + configfile.ValidConfigNames()[0],
flagPath: "a/b/" + configfile.DefaultName,
expectError: true,
},
}
Expand All @@ -121,7 +121,7 @@ func TestFindParentDirAtPath(t *testing.T) {

var absProjectPath string
if testCase.projectDir != "" {
absProjectPath, err = filepath.Abs(filepath.Join(root, testCase.projectDir, configfile.ValidConfigNames()[0]))
absProjectPath, err = filepath.Abs(filepath.Join(root, testCase.projectDir, configfile.DefaultName))
assert.NoError(err)
err = os.WriteFile(absProjectPath, []byte("{}"), 0o666)
assert.NoError(err)
Expand Down
8 changes: 3 additions & 5 deletions internal/devconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ func DefaultConfig() *Config {
return cfg
}

func IsNotDefault(path string) bool {
func IsDefault(path string) bool {
cfg, err := readFromFile(path)
if err != nil {
return false
}
return !cfg.Root.Equals(&DefaultConfig().Root)
return cfg.Root.Equals(&DefaultConfig().Root)
}

func LoadForTest(path string) (*Config, error) {
Expand Down Expand Up @@ -171,9 +171,7 @@ func (c *Config) Packages() []configfile.Package {
for _, i := range c.included {
packages = append(packages, i.Packages()...)
if i.pluginData.RemoveTriggerPackage {
if pkg, ok := i.pluginData.Source.(interface{ LockfileKey() string }); ok {
packagesToRemove[pkg.LockfileKey()] = true
}
packagesToRemove[i.pluginData.Source.LockfileKey()] = true
}
}

Expand Down
26 changes: 1 addition & 25 deletions internal/devconfig/configfile/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,13 @@ import (

"github.com/pkg/errors"
"github.com/tailscale/hujson"
"go.jetpack.io/devbox/internal/boxcli/featureflag"
"go.jetpack.io/devbox/internal/boxcli/usererr"
"go.jetpack.io/devbox/internal/cachehash"
"go.jetpack.io/devbox/internal/devbox/shellcmd"
)

const (
DefaultName = "devbox.json"
DefaultTySONName = "devbox.tson"
)

const (
JSONFormat = iota
TSONFormat
DefaultName = "devbox.json"
)

// ConfigFile defines a devbox environment as JSON.
Expand Down Expand Up @@ -59,8 +52,6 @@ type ConfigFile struct {
Include []string `json:"include,omitempty"`

ast *configAST
// TODO Remove tyson support and this field.
Format int
}

type shellConfig struct {
Expand Down Expand Up @@ -117,9 +108,6 @@ func (c *ConfigFile) InitHook() *shellcmd.Commands {

// SaveTo writes the config to a file.
func (c *ConfigFile) SaveTo(path string) error {
if c.Format != JSONFormat {
return errors.New("cannot save config to non-json format")
}
return os.WriteFile(filepath.Join(path, DefaultName), c.Bytes(), 0o644)
}

Expand Down Expand Up @@ -209,15 +197,3 @@ func ValidateNixpkg(cfg *ConfigFile) error {
}
return nil
}

func IsConfigName(name string) bool {
return slices.Contains(ValidConfigNames(), name)
}

func ValidConfigNames() []string {
names := []string{DefaultName}
if featureflag.TySON.Enabled() {
names = append(names, DefaultTySONName)
}
return names
}
40 changes: 0 additions & 40 deletions internal/devconfig/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,16 @@
package devconfig

import (
"context"
"errors"
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"strings"

"github.com/fatih/color"

"go.jetpack.io/devbox/internal/boxcli/featureflag"
"go.jetpack.io/devbox/internal/devconfig/configfile"
"go.jetpack.io/devbox/internal/devpkg/pkgtype"
"go.jetpack.io/devbox/internal/fileutil"
"go.jetpack.io/devbox/internal/initrec"
)

Expand Down Expand Up @@ -71,40 +66,5 @@ func initConfigFile(path string) (created bool, err error) {

func Open(projectDir string) (*Config, error) {
cfgPath := filepath.Join(projectDir, configfile.DefaultName)

if !featureflag.TySON.Enabled() {
return readFromFile(cfgPath)
}

tysonCfgPath := filepath.Join(projectDir, configfile.DefaultTySONName)

// If tyson config exists use it. Otherwise fallback to json config.
// In the future we may error out if both configs exist, but for now support
// both while we experiment with tyson support.
if fileutil.Exists(tysonCfgPath) {
paths, err := pkgtype.RunXClient().Install(context.TODO(), "jetpack-io/tyson")
if err != nil {
return nil, err
}
binPath := filepath.Join(paths[0], "tyson")
tmpFile, err := os.CreateTemp("", "*.json")
if err != nil {
return nil, err
}
cmd := exec.Command(binPath, "eval", tysonCfgPath)
cmd.Stderr = os.Stderr
cmd.Stdout = tmpFile
if err = cmd.Run(); err != nil {
return nil, err
}
cfgPath = tmpFile.Name()
config, err := readFromFile(cfgPath)
if err != nil {
return nil, err
}
config.Root.Format = configfile.TSONFormat
return config, nil
}

return readFromFile(cfgPath)
}
11 changes: 7 additions & 4 deletions internal/devpkg/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func isAmbiguous(raw string, parsed flake.Installable) bool {
// resolve is the implementation of Package.resolve, where it is wrapped in a
// sync.OnceValue function. It should not be called directly.
func resolve(pkg *Package) error {
resolved, err := pkg.lockfile.Resolve(pkg.Raw)
resolved, err := pkg.lockfile.Resolve(pkg.LockfileKey())
if err != nil {
return err
}
Expand Down Expand Up @@ -590,7 +590,7 @@ func (p *Package) InputAddressedPaths() ([]string, error) {
errors.Errorf("Package %q cannot be fetched from binary cache store", p.Raw)
}

entry, err := p.lockfile.Resolve(p.Raw)
entry, err := p.lockfile.Resolve(p.LockfileKey())
if err != nil {
return nil, err
}
Expand All @@ -617,7 +617,7 @@ func (p *Package) InputAddressedPathForOutput(output string) (string, error) {
errors.Errorf("Package %q cannot be fetched from binary cache store", p.Raw)
}

entry, err := p.lockfile.Resolve(p.Raw)
entry, err := p.lockfile.Resolve(p.LockfileKey())
if err != nil {
return "", err
}
Expand Down Expand Up @@ -657,7 +657,7 @@ func (p *Package) EnsureUninstallableIsInLockfile() error {
if !p.IsInstallable() || !p.IsDevboxPackage {
return nil
}
_, err := p.lockfile.Resolve(p.Raw)
_, err := p.lockfile.Resolve(p.LockfileKey())
return err
}

Expand All @@ -681,6 +681,9 @@ func (p *Package) String() string {
}

func (p *Package) LockfileKey() string {
// Use p.Raw instead of p.installable.Ref.String() because that will have
// absolute paths. TODO: We may want to change SetInstallable to avoid making
// flake ref absolute.
return p.Raw
}

Expand Down
11 changes: 6 additions & 5 deletions internal/plugin/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,11 @@ import (

"go.jetpack.io/devbox/internal/boxcli/usererr"
"go.jetpack.io/devbox/internal/cachehash"
"go.jetpack.io/devbox/nix/flake"
)

type githubPlugin struct {
ref RefLike
ref flake.Ref
}

func (p *githubPlugin) Fetch() ([]byte, error) {
Expand Down Expand Up @@ -40,9 +41,9 @@ func (p *githubPlugin) FileContent(subpath string) ([]byte, error) {
defer res.Body.Close()
if res.StatusCode != http.StatusOK {
return nil, usererr.New(
"failed to get plugin github:%s @ %s (Status code %d). \nPlease make "+
"failed to get plugin %s @ %s (Status code %d). \nPlease make "+
"sure a plugin.json file exists in plugin directory.",
p.ref.String(),
p.LockfileKey(),
contentURL,
res.StatusCode,
)
Expand All @@ -57,12 +58,12 @@ func (p *githubPlugin) url(subpath string) (string, error) {
"https://raw.githubusercontent.com/",
p.ref.Owner,
p.ref.Repo,
cmp.Or(p.ref.Rev, p.ref.Ref.Ref, "master"),
cmp.Or(p.ref.Rev, p.ref.Ref, "master"),
p.ref.Dir,
subpath,
)
}

func (p *githubPlugin) LockfileKey() string {
return p.ref.raw
return p.ref.String()
}
60 changes: 22 additions & 38 deletions internal/plugin/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@ func TestNewGithubPlugin(t *testing.T) {
name: "parse basic github plugin",
Include: "github:jetpack-io/devbox-plugins",
expected: githubPlugin{
ref: RefLike{
Ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
},
raw: "github:jetpack-io/devbox-plugins",
filename: pluginConfigName,
ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
},
},
expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/master",
Expand All @@ -34,15 +30,11 @@ func TestNewGithubPlugin(t *testing.T) {
name: "parse github plugin with dir param",
Include: "github:jetpack-io/devbox-plugins?dir=mongodb",
expected: githubPlugin{
ref: RefLike{
Ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
Dir: "mongodb",
},
raw: "github:jetpack-io/devbox-plugins?dir=mongodb",
filename: pluginConfigName,
ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
Dir: "mongodb",
},
},
expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/master/mongodb",
Expand All @@ -51,16 +43,12 @@ func TestNewGithubPlugin(t *testing.T) {
name: "parse github plugin with dir param and rev",
Include: "github:jetpack-io/devbox-plugins/my-branch?dir=mongodb",
expected: githubPlugin{
ref: RefLike{
Ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
Ref: "my-branch",
Dir: "mongodb",
},
raw: "github:jetpack-io/devbox-plugins/my-branch?dir=mongodb",
filename: pluginConfigName,
ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
Ref: "my-branch",
Dir: "mongodb",
},
},
expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/my-branch/mongodb",
Expand All @@ -69,16 +57,12 @@ func TestNewGithubPlugin(t *testing.T) {
name: "parse github plugin with dir param and rev",
Include: "github:jetpack-io/devbox-plugins/initials/my-branch?dir=mongodb",
expected: githubPlugin{
ref: RefLike{
Ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
Ref: "initials/my-branch",
Dir: "mongodb",
},
raw: "github:jetpack-io/devbox-plugins/initials/my-branch?dir=mongodb",
filename: pluginConfigName,
ref: flake.Ref{
Type: "github",
Owner: "jetpack-io",
Repo: "devbox-plugins",
Ref: "initials/my-branch",
Dir: "mongodb",
},
},
expectedURL: "https://raw.githubusercontent.com/jetpack-io/devbox-plugins/initials/my-branch/mongodb",
Expand All @@ -87,7 +71,7 @@ func TestNewGithubPlugin(t *testing.T) {

for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
actual, _ := parseReflike(testCase.Include, "")
actual, _ := parseIncludable(testCase.Include, "")
assert.Equal(t, &testCase.expected, actual)
u, err := testCase.expected.url("")
assert.Nil(t, err)
Expand Down
Loading

0 comments on commit 9f11a98

Please sign in to comment.