Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly read Git metadata when running inside workspace #1945

Merged
merged 25 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
31db923
Properly read git metadata when inside Workspace
denik Nov 27, 2024
edac960
Update bundle/bundle.go
denik Dec 2, 2024
4fa99d0
update comment
denik Dec 2, 2024
0ba2314
add test for FetchRepositoryInfo
denik Dec 2, 2024
b4251d7
make methods private
denik Dec 2, 2024
e49f2f3
rename fixResponsePath to ensureWorkspacePrefix
denik Dec 2, 2024
97c8042
reorder to improve readability
denik Dec 2, 2024
80d9b6a
Expand comment on WorktreeRoot
denik Dec 2, 2024
4250251
use t.Cleanup instead of defer
denik Dec 2, 2024
017836f
prefix integration tests with TestAcc
denik Dec 2, 2024
242b53a
remove incorrect info from the comment
denik Dec 2, 2024
d204a93
use path.Join to add /Workspace prefix
denik Dec 2, 2024
8fe958f
Do not fail on git operations in bundle and sync, log warnings instea…
denik Dec 2, 2024
4d38475
refactor test code a bit - extract helper assert groups
denik Dec 3, 2024
8a31be7
Add GuessedWorktreeRoot field; this simplifies usage on the caller si…
denik Dec 3, 2024
e7a49a2
clarify comment
denik Dec 3, 2024
c4c7a87
expand FileSet tests with cases root != worktreeRoot
denik Dec 3, 2024
a623cfd
Switch to string from vfs.Path in the signature
denik Dec 3, 2024
d0b088a
s/.git/[leafName]
denik Dec 4, 2024
5131488
clean up comment
denik Dec 4, 2024
1c3f5f2
clean up GuessedWorktreeRoot field; let the caller do the guessing
denik Dec 4, 2024
2d3ee18
don't use result in case of an error
denik Dec 4, 2024
a689396
s/GitRepositoryInfo/RepositoryInfo
denik Dec 4, 2024
a064d91
use Join instead of +
denik Dec 4, 2024
43da63b
lowercase variable
denik Dec 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,10 @@ type Bundle struct {
// Exclusively use this field for filesystem operations.
SyncRoot vfs.Path

// Path to the root of git worktree containing the bundle.
// https://git-scm.com/docs/git-worktree
WorktreeRoot vfs.Path

// Config contains the bundle configuration.
// It is loaded from the bundle configuration files and mutators may update it.
Config config.Root
Expand Down
4 changes: 4 additions & 0 deletions bundle/bundle_read_only.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ func (r ReadOnlyBundle) SyncRoot() vfs.Path {
return r.b.SyncRoot
}

func (r ReadOnlyBundle) WorktreeRoot() vfs.Path {
return r.b.WorktreeRoot
}

func (r ReadOnlyBundle) WorkspaceClient() *databricks.WorkspaceClient {
return r.b.WorkspaceClient()
}
Expand Down
49 changes: 22 additions & 27 deletions bundle/config/mutator/load_git_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/databricks/cli/bundle"
"github.com/databricks/cli/libs/diag"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/vfs"
)

type loadGitDetails struct{}
Expand All @@ -21,45 +21,40 @@ func (m *loadGitDetails) Name() string {
}

func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics {
// Load relevant git repository
repo, err := git.NewRepository(b.BundleRoot)
var diags diag.Diagnostics
info, err := git.FetchRepositoryInfo(ctx, b.BundleRoot.Native(), b.WorkspaceClient())
if err != nil {
return diag.FromErr(err)
diags = append(diags, diag.WarningFromErr(err)...)
}

// Read branch name of current checkout
branch, err := repo.CurrentBranch()
if err == nil {
b.Config.Bundle.Git.ActualBranch = branch
if b.Config.Bundle.Git.Branch == "" {
// Only load branch if there's no user defined value
b.Config.Bundle.Git.Inferred = true
b.Config.Bundle.Git.Branch = branch
}
if info.WorktreeRoot == "" {
b.WorktreeRoot = b.BundleRoot
} else {
log.Warnf(ctx, "failed to load current branch: %s", err)
b.WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
}

b.Config.Bundle.Git.ActualBranch = info.CurrentBranch
denik marked this conversation as resolved.
Show resolved Hide resolved
if b.Config.Bundle.Git.Branch == "" {
// Only load branch if there's no user defined value
b.Config.Bundle.Git.Inferred = true
b.Config.Bundle.Git.Branch = info.CurrentBranch
}

// load commit hash if undefined
if b.Config.Bundle.Git.Commit == "" {
commit, err := repo.LatestCommit()
if err != nil {
log.Warnf(ctx, "failed to load latest commit: %s", err)
} else {
b.Config.Bundle.Git.Commit = commit
}
b.Config.Bundle.Git.Commit = info.LatestCommit
}

// load origin url if undefined
if b.Config.Bundle.Git.OriginURL == "" {
remoteUrl := repo.OriginUrl()
b.Config.Bundle.Git.OriginURL = remoteUrl
b.Config.Bundle.Git.OriginURL = info.OriginURL
}

// repo.Root() returns the absolute path of the repo
relBundlePath, err := filepath.Rel(repo.Root(), b.BundleRoot.Native())
relBundlePath, err := filepath.Rel(b.WorktreeRoot.Native(), b.BundleRoot.Native())
if err != nil {
return diag.FromErr(err)
diags = append(diags, diag.FromErr(err)...)
} else {
b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath)
}
b.Config.Bundle.Git.BundleRootPath = filepath.ToSlash(relBundlePath)
return nil
return diags
}
1 change: 1 addition & 0 deletions bundle/config/validate/files_to_sync_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func setupBundleForFilesToSyncTest(t *testing.T) *bundle.Bundle {
BundleRoot: vfs.MustNew(dir),
SyncRootPath: dir,
SyncRoot: vfs.MustNew(dir),
WorktreeRoot: vfs.MustNew(dir),
Config: config.Root{
Bundle: config.Bundle{
Target: "default",
Expand Down
9 changes: 5 additions & 4 deletions bundle/deploy/files/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ func GetSyncOptions(ctx context.Context, rb bundle.ReadOnlyBundle) (*sync.SyncOp
}

opts := &sync.SyncOptions{
LocalRoot: rb.SyncRoot(),
Paths: rb.Config().Sync.Paths,
Include: includes,
Exclude: rb.Config().Sync.Exclude,
WorktreeRoot: rb.WorktreeRoot(),
LocalRoot: rb.SyncRoot(),
Paths: rb.Config().Sync.Paths,
Include: includes,
Exclude: rb.Config().Sync.Exclude,

RemotePath: rb.Config().Workspace.FilePath,
Host: rb.WorkspaceClient().Config.Host,
Expand Down
32 changes: 27 additions & 5 deletions cmd/sync/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (
"github.com/databricks/cli/bundle/deploy/files"
"github.com/databricks/cli/cmd/root"
"github.com/databricks/cli/libs/flags"
"github.com/databricks/cli/libs/git"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/sync"
"github.com/databricks/cli/libs/vfs"
"github.com/spf13/cobra"
Expand All @@ -37,6 +39,7 @@ func (f *syncFlags) syncOptionsFromBundle(cmd *cobra.Command, args []string, b *

opts.Full = f.full
opts.PollInterval = f.interval
opts.WorktreeRoot = b.WorktreeRoot
return opts, nil
}

Expand All @@ -60,11 +63,30 @@ func (f *syncFlags) syncOptionsFromArgs(cmd *cobra.Command, args []string) (*syn
}
}

ctx := cmd.Context()
client := root.WorkspaceClient(ctx)

localRoot := vfs.MustNew(args[0])
info, err := git.FetchRepositoryInfo(ctx, localRoot.Native(), client)

if err != nil {
log.Warnf(ctx, "Failed to read git info: %s", err)
}

var WorktreeRoot vfs.Path
denik marked this conversation as resolved.
Show resolved Hide resolved

if info.WorktreeRoot == "" {
WorktreeRoot = localRoot
} else {
WorktreeRoot = vfs.MustNew(info.WorktreeRoot)
}

opts := sync.SyncOptions{
LocalRoot: vfs.MustNew(args[0]),
Paths: []string{"."},
Include: nil,
Exclude: nil,
WorktreeRoot: WorktreeRoot,
LocalRoot: localRoot,
Paths: []string{"."},
Include: nil,
Exclude: nil,

RemotePath: args[1],
Full: f.full,
Expand All @@ -75,7 +97,7 @@ func (f *syncFlags) syncOptionsFromArgs(cmd *cobra.Command, args []string) (*syn
// The sync code will automatically create this directory if it doesn't
// exist and add it to the `.gitignore` file in the root.
SnapshotBasePath: filepath.Join(args[0], ".databricks"),
WorkspaceClient: root.WorkspaceClient(cmd.Context()),
WorkspaceClient: client,

OutputHandler: outputHandler,
}
Expand Down
172 changes: 172 additions & 0 deletions internal/git_fetch_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
package internal

import (
"os"
"os/exec"
"path"
"path/filepath"
"testing"

"github.com/databricks/cli/internal/acc"
"github.com/databricks/cli/libs/dbr"
"github.com/databricks/cli/libs/git"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

const examplesRepoUrl = "https://github.com/databricks/bundle-examples"
const examplesRepoProvider = "gitHub"

func assertFullGitInfo(t *testing.T, expectedRoot string, info git.RepositoryInfo) {
assert.Equal(t, "main", info.CurrentBranch)
assert.NotEmpty(t, info.LatestCommit)
assert.Equal(t, examplesRepoUrl, info.OriginURL)
assert.Equal(t, expectedRoot, info.WorktreeRoot)
}

func assertEmptyGitInfo(t *testing.T, info git.RepositoryInfo) {
assertSparseGitInfo(t, "", info)
}

func assertSparseGitInfo(t *testing.T, expectedRoot string, info git.RepositoryInfo) {
assert.Equal(t, "", info.CurrentBranch)
assert.Equal(t, "", info.LatestCommit)
assert.Equal(t, "", info.OriginURL)
assert.Equal(t, expectedRoot, info.WorktreeRoot)
}

func TestAccFetchRepositoryInfoAPI_FromRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
me, err := wt.W.CurrentUser.Me(ctx)
require.NoError(t, err)

targetPath := acc.RandomName(path.Join("/Workspace/Users", me.UserName, "/testing-clone-bundle-examples-"))
stdout, stderr := RequireSuccessfulRun(t, "repos", "create", examplesRepoUrl, examplesRepoProvider, "--path", targetPath)
t.Cleanup(func() {
RequireSuccessfulRun(t, "repos", "delete", targetPath)
})

assert.Empty(t, stderr.String())
assert.NotEmpty(t, stdout.String())
ctx = dbr.MockRuntime(ctx, true)

for _, inputPath := range []string{
path.Join(targetPath, "knowledge_base/dashboard_nyc_taxi"),
targetPath,
} {
t.Run(inputPath, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, inputPath, wt.W)
assert.NoError(t, err)
assertFullGitInfo(t, targetPath, info)
})
}
}

func TestAccFetchRepositoryInfoAPI_FromNonRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)
me, err := wt.W.CurrentUser.Me(ctx)
require.NoError(t, err)

rootPath := acc.RandomName(path.Join("/Workspace/Users", me.UserName, "testing-nonrepo-"))
_, stderr := RequireSuccessfulRun(t, "workspace", "mkdirs", path.Join(rootPath, "a/b/c"))
t.Cleanup(func() {
RequireSuccessfulRun(t, "workspace", "delete", "--recursive", rootPath)
})

assert.Empty(t, stderr.String())
ctx = dbr.MockRuntime(ctx, true)

tests := []struct {
input string
msg string
denik marked this conversation as resolved.
Show resolved Hide resolved
}{
{
input: path.Join(rootPath, "a/b/c"),
msg: "",
},
{
input: rootPath,
msg: "",
},
{
input: path.Join(rootPath, "/non-existent"),
msg: "doesn't exist",
},
}

for _, test := range tests {
t.Run(test.input+" <==> "+test.msg, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, test.input, wt.W)
if test.msg == "" {
assert.NoError(t, err)
} else {
assert.Error(t, err)
assert.Contains(t, err.Error(), test.msg)
}
assertEmptyGitInfo(t, info)
})
}
}

func TestAccFetchRepositoryInfoDotGit_FromGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)

repo := cloneRepoLocally(t, examplesRepoUrl)

for _, inputPath := range []string{
filepath.Join(repo, "knowledge_base/dashboard_nyc_taxi"),
repo,
} {
t.Run(inputPath, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, inputPath, wt.W)
assert.NoError(t, err)
assertFullGitInfo(t, repo, info)
})
}
}

func cloneRepoLocally(t *testing.T, repoUrl string) string {
tempDir := t.TempDir()
localRoot := filepath.Join(tempDir, "repo")

cmd := exec.Command("git", "clone", "--depth=1", examplesRepoUrl, localRoot)
err := cmd.Run()
require.NoError(t, err)
return localRoot
}

func TestAccFetchRepositoryInfoDotGit_FromNonGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)

tempDir := t.TempDir()
root := filepath.Join(tempDir, "repo")
require.NoError(t, os.MkdirAll(filepath.Join(root, "a/b/c"), 0700))

tests := []string{
filepath.Join(root, "a/b/c"),
root,
filepath.Join(root, "/non-existent"),
}
denik marked this conversation as resolved.
Show resolved Hide resolved

for _, input := range tests {
t.Run(input, func(t *testing.T) {
info, err := git.FetchRepositoryInfo(ctx, input, wt.W)
assert.NoError(t, err)
assertEmptyGitInfo(t, info)
})
}
}

func TestAccFetchRepositoryInfoDotGit_FromBrokenGitRepo(t *testing.T) {
ctx, wt := acc.WorkspaceTest(t)

tempDir := t.TempDir()
root := filepath.Join(tempDir, "repo")
path := filepath.Join(root, "a/b/c")
require.NoError(t, os.MkdirAll(path, 0700))
require.NoError(t, os.WriteFile(filepath.Join(root, ".git"), []byte(""), 0000))

info, err := git.FetchRepositoryInfo(ctx, path, wt.W)
assert.NoError(t, err)
assertSparseGitInfo(t, root, info)
}
13 changes: 13 additions & 0 deletions libs/diag/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,19 @@ func FromErr(err error) Diagnostics {
}
}

// FromErr returns a new warning diagnostic from the specified error, if any.
func WarningFromErr(err error) Diagnostics {
if err == nil {
return nil
}
return []Diagnostic{
{
Severity: Warning,
Summary: err.Error(),
},
}
}

// Warningf creates a new warning diagnostic.
func Warningf(format string, args ...any) Diagnostics {
return []Diagnostic{
Expand Down
Loading
Loading