diff --git a/commands/pkg/get/cmdget.go b/commands/pkg/get/cmdget.go index 7158049fc..3ea0dc66c 100644 --- a/commands/pkg/get/cmdget.go +++ b/commands/pkg/get/cmdget.go @@ -76,6 +76,8 @@ type Runner struct { func (r *Runner) preRunE(_ *cobra.Command, args []string) error { const op errors.Op = "cmdget.preRunE" + // Track if destination was explicitly provided + explicitDest := len(args) > 1 if len(args) == 1 { args = append(args, pkg.CurDir) } else { @@ -88,7 +90,7 @@ func (r *Runner) preRunE(_ *cobra.Command, args []string) error { args[1] = resolvedPath } } - t, err := parse.GitParseArgs(r.ctx, args) + t, err := parse.GitParseArgs(r.ctx, args, explicitDest) if err != nil { return errors.E(op, err) } diff --git a/commands/pkg/get/cmdget_test.go b/commands/pkg/get/cmdget_test.go index 8c8a5f2d3..d0ca7f035 100644 --- a/commands/pkg/get/cmdget_test.go +++ b/commands/pkg/get/cmdget_test.go @@ -111,7 +111,7 @@ func TestCmdMainBranch_execute(t *testing.T) { } r := get.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") - r.Command.SetArgs([]string{"file://" + g.RepoDirectory + ".git/", "./"}) + r.Command.SetArgs([]string{"file://" + g.RepoDirectory + ".git/"}) err = r.Command.Execute() assert.NoError(t, err) @@ -159,9 +159,9 @@ func TestCmd_fail(t *testing.T) { r := get.NewRunner(fake.CtxWithDefaultPrinter(), "kpt") r.Command.SilenceErrors = true r.Command.SilenceUsage = true - r.Command.SetArgs([]string{"file://" + filepath.Join("not", "real", "dir") + ".git/@master", "./"}) + r.Command.SetArgs([]string{"file://" + filepath.Join("not", "real", "dir") + ".git/@master", "nonexistent"}) - defer os.RemoveAll("dir") + defer os.RemoveAll("nonexistent") err := r.Command.Execute() if !assert.Error(t, err) { @@ -234,7 +234,7 @@ func TestCmd_Execute_flagAndArgParsing(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "master", r.Get.Git.Ref) assert.Equal(t, "something://foo", r.Get.Git.Repo) - assert.Equal(t, filepath.Join(pathPrefix, w.WorkspaceDirectory, "foo"), r.Get.Destination) + assert.Equal(t, filepath.Join(pathPrefix, w.WorkspaceDirectory), r.Get.Destination) }, }, "repo arg is split up correctly into ref, directory and repo": { @@ -247,10 +247,10 @@ func TestCmd_Execute_flagAndArgParsing(t *testing.T) { assert.Equal(t, fmt.Sprintf("file://%s", repo), r.Get.Git.Repo) assert.Equal(t, "master", r.Get.Git.Ref) assert.Equal(t, "/blueprints/java", r.Get.Git.Directory) - assert.Equal(t, filepath.Join(pathPrefix, w.WorkspaceDirectory, "java"), r.Get.Destination) + assert.Equal(t, filepath.Join(pathPrefix, w.WorkspaceDirectory), r.Get.Destination) }, }, - "current working dir -- should use package name": { + "current working dir -- should use current directory directly": { argsFunc: func(repo, _ string) []string { return []string{fmt.Sprintf("file://%s.git/blueprints/java", repo), "foo/../bar/../"} }, @@ -260,7 +260,7 @@ func TestCmd_Execute_flagAndArgParsing(t *testing.T) { assert.Equal(t, fmt.Sprintf("file://%s", repo), r.Get.Git.Repo) assert.Equal(t, "master", r.Get.Git.Ref) assert.Equal(t, "/blueprints/java", r.Get.Git.Directory) - assert.Equal(t, filepath.Join(pathPrefix, w.WorkspaceDirectory, "java"), r.Get.Destination) + assert.Equal(t, filepath.Join(pathPrefix, w.WorkspaceDirectory), r.Get.Destination) }, }, "clean relative path": { diff --git a/internal/util/get/get.go b/internal/util/get/get.go index db9ea9d50..98be8e49a 100644 --- a/internal/util/get/get.go +++ b/internal/util/get/get.go @@ -70,12 +70,30 @@ func (c Command) Run(ctx context.Context) error { return errors.E(op, err) } - if _, err := os.Stat(c.Destination); !goerrors.Is(err, os.ErrNotExist) { - return errors.E(op, errors.Exist, types.UniquePath(c.Destination), fmt.Errorf("destination directory already exists")) - } + destInfo, err := os.Stat(c.Destination) + switch { + case err == nil: + // Destination exists - check if it's an empty directory + if !destInfo.IsDir() { + return errors.E(op, errors.Exist, types.UniquePath(c.Destination), fmt.Errorf("destination exists and is not a directory")) + } - err := os.MkdirAll(c.Destination, 0700) - if err != nil { + // Check if directory is empty + entries, err := os.ReadDir(c.Destination) + if err != nil { + return errors.E(op, errors.IO, types.UniquePath(c.Destination), err) + } + if len(entries) > 0 { + return errors.E(op, errors.Exist, types.UniquePath(c.Destination), fmt.Errorf("destination directory already exists")) + } + // Directory exists but is empty, we can use it + case goerrors.Is(err, os.ErrNotExist): + // Directory doesn't exist, create it + err = os.MkdirAll(c.Destination, 0700) + if err != nil { + return errors.E(op, errors.IO, types.UniquePath(c.Destination), err) + } + default: return errors.E(op, errors.IO, types.UniquePath(c.Destination), err) } diff --git a/internal/util/parse/parse.go b/internal/util/parse/parse.go index 2048fb268..5a5a6e455 100644 --- a/internal/util/parse/parse.go +++ b/internal/util/parse/parse.go @@ -35,7 +35,7 @@ type Target struct { Destination string } -func GitParseArgs(ctx context.Context, args []string) (Target, error) { +func GitParseArgs(ctx context.Context, args []string, explicitDest bool) (Target, error) { g := Target{} if args[0] == "-" { return g, nil @@ -43,7 +43,7 @@ func GitParseArgs(ctx context.Context, args []string) (Target, error) { // Simple parsing if contains .git{$|/) if HasGitSuffix(args[0]) { - return targetFromPkgURL(ctx, args[0], args[1]) + return targetFromPkgURL(ctx, args[0], args[1], explicitDest) } // GitHub parsing if contains github.com @@ -52,7 +52,7 @@ func GitParseArgs(ctx context.Context, args []string) (Target, error) { if err != nil { return g, err } - return targetFromPkgURL(ctx, ghPkgURL, args[1]) + return targetFromPkgURL(ctx, ghPkgURL, args[1], explicitDest) } uri, version, err := getURIAndVersion(args[0]) @@ -75,7 +75,7 @@ func GitParseArgs(ctx context.Context, args []string) (Target, error) { version = defaultRef } - destination, err := getDest(args[1], repo, remoteDir) + destination, err := getDest(args[1], repo, remoteDir, explicitDest) if err != nil { return g, err } @@ -87,7 +87,7 @@ func GitParseArgs(ctx context.Context, args []string) (Target, error) { } // targetFromPkgURL parses a pkg url and destination into kptfile git info and local destination Target -func targetFromPkgURL(ctx context.Context, pkgURL string, dest string) (Target, error) { +func targetFromPkgURL(ctx context.Context, pkgURL string, dest string, explicitDest bool) (Target, error) { g := Target{} repo, dir, ref, err := URL(pkgURL) if err != nil { @@ -107,7 +107,7 @@ func targetFromPkgURL(ctx context.Context, pkgURL string, dest string) (Target, } ref = defaultRef } - destination, err := getDest(dest, repo, dir) + destination, err := getDest(dest, repo, dir, explicitDest) if err != nil { return g, err } @@ -255,7 +255,8 @@ func getRepoAndPkg(v string) (string, string, error) { return repoAndPkg[0], repoAndPkg[1], nil } -func getDest(v, repo, subdir string) (string, error) { +func getDest(v, repo, subdir string, explicitDest bool) (string, error) { + originalV := v v = filepath.Clean(v) f, err := os.Stat(v) @@ -274,6 +275,12 @@ func getDest(v, repo, subdir string) (string, error) { } // LOCATION EXISTS + // Check if user explicitly specified current directory (. or paths that resolve to .) + // to match git clone behavior + if explicitDest && (v == "." || originalV == "") { + return v, nil + } + // default the location to a new subdirectory matching the pkg URI base repo = strings.TrimSuffix(repo, "/") repo = strings.TrimSuffix(repo, ".git") diff --git a/internal/util/parse/parse_test.go b/internal/util/parse/parse_test.go index 45d2393b1..222800a21 100644 --- a/internal/util/parse/parse_test.go +++ b/internal/util/parse/parse_test.go @@ -240,7 +240,7 @@ func Test_GitParseArgs(t *testing.T) { Directory: "/", Ref: "main", }, - Destination: "kpt"}, + Destination: "."}, }, "starts with github.com": { ghURL: "https://github.com/kptdev/kpt", @@ -249,7 +249,7 @@ func Test_GitParseArgs(t *testing.T) { Directory: "/", Ref: "main", }, - Destination: "kpt"}, + Destination: "."}, }, } for name, test := range tests { @@ -258,7 +258,7 @@ func Test_GitParseArgs(t *testing.T) { t.SkipNow() } ctx := printer.WithContext(context.Background(), printer.New(nil, nil)) - actual, err := GitParseArgs(ctx, []string{test.ghURL, ""}) + actual, err := GitParseArgs(ctx, []string{test.ghURL, ""}, true) assert.NoError(t, err) assert.Equal(t, test.expected, actual) })