Skip to content

Commit 0e947c8

Browse files
LucilleHLucilleH
authored
Change the merging logic of devbox.json and planners (#103)
## Summary We have decided on the following merging logic between `devbox.json` and `planners`: - devbox.json `packages` field will merge with planners `devPackages` and `runtimePackages`. Merge will be `appendWithSlice`. - devbox.json `start_stage`, `build_stage` and `install_stage` subfields (shared plan): - if empty, will inherit the corresponding planners fields - if set, will override corresponding planners fields TODO (Will follow up with separate PRs) - devbox.json schema would be a separate one from the planner ## How was it tested? go build -o devbox cmd/devbox/main.go devbox plan
1 parent 14a5aff commit 0e947c8

File tree

11 files changed

+203
-69
lines changed

11 files changed

+203
-69
lines changed

‎boxcli/plan.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@ func runPlanCmd(cmd *cobra.Command, args []string) error {
3030
return errors.WithStack(err)
3131
}
3232

33-
plan := box.Plan()
33+
plan, err := box.Plan()
34+
if err != nil {
35+
return errors.WithStack(err)
36+
}
3437
if plan.Invalid() {
3538
return plan.Error()
3639
}

‎devbox.go‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,19 +100,23 @@ func (d *Devbox) Build(opts ...docker.BuildOptions) error {
100100

101101
// Plan creates a plan of the actions that devbox will take to generate its
102102
// environment.
103-
func (d *Devbox) Plan() *plansdk.Plan {
104-
basePlan := &plansdk.Plan{
103+
func (d *Devbox) Plan() (*plansdk.Plan, error) {
104+
userPlan := &plansdk.Plan{
105105
DevPackages: d.cfg.Packages,
106106
RuntimePackages: d.cfg.Packages,
107107
SharedPlan: d.cfg.SharedPlan,
108108
}
109-
return plansdk.MergePlans(basePlan, planner.GetPlan(d.srcDir))
109+
110+
return plansdk.MergeUserPlan(userPlan, planner.GetPlan(d.srcDir))
110111
}
111112

112113
// Generate creates the directory of Nix files and the Dockerfile that define
113114
// the devbox environment.
114115
func (d *Devbox) Generate() error {
115-
plan := d.Plan()
116+
plan, err := d.Plan()
117+
if err != nil {
118+
return errors.WithStack(err)
119+
}
116120
if plan.Invalid() {
117121
return plan.Error()
118122
}
@@ -122,11 +126,14 @@ func (d *Devbox) Generate() error {
122126
// Shell generates the devbox environment and launches nix-shell as a child
123127
// process.
124128
func (d *Devbox) Shell() error {
125-
plan := d.Plan()
129+
plan, err := d.Plan()
130+
if err != nil {
131+
return errors.WithStack(err)
132+
}
126133
if plan.Invalid() {
127134
return plan.Error()
128135
}
129-
err := generate(d.srcDir, plan, shellFiles)
136+
err = generate(d.srcDir, plan, shellFiles)
130137
if err != nil {
131138
return errors.WithStack(err)
132139
}

‎devbox_test.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ func testExample(t *testing.T, testPath string) {
3535

3636
box, err := Open(baseDir)
3737
assert.NoErrorf(err, "%s should be a valid devbox project", baseDir)
38-
plan := box.Plan()
38+
plan, err := box.Plan()
39+
assert.NoError(err, "devbox plan should not fail")
3940

4041
err = box.Generate()
4142
assert.NoError(err, "devbox generate should not fail")

‎planner/languages/golang/golang_planner.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,15 @@ func (p *Planner) GetPlan(srcDir string) *plansdk.Plan {
4242
},
4343
SharedPlan: plansdk.SharedPlan{
4444
InstallStage: &plansdk.Stage{
45-
Command: "go get",
45+
InputFiles: []string{"."},
46+
Command: "go get",
4647
},
4748
BuildStage: &plansdk.Stage{
4849
Command: "CGO_ENABLED=0 go build -o app",
4950
},
5051
StartStage: &plansdk.Stage{
51-
Command: "./app",
52+
InputFiles: []string{"."},
53+
Command: "./app",
5254
},
5355
},
5456
}

‎planner/languages/javascript/nodejs_planner.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ func (p *Planner) GetPlan(srcDir string) *plansdk.Plan {
4949
},
5050

5151
StartStage: &plansdk.Stage{
52-
Command: p.startCommand(pkgManager, project),
52+
InputFiles: []string{"."},
53+
Command: p.startCommand(pkgManager, project),
5354
},
5455
},
5556
}

‎planner/languages/php/php_planner.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,12 @@ func (p *Planner) GetPlan(srcDir string) *plansdk.Plan {
5656
}
5757

5858
plan.InstallStage = &plansdk.Stage{
59-
Command: "composer install --no-dev --no-ansi",
59+
InputFiles: []string{"."},
60+
Command: "composer install --no-dev --no-ansi",
6061
}
6162
plan.StartStage = &plansdk.Stage{
62-
Command: "php -S 0.0.0.0:8080 -t public",
63+
InputFiles: []string{"."},
64+
Command: "php -S 0.0.0.0:8080 -t public",
6365
}
6466
return plan
6567
}

‎planner/languages/python/python_poetry_planner.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ func (p *Planner) GetPlan(srcDir string) *plansdk.Plan {
4545
}
4646

4747
plan.InstallStage = &plansdk.Stage{
48+
InputFiles: []string{"."},
4849
// pex is is incompatible with certain less common python versions,
4950
// but because versions are sometimes expressed open-ended (e.g. ^3.10)
5051
// It will cause `poetry add pex` to fail. One solution is to use: --version
@@ -53,7 +54,10 @@ func (p *Planner) GetPlan(srcDir string) *plansdk.Plan {
5354
"poetry install --no-dev -n --no-ansi",
5455
}
5556
plan.BuildStage = &plansdk.Stage{Command: p.buildCommand(srcDir)}
56-
plan.StartStage = &plansdk.Stage{Command: "python ./app.pex"}
57+
plan.StartStage = &plansdk.Stage{
58+
InputFiles: []string{"."},
59+
Command: "python ./app.pex",
60+
}
5761
return plan
5862
}
5963

‎planner/plansdk/plansdk.go‎

Lines changed: 35 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -122,32 +122,54 @@ func MergePlans(plans ...*Plan) *Plan {
122122
plan := &Plan{
123123
DevPackages: []string{},
124124
RuntimePackages: []string{},
125-
SharedPlan: SharedPlan{
126-
InstallStage: &Stage{},
127-
BuildStage: &Stage{},
128-
StartStage: &Stage{},
129-
},
130125
}
131126
for _, p := range plans {
132-
err := mergo.Merge(plan, p, mergo.WithAppendSlice)
127+
err := mergo.Merge(
128+
plan,
129+
&Plan{
130+
DevPackages: p.DevPackages,
131+
RuntimePackages: p.RuntimePackages,
132+
},
133+
// Only WithAppendSlice the dev and runtime packages field.
134+
mergo.WithAppendSlice,
135+
)
133136
if err != nil {
134137
panic(err) // TODO: propagate error.
135138
}
136139
}
137140

138141
plan.DevPackages = pkgslice.Unique(plan.DevPackages)
139142
plan.RuntimePackages = pkgslice.Unique(plan.RuntimePackages)
143+
plan.SharedPlan = findBuildablePlan(plans...).SharedPlan
140144

141-
// Set default files for install stage to copy.
142-
if plan.SharedPlan.InstallStage.InputFiles == nil {
143-
plan.SharedPlan.InstallStage.InputFiles = []string{"."}
145+
return plan
146+
}
147+
148+
func findBuildablePlan(plans ...*Plan) *Plan {
149+
for _, p := range plans {
150+
// For now, pick the first buildable plan.
151+
if p.Buildable() {
152+
return p
153+
}
144154
}
145-
// Set default files for install stage to copy over from build step.
146-
if plan.SharedPlan.StartStage.InputFiles == nil {
147-
plan.SharedPlan.StartStage.InputFiles = []string{"."}
155+
return &Plan{}
156+
}
157+
158+
func MergeUserPlan(userPlan *Plan, automatedPlan *Plan) (*Plan, error) {
159+
plan := MergePlans(userPlan, automatedPlan)
160+
sharedPlan := &Plan{
161+
SharedPlan: userPlan.SharedPlan,
162+
}
163+
// fields in sharedPlan:
164+
// if empty, will inherit the corresponding fields in the automatedPlan
165+
// if set, will override corresponding automatedPlan fields
166+
if err := mergo.Merge(sharedPlan, automatedPlan); err != nil {
167+
return nil, err
148168
}
149169

150-
return plan
170+
plan.SharedPlan = sharedPlan.SharedPlan
171+
172+
return plan, nil
151173
}
152174

153175
func (p PlanError) MarshalJSON() ([]byte, error) {

‎planner/plansdk/plansdk_test.go‎

Lines changed: 130 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -22,19 +22,7 @@ func TestMergePlans(t *testing.T) {
2222
expected := &Plan{
2323
DevPackages: []string{"foo", "bar", "baz"},
2424
RuntimePackages: []string{"a", "b", "c"},
25-
SharedPlan: SharedPlan{
26-
InstallStage: &Stage{
27-
Command: "",
28-
InputFiles: []string{"."},
29-
},
30-
BuildStage: &Stage{
31-
Command: "",
32-
},
33-
StartStage: &Stage{
34-
Command: "",
35-
InputFiles: []string{"."},
36-
},
37-
},
25+
SharedPlan: SharedPlan{},
3826
}
3927
actual := MergePlans(plan1, plan2)
4028
assert.Equal(t, expected, actual)
@@ -58,17 +46,9 @@ func TestMergePlans(t *testing.T) {
5846
DevPackages: []string{},
5947
RuntimePackages: []string{},
6048
SharedPlan: SharedPlan{
61-
InstallStage: &Stage{
62-
Command: "",
63-
InputFiles: []string{"."},
64-
},
6549
BuildStage: &Stage{
6650
Command: "plan1",
6751
},
68-
StartStage: &Stage{
69-
Command: "",
70-
InputFiles: []string{"."},
71-
},
7252
},
7353
}
7454
actual = MergePlans(plan1, plan2)
@@ -93,18 +73,142 @@ func TestMergePlans(t *testing.T) {
9373
RuntimePackages: []string{},
9474
SharedPlan: SharedPlan{
9575
InstallStage: &Stage{
96-
Command: "",
9776
InputFiles: []string{"package.json"},
9877
},
99-
BuildStage: &Stage{
100-
Command: "",
101-
},
10278
StartStage: &Stage{
103-
Command: "",
10479
InputFiles: []string{"input"},
10580
},
10681
},
10782
}
10883
actual = MergePlans(plan1, plan2)
10984
assert.Equal(t, expected, actual)
11085
}
86+
87+
func TestMergeUserPlans(t *testing.T) {
88+
plannerPlan := &Plan{
89+
DevPackages: []string{"nodejs"},
90+
RuntimePackages: []string{"nodejs"},
91+
SharedPlan: SharedPlan{
92+
InstallStage: &Stage{
93+
InputFiles: []string{"package.json"},
94+
Command: "npm install",
95+
},
96+
BuildStage: &Stage{
97+
InputFiles: []string{"."},
98+
},
99+
StartStage: &Stage{
100+
InputFiles: []string{"."},
101+
Command: "npm start",
102+
},
103+
},
104+
}
105+
cases := []struct {
106+
name string
107+
in *Plan
108+
out *Plan
109+
}{
110+
{
111+
name: "empty base plan",
112+
in: &Plan{
113+
SharedPlan: SharedPlan{},
114+
},
115+
out: &Plan{
116+
DevPackages: []string{"nodejs"},
117+
RuntimePackages: []string{"nodejs"},
118+
SharedPlan: SharedPlan{
119+
InstallStage: &Stage{
120+
InputFiles: []string{"package.json"},
121+
Command: "npm install",
122+
},
123+
BuildStage: &Stage{
124+
InputFiles: []string{"."},
125+
},
126+
StartStage: &Stage{
127+
InputFiles: []string{"."},
128+
Command: "npm start",
129+
},
130+
},
131+
},
132+
},
133+
{
134+
name: "different input files",
135+
in: &Plan{
136+
DevPackages: []string{"nodejs", "yarn"},
137+
RuntimePackages: []string{"nodejs"},
138+
SharedPlan: SharedPlan{
139+
InstallStage: &Stage{
140+
InputFiles: []string{"package.json", "yarn.lock"},
141+
Command: "",
142+
},
143+
BuildStage: &Stage{
144+
InputFiles: []string{"."},
145+
Command: "",
146+
},
147+
StartStage: &Stage{
148+
InputFiles: []string{"."},
149+
Command: "npm start",
150+
},
151+
},
152+
},
153+
out: &Plan{
154+
DevPackages: []string{"nodejs", "yarn"},
155+
RuntimePackages: []string{"nodejs"},
156+
SharedPlan: SharedPlan{
157+
InstallStage: &Stage{
158+
InputFiles: []string{"package.json", "yarn.lock"},
159+
Command: "npm install",
160+
},
161+
BuildStage: &Stage{
162+
InputFiles: []string{"."},
163+
Command: "",
164+
},
165+
StartStage: &Stage{
166+
InputFiles: []string{"."},
167+
Command: "npm start",
168+
},
169+
},
170+
},
171+
},
172+
{
173+
name: "custom build command",
174+
in: &Plan{
175+
SharedPlan: SharedPlan{
176+
InstallStage: &Stage{
177+
InputFiles: []string{"app"},
178+
},
179+
BuildStage: &Stage{
180+
Command: "npm run build",
181+
},
182+
},
183+
},
184+
out: &Plan{
185+
DevPackages: []string{"nodejs"},
186+
RuntimePackages: []string{"nodejs"},
187+
SharedPlan: SharedPlan{
188+
InstallStage: &Stage{
189+
InputFiles: []string{"app"},
190+
Command: "npm install",
191+
},
192+
BuildStage: &Stage{
193+
InputFiles: []string{"."},
194+
Command: "npm run build",
195+
},
196+
StartStage: &Stage{
197+
InputFiles: []string{"."},
198+
Command: "npm start",
199+
},
200+
},
201+
},
202+
},
203+
}
204+
205+
for _, tc := range cases {
206+
t.Run(tc.name, func(t *testing.T) {
207+
assert := assert.New(t)
208+
got, err := MergeUserPlan(tc.in, plannerPlan)
209+
210+
assert.NoError(err)
211+
assert.Equal(tc.out, got, "plans should match")
212+
})
213+
}
214+
}

0 commit comments

Comments
 (0)