Skip to content

Commit bb67992

Browse files
authored
[init] init config should return error if already exists (#2346)
## Summary This fixes a NPE in autodetect. Previously when initializing a config with a directory that already contained a config we would return `(nil, nil)` but this is a mistake and a bit inconsistent with typical go. This changes it so that if config already exists we return an error instead. IMO this also provides better UX displaying a message indicating config already exists. This does have a minor backward compatibility issue. If users where used to calling `devbox init` and having it not fail they would now need to first test if devbox.json already exists. ## How was it tested?
1 parent 0b2e47e commit bb67992

File tree

6 files changed

+24
-10
lines changed

6 files changed

+24
-10
lines changed

internal/boxcli/global.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func ensureGlobalConfig() (string, error) {
6363
if err != nil {
6464
return "", err
6565
}
66-
err = devbox.InitConfig(globalConfigPath)
66+
err = devbox.EnsureConfig(globalConfigPath)
6767
if err != nil {
6868
return "", err
6969
}

internal/boxcli/init.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ package boxcli
55

66
import (
77
"fmt"
8+
"os"
89

910
"github.com/pkg/errors"
1011
"github.com/spf13/cobra"
1112

1213
"go.jetpack.io/devbox/internal/devbox"
14+
"go.jetpack.io/devbox/internal/ux"
1315
"go.jetpack.io/devbox/pkg/autodetect"
1416
)
1517

@@ -28,7 +30,16 @@ func initCmd() *cobra.Command {
2830
"You can then add packages using `devbox add`",
2931
Args: cobra.MaximumNArgs(1),
3032
RunE: func(cmd *cobra.Command, args []string) error {
31-
return runInitCmd(cmd, args, flags)
33+
err := runInitCmd(cmd, args, flags)
34+
if errors.Is(err, os.ErrExist) {
35+
path := pathArg(args)
36+
if path == "" || path == "." {
37+
path, _ = os.Getwd()
38+
}
39+
ux.Fwarningf(cmd.ErrOrStderr(), "devbox.json already exists in %q.", path)
40+
err = nil
41+
}
42+
return err
3243
},
3344
}
3445

internal/devbox/devbox.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,14 @@ func InitConfig(dir string) error {
7676
return err
7777
}
7878

79+
func EnsureConfig(dir string) error {
80+
err := InitConfig(dir)
81+
if err != nil && !errors.Is(err, os.ErrExist) {
82+
return err
83+
}
84+
return nil
85+
}
86+
7987
func Open(opts *devopt.Opts) (*Devbox, error) {
8088
var cfg *devconfig.Config
8189
var err error

internal/devbox/util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ func ensureDevboxUtilityConfig() (string, error) {
5555
return "", err
5656
}
5757

58-
err = InitConfig(path)
58+
err = EnsureConfig(path)
5959
if err != nil {
6060
return "", err
6161
}

internal/devconfig/init.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package devconfig
55

66
import (
7-
"errors"
87
"os"
98
"path/filepath"
109

@@ -17,11 +16,6 @@ func Init(dir string) (*Config, error) {
1716
os.O_RDWR|os.O_CREATE|os.O_EXCL,
1817
0o644,
1918
)
20-
if errors.Is(err, os.ErrExist) {
21-
// TODO: Should we return an error here?
22-
// If we do, it breaks a bunch of tests, but it's likely the correct behavior
23-
return nil, nil
24-
}
2519
if err != nil {
2620
return nil, err
2721
}

testscripts/init/empty.test.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@ exists devbox.json
66

77
json.superset devbox.json expected.json
88

9-
# Second init should be a no-op.
9+
# Second init should be a no-op with a warning
1010
exec devbox init
11+
stderr 'devbox.json already exists in'
1112

1213
-- expected.json --
1314
{

0 commit comments

Comments
 (0)