Skip to content

Commit 7b1e7d1

Browse files
mohsenarimohsenari
committed
Addressed PR comments
1 parent 31ecdba commit 7b1e7d1

File tree

5 files changed

+50
-50
lines changed

5 files changed

+50
-50
lines changed

‎devbox.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ type Devbox interface {
2525
// Generate creates the directory of Nix files and the Dockerfile that define
2626
// the devbox environment.
2727
Generate(ctx context.Context) error
28-
GenerateDevcontainer(ctx context.Context, force bool) error
29-
GenerateDockerfile(ctx context.Context, force bool) error
28+
GenerateDevcontainer(ctx context.Context) error
29+
GenerateDockerfile(ctx context.Context) error
3030
GenerateEnvrcFile(ctx context.Context, force bool, envFlags devopt.EnvFlags) error
3131
Info(ctx context.Context, pkg string, markdown bool) error
3232
Install(ctx context.Context) error

‎internal/boxcli/generate.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ func devcontainerCmd() *cobra.Command {
6565
}
6666
command.Flags().BoolVarP(
6767
&flags.force, "force", "f", false, "force overwrite on existing files")
68-
command.Flags().BoolVarP(
69-
&flags.rootUser, "root-user", "r", false, "Use root as default user inside the container")
68+
command.Flags().BoolVar(
69+
&flags.rootUser, "root-user", false, "Use root as default user inside the container")
7070
return command
7171
}
7272

@@ -84,8 +84,8 @@ func dockerfileCmd() *cobra.Command {
8484
}
8585
command.Flags().BoolVarP(
8686
&flags.force, "force", "f", false, "force overwrite existing files")
87-
command.Flags().BoolVarP(
88-
&flags.rootUser, "root-user", "r", false, "Use root as default user inside the container")
87+
command.Flags().BoolVar(
88+
&flags.rootUser, "root-user", false, "Use root as default user inside the container")
8989
flags.config.register(command)
9090
return command
9191
}
@@ -140,6 +140,10 @@ func runGenerateCmd(cmd *cobra.Command, flags *generateCmdFlags) error {
140140
box, err := devbox.Open(&devopt.Opts{
141141
Dir: flags.config.path,
142142
Writer: cmd.ErrOrStderr(),
143+
GenerateOpts: devopt.GenerateOpts{
144+
Force: flags.force,
145+
RootUser: flags.rootUser,
146+
},
143147
})
144148
if err != nil {
145149
return errors.WithStack(err)
@@ -148,9 +152,9 @@ func runGenerateCmd(cmd *cobra.Command, flags *generateCmdFlags) error {
148152
case "debug":
149153
return box.Generate(cmd.Context())
150154
case "devcontainer":
151-
return box.GenerateDevcontainer(cmd.Context(), flags.force, flags.rootUser)
155+
return box.GenerateDevcontainer(cmd.Context())
152156
case "dockerfile":
153-
return box.GenerateDockerfile(cmd.Context(), flags.force, flags.rootUser)
157+
return box.GenerateDockerfile(cmd.Context())
154158
}
155159
return nil
156160
}

‎internal/impl/devbox.go

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ type Devbox struct {
6161
pure bool
6262
allowInsecureAdds bool
6363
customProcessComposeFile string
64+
GenerateOpts *devopt.GenerateOpts
6465

6566
// Possible TODO: hardcode this to stderr. Allowing the caller to specify the
6667
// writer is error prone. Since it is almost always stderr, we should default
@@ -93,6 +94,10 @@ func Open(opts *devopt.Opts) (*Devbox, error) {
9394
pure: opts.Pure,
9495
customProcessComposeFile: opts.CustomProcessComposeFile,
9596
allowInsecureAdds: opts.AllowInsecureAdds,
97+
GenerateOpts: &devopt.GenerateOpts{
98+
Force: opts.GenerateOpts.Force,
99+
RootUser: opts.GenerateOpts.RootUser,
100+
},
96101
}
97102

98103
lock, err := lock.GetFile(box)
@@ -370,7 +375,7 @@ func (d *Devbox) Info(ctx context.Context, pkg string, markdown bool) error {
370375

371376
// GenerateDevcontainer generates devcontainer.json and Dockerfile for vscode run-in-container
372377
// and GitHub Codespaces
373-
func (d *Devbox) GenerateDevcontainer(ctx context.Context, force bool, rootUser bool) error {
378+
func (d *Devbox) GenerateDevcontainer(ctx context.Context) error {
374379
ctx, task := trace.NewTask(ctx, "devboxGenerateDevcontainer")
375380
defer task.End()
376381

@@ -381,7 +386,7 @@ func (d *Devbox) GenerateDevcontainer(ctx context.Context, force bool, rootUser
381386

382387
// check if devcontainer.json or Dockerfile exist
383388
filesExist := fileutil.Exists(devContainerJSONPath) || fileutil.Exists(dockerfilePath)
384-
if !force && filesExist {
389+
if !d.GenerateOpts.Force && filesExist {
385390
return usererr.New(
386391
"Files devcontainer.json or Dockerfile are already present in .devcontainer/. " +
387392
"Remove the files or use --force to overwrite them.",
@@ -394,21 +399,24 @@ func (d *Devbox) GenerateDevcontainer(ctx context.Context, force bool, rootUser
394399
return redact.Errorf("error creating dev container directory in <project>/%s: %w",
395400
redact.Safe(filepath.Base(devContainerPath)), err)
396401
}
397-
isDevcontainer := true
398402

399403
// Setup generate parameters
400-
g := generate.Open(
401-
ctx, devContainerPath, rootUser, isDevcontainer, d.Packages(), d.getLocalFlakesDirs(),
402-
)
404+
g := &generate.Options{
405+
Path: devContainerPath,
406+
RootUser: d.GenerateOpts.RootUser,
407+
IsDevcontainer: true,
408+
Pkgs: d.Packages(),
409+
LocalFlakeDirs: d.getLocalFlakesDirs(),
410+
}
403411

404412
// generate dockerfile
405-
err = g.CreateDockerfile()
413+
err = g.CreateDockerfile(ctx)
406414
if err != nil {
407415
return redact.Errorf("error generating dev container Dockerfile in <project>/%s: %w",
408416
redact.Safe(filepath.Base(devContainerPath)), err)
409417
}
410418
// generate devcontainer.json
411-
err = g.CreateDevcontainer()
419+
err = g.CreateDevcontainer(ctx)
412420
if err != nil {
413421
return redact.Errorf("error generating devcontainer.json in <project>/%s: %w",
414422
redact.Safe(filepath.Base(devContainerPath)), err)
@@ -417,28 +425,31 @@ func (d *Devbox) GenerateDevcontainer(ctx context.Context, force bool, rootUser
417425
}
418426

419427
// GenerateDockerfile generates a Dockerfile that replicates the devbox shell
420-
func (d *Devbox) GenerateDockerfile(ctx context.Context, force bool, rootUser bool) error {
428+
func (d *Devbox) GenerateDockerfile(ctx context.Context) error {
421429
ctx, task := trace.NewTask(ctx, "devboxGenerateDockerfile")
422430
defer task.End()
423431

424432
dockerfilePath := filepath.Join(d.projectDir, "Dockerfile")
425433
// check if Dockerfile doesn't exist
426434
filesExist := fileutil.Exists(dockerfilePath)
427-
if !force && filesExist {
435+
if !d.GenerateOpts.Force && filesExist {
428436
return usererr.New(
429437
"Dockerfile is already present in the current directory. " +
430438
"Remove it or use --force to overwrite it.",
431439
)
432440
}
433-
isDevcontainer := false
434441

435442
// Setup Generate parameters
436-
g := generate.Open(
437-
ctx, d.projectDir, rootUser, isDevcontainer, d.Packages(), d.getLocalFlakesDirs(),
438-
)
443+
g := &generate.Options{
444+
Path: d.projectDir,
445+
RootUser: d.GenerateOpts.RootUser,
446+
IsDevcontainer: false,
447+
Pkgs: d.Packages(),
448+
LocalFlakeDirs: d.getLocalFlakesDirs(),
449+
}
439450

440451
// generate dockerfile
441-
return errors.WithStack(g.CreateDockerfile())
452+
return errors.WithStack(g.CreateDockerfile(ctx))
442453
}
443454

444455
func PrintEnvrcContent(w io.Writer, envFlags devopt.EnvFlags) error {

‎internal/impl/devopt/devboxopts.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,11 @@ import (
44
"io"
55
)
66

7+
type GenerateOpts struct {
8+
Force bool
9+
RootUser bool
10+
}
11+
712
type Opts struct {
813
AllowInsecureAdds bool
914
Dir string
@@ -12,6 +17,7 @@ type Opts struct {
1217
IgnoreWarnings bool
1318
CustomProcessComposeFile string
1419
Writer io.Writer
20+
GenerateOpts GenerateOpts
1521
}
1622

1723
type EnvFlags struct {

‎internal/impl/generate/devcontainer_util.go

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,7 @@ import (
2525
//go:embed tmpl/*
2626
var tmplFS embed.FS
2727

28-
type Generate struct {
29-
Ctx context.Context
28+
type Options struct {
3029
Path string
3130
RootUser bool
3231
IsDevcontainer bool
@@ -61,28 +60,9 @@ type dockerfileData struct {
6160
LocalFlakeDirs []string
6261
}
6362

64-
func Open(
65-
ctx context.Context,
66-
path string,
67-
rootUser bool,
68-
isDevcontainer bool,
69-
pkgs []string,
70-
localFlakeDirs []string,
71-
) *Generate {
72-
73-
return &Generate{
74-
Ctx: ctx,
75-
Path: path,
76-
RootUser: rootUser,
77-
IsDevcontainer: isDevcontainer,
78-
Pkgs: pkgs,
79-
LocalFlakeDirs: localFlakeDirs,
80-
}
81-
}
82-
8363
// CreateDockerfile creates a Dockerfile in path and writes devcontainerDockerfile.tmpl's content into it
84-
func (g *Generate) CreateDockerfile() error {
85-
defer trace.StartRegion(g.Ctx, "createDockerfile").End()
64+
func (g *Options) CreateDockerfile(ctx context.Context) error {
65+
defer trace.StartRegion(ctx, "createDockerfile").End()
8666

8767
// create dockerfile
8868
file, err := os.Create(filepath.Join(g.Path, "Dockerfile"))
@@ -102,8 +82,8 @@ func (g *Generate) CreateDockerfile() error {
10282
}
10383

10484
// CreateDevcontainer creates a devcontainer.json in path and writes getDevcontainerContent's output into it
105-
func (g *Generate) CreateDevcontainer() error {
106-
defer trace.StartRegion(g.Ctx, "createDevcontainer").End()
85+
func (g *Options) CreateDevcontainer(ctx context.Context) error {
86+
defer trace.StartRegion(ctx, "createDevcontainer").End()
10787

10888
// create devcontainer.json file
10989
file, err := os.Create(filepath.Join(g.Path, "devcontainer.json"))
@@ -151,7 +131,7 @@ func CreateEnvrc(ctx context.Context, path string, envFlags devopt.EnvFlags) err
151131
})
152132
}
153133

154-
func (g *Generate) getDevcontainerContent() *devcontainerObject {
134+
func (g *Options) getDevcontainerContent() *devcontainerObject {
155135
// object that gets written in devcontainer.json
156136
devcontainerContent := &devcontainerObject{
157137
// For format details, see https://aka.ms/devcontainer.json. For config options, see the README at:
@@ -172,7 +152,6 @@ func (g *Generate) getDevcontainerContent() *devcontainerObject {
172152
},
173153
},
174154
},
175-
// Comment out to connect as root instead. More info: https://aka.ms/vscode-remote/containers/non-root.
176155
RemoteUser: "devbox",
177156
}
178157
if g.RootUser {

0 commit comments

Comments
 (0)