Skip to content

Commit e2478a2

Browse files
authored
[devbox.json] Allowing including plugins explicitly via "include" field. (#986)
## Summary This adds the ability to include plugins explicitly via: ``` "include": [ "plugin:php" ] ``` in devbox.json. The idea is to allow including any devbox.json into an existing one. The proposed syntax is: * path: for local files * https:// for remote files * plugin: for built-in plugins only plugin is implemented in this PR. Improved existing plugin mechanism by using `lockfile` to decide is `devbox.d` directory needs to be created by keeping track of the plugin version. In the future we could allow upgrading of configs if there is version mismatch. A few design questions: (cc: @Lagoja @loreto ) * Should include be merged into `packges` instead? We could introspect the include to see if it's a flake or a plugin. This would simplify the devbox.json but it might be a weird way to include other configs. * One kinda ugly thing of current design is that the php plugin doesn't actually install PHP. That feels a bit wrong, but the way we've currently designed plugins is that they trigger on packages so not sure how to improve this. One option is to rename `builtin` to something that makes it more explicit that it extends functionality on existing nix packages. Maybe `extend:php`. Another option: In the future I expect includes to contain packages which would be installed as well. So could parametrize the php plugin so it can take the top level php package as an input (but otherwise it could install its own) Edit: After chatting with @loreto and @Lagoja we agreed to keep this design with following: * replace "builtin" with "plugin". * Including plugins works the same way current plugins. Specifically init hooks, and environment get added to existing environment. No new packages are installed. ## How was it tested? In php flake example, did `devbox services up`
1 parent 3dc466c commit e2478a2

File tree

12 files changed

+130
-33
lines changed

12 files changed

+130
-33
lines changed

examples/flakes/php/devbox.json

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,8 @@
88
},
99
"nixpkgs": {
1010
"commit": "f80ac848e3d6f0c12c52758c0f25c10c97ca3b62"
11-
}
12-
}
11+
},
12+
"include": [
13+
"plugin:php"
14+
]
15+
}

internal/impl/config.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,13 @@ type Config struct {
4040

4141
// Nixpkgs specifies the repository to pull packages from
4242
Nixpkgs NixpkgsConfig `json:"nixpkgs,omitempty"`
43+
44+
// Reserved to allow including other config files. Proposed format is:
45+
// path: for local files
46+
// https:// for remote files
47+
// plugin: for built-in plugins
48+
// This is a similar format to nix inputs
49+
Include []string `json:"include,omitempty"`
4350
}
4451

4552
type NixpkgsConfig struct {

internal/impl/devbox.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ func Open(path string, writer io.Writer) (*Devbox, error) {
125125
if err != nil {
126126
return nil, err
127127
}
128+
box.pluginManager.ApplyOptions(plugin.WithLockfile(lock))
128129
box.lockfile = lock
129130
return box, nil
130131
}
@@ -448,7 +449,11 @@ func (d *Devbox) saveCfg() error {
448449
}
449450

450451
func (d *Devbox) Services() (services.Services, error) {
451-
pluginSvcs, err := plugin.GetServices(d.packagesAsInputs(), d.projectDir)
452+
pluginSvcs, err := d.pluginManager.GetServices(
453+
d.packagesAsInputs(),
454+
d.cfg.Include,
455+
d.projectDir,
456+
)
452457
if err != nil {
453458
return nil, err
454459
}
@@ -782,7 +787,8 @@ func (d *Devbox) computeNixEnv(ctx context.Context, usePrintDevEnvCache bool) (m
782787
// We still need to be able to add env variables to non-service binaries
783788
// (e.g. ruby). This would involve understanding what binaries are associated
784789
// to a given plugin.
785-
pluginEnv, err := plugin.Env(d.packagesAsInputs(), d.projectDir, env)
790+
pluginEnv, err := d.pluginManager.Env(
791+
d.packagesAsInputs(), d.cfg.Include, d.projectDir, env)
786792
if err != nil {
787793
return nil, err
788794
}

internal/impl/generate.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,16 @@ func (d *Devbox) generateShellFiles() error {
6161
}
6262

6363
for _, pkg := range d.packagesAsInputs() {
64-
if err := d.pluginManager.CreateFilesAndShowReadme(d.writer, pkg, d.projectDir); err != nil {
64+
if err := d.pluginManager.Create(d.writer, pkg, d.projectDir); err != nil {
65+
return err
66+
}
67+
}
68+
69+
for _, included := range d.cfg.Include {
70+
if err := d.lockfile.Add(included); err != nil {
71+
return err
72+
}
73+
if err := d.pluginManager.Include(d.writer, included, d.projectDir); err != nil {
6574
return err
6675
}
6776
}

internal/impl/packages.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@ func (d *Devbox) Add(ctx context.Context, pkgsNames ...string) error {
6363
}
6464
}
6565

66-
d.pluginManager.ApplyOptions(plugin.WithAddMode())
6766
if err := d.ensurePackagesAreInstalled(ctx, install); err != nil {
6867
return usererr.WithUserMessage(
6968
err,

internal/impl/update.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ func (d *Devbox) Update(ctx context.Context, pkgs ...string) error {
2525
fmt.Fprintf(d.writer, "Skipping %s because it is not a versioned package\n", pkg)
2626
continue
2727
}
28-
existing := d.lockfile.Entry(pkg)
28+
existing := d.lockfile.Packages[pkg]
2929
newEntry, err := d.lockfile.ForceResolve(pkg)
3030
if err != nil {
3131
return err
File renamed without changes.

internal/lock/lockfile.go

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,10 @@ type File struct {
2626
}
2727

2828
type Package struct {
29-
LastModified string `json:"last_modified"`
30-
Resolved string `json:"resolved"`
31-
Version string `json:"version"`
29+
LastModified string `json:"last_modified,omitempty"`
30+
PluginVersion string `json:"plugin_version,omitempty"`
31+
Resolved string `json:"resolved,omitempty"`
32+
Version string `json:"version,omitempty"`
3233
}
3334

3435
func GetFile(project devboxProject, resolver resolver) (*File, error) {
@@ -51,10 +52,8 @@ func GetFile(project devboxProject, resolver resolver) (*File, error) {
5152

5253
func (l *File) Add(pkgs ...string) error {
5354
for _, p := range pkgs {
54-
if IsVersionedPackage(p) {
55-
if _, err := l.Resolve(p); err != nil {
56-
return err
57-
}
55+
if _, err := l.Resolve(p); err != nil {
56+
return err
5857
}
5958
}
6059
return l.Save()
@@ -71,14 +70,14 @@ func (l *File) Remove(pkgs ...string) error {
7170
// This avoids writing values that may need to be removed in case of error.
7271
func (l *File) Resolve(pkg string) (*Package, error) {
7372
if entry, ok := l.Packages[pkg]; !ok || entry.Resolved == "" {
74-
var locked *Package
73+
locked := &Package{}
7574
var err error
7675
if IsVersionedPackage(pkg) {
7776
locked, err = l.resolver.Resolve(pkg)
7877
if err != nil {
7978
return nil, err
8079
}
81-
} else {
80+
} else if IsLegacyPackage(pkg) {
8281
// These are legacy packages without a version. Resolve to nixpkgs with
8382
// whatever hash is in the devbox.json
8483
locked = &Package{Resolved: l.LegacyNixpkgsPath(pkg)}
@@ -94,10 +93,6 @@ func (l *File) ForceResolve(pkg string) (*Package, error) {
9493
return l.Resolve(pkg)
9594
}
9695

97-
func (l *File) Entry(pkg string) *Package {
98-
return l.Packages[pkg]
99-
}
100-
10196
func (l *File) Save() error {
10297
// Never write lockfile if versioned packages is not enabled
10398
if !featureflag.LockFile.Enabled() {
@@ -120,6 +115,18 @@ func IsVersionedPackage(pkg string) bool {
120115
return found && name != "" && version != ""
121116
}
122117

118+
// This probably belongs in input.go but can't add it there because it will
119+
// create a circular dependency. We could move Input into own package.
120+
func IsLegacyPackage(pkg string) bool {
121+
return !IsVersionedPackage(pkg) &&
122+
!strings.Contains(pkg, ":") &&
123+
// We don't support absolute paths without "path:" prefix, but adding here
124+
// just inc ase we ever do.
125+
// Landau note: I don't think we should support it, it's hard to read and a
126+
// bit ambiguous.
127+
!strings.HasPrefix(pkg, "/")
128+
}
129+
123130
func lockFilePath(project devboxProject) string {
124131
return filepath.Join(project.ProjectDir(), "devbox.lock")
125132
}

internal/plugin/includes.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
package plugin
2+
3+
import (
4+
"strings"
5+
6+
"go.jetpack.io/devbox/internal/boxcli/usererr"
7+
"go.jetpack.io/devbox/internal/nix"
8+
)
9+
10+
func (m *Manager) parseInclude(include string) (*nix.Input, error) {
11+
includeType, name, _ := strings.Cut(include, ":")
12+
if includeType != "plugin" {
13+
return nil, usererr.New("unknown include type %q", includeType)
14+
} else if name == "" {
15+
return nil, usererr.New("include name is required")
16+
}
17+
return nix.InputFromString(name, m.lockfile), nil
18+
}

internal/plugin/manager.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@
33

44
package plugin
55

6+
import "go.jetpack.io/devbox/internal/lock"
7+
68
type Manager struct {
7-
addMode bool
9+
lockfile *lock.File
810
}
911

1012
type managerOption func(*Manager)
@@ -15,9 +17,9 @@ func NewManager(opts ...managerOption) *Manager {
1517
return m
1618
}
1719

18-
func WithAddMode() managerOption {
20+
func WithLockfile(lockfile *lock.File) managerOption {
1921
return func(m *Manager) {
20-
m.addMode = true
22+
m.lockfile = lockfile
2123
}
2224
}
2325

0 commit comments

Comments
 (0)