Support tgz artifacts built from a directory or git ref #6428
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,224 @@ | ||
| package artifacts | ||
|
|
||
| import ( | ||
| "archive/tar" | ||
| "compress/gzip" | ||
| "context" | ||
| "errors" | ||
| "fmt" | ||
| "io" | ||
| "os" | ||
| "os/exec" | ||
| "path" | ||
| "path/filepath" | ||
| "slices" | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/databricks/cli/bundle/deploy/files" | ||
| "github.com/databricks/cli/libs/fileset" | ||
| libsync "github.com/databricks/cli/libs/sync" | ||
| "github.com/databricks/cli/libs/vfs" | ||
| ) | ||
|
|
||
| // tarballEpoch stamps every entry so the archive is reproducible: identical contents | ||
| // produce identical bytes regardless of file mtimes. | ||
| var tarballEpoch = time.Date(2000, 1, 1, 0, 0, 0, 0, time.UTC) | ||
|
|
||
| // buildTarballArtifact produces the gzipped tarball for a `type: tgz` artifact that | ||
| // DABs builds itself (no user `build` command). Archive entries are the packed files | ||
| // named relative to the artifact's `path`. With `git` set the tarball snapshots that | ||
| // ref; otherwise it packs the working tree scoped to `include`. The result is written | ||
| // to the artifact's single output file, which the normal artifact upload path uploads | ||
| // and references. | ||
| func buildTarballArtifact(ctx context.Context, b *bundle.Bundle, name string, a *config.Artifact) error { | ||
| if len(a.Files) != 1 { | ||
| return fmt.Errorf("artifact %q: a tgz artifact needs exactly one `files` entry naming the output path", name) | ||
| } | ||
| out := a.Files[0].Source // made absolute by artifacts.Prepare | ||
|
Contributor
There was a problem hiding this comment. Why do we need the output path to be specified? We can interpolate a temporary path, under Artifacts are uploaded separately from regular files. The places that refer to artifacts do path rewriting to make sure the final path we use in the workspace is the right one.
Contributor
Author
There was a problem hiding this comment. For now I think this is still necessary. We can mark it as a follow up if you feel it shouldn't be, but right now the BYOT API specifies I asked claude what it would take to bridge this gap and we would need to let
Contributor
There was a problem hiding this comment. Thanks, yeah I agree with the opaque bit, and that referencing via interpolation is the right solution. You'd get something like this: code_source_path: ${artifacts.air_code_source.output} |
||
| if err := os.MkdirAll(filepath.Dir(out), 0o755); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // Write to a temp file and rename on success, so a failed build never leaves a | ||
| // partial tarball at `out`. | ||
| tmp, err := os.CreateTemp(filepath.Dir(out), filepath.Base(out)+".*.tmp") | ||
| if err != nil { | ||
| return err | ||
| } | ||
| tmpName := tmp.Name() | ||
| defer func() { | ||
| tmp.Close() | ||
| os.Remove(tmpName) // no-op once renamed into place | ||
| }() | ||
|
|
||
| if a.Git != nil { | ||
| err = tarballFromGit(ctx, b, a, tmp) | ||
| } else { | ||
| err = tarballFromInclude(ctx, b, a, tmp) | ||
| } | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if err := tmp.Close(); err != nil { | ||
| return err | ||
| } | ||
| return os.Rename(tmpName, out) | ||
| } | ||
|
|
||
| // codeRel returns the artifact's `path` as a slash-separated path relative to the sync | ||
| // root ("." when they are the same). include paths and archive entry names are relative | ||
| // to it. | ||
| func codeRel(b *bundle.Bundle, a *config.Artifact) (string, error) { | ||
| rel, err := filepath.Rel(b.SyncRootPath, a.Path) | ||
| if err != nil { | ||
| return "", fmt.Errorf("artifact path %q: %w", a.Path, err) | ||
| } | ||
| return filepath.ToSlash(rel), nil | ||
| } | ||
|
|
||
| // tarballFromInclude packs the working tree under the artifact's `path`, optionally | ||
| // narrowed to `include` subpaths (relative to `path`). Entries are named relative to | ||
| // `path`. The bundle's sync walker is used so .gitignore is honored, but the | ||
| // bundle-wide sync.include/sync.exclude are deliberately NOT applied — they scope | ||
| // bundle file sync, not a code artifact. | ||
| func tarballFromInclude(ctx context.Context, b *bundle.Bundle, a *config.Artifact, w io.Writer) error { | ||
| relBase, err := codeRel(b, a) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| opts, err := files.GetSyncOptions(ctx, b) | ||
|
vinchenzo-db marked this conversation as resolved.
|
||
| if err != nil { | ||
| return err | ||
| } | ||
| paths := []string{relBase} | ||
| if len(a.Include) > 0 { | ||
| paths = paths[:0] | ||
| for _, inc := range a.Include { | ||
| paths = append(paths, path.Join(relBase, filepath.ToSlash(inc))) | ||
| } | ||
| } | ||
| // nil include/exclude: only .gitignore filters the walk, not the bundle sync globs. | ||
| fl, err := libsync.NewFileList(ctx, opts.WorktreeRoot, opts.LocalRoot, paths, nil, nil) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| list, err := fl.Files(ctx) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if relBase != "." { | ||
| prefix := relBase + "/" | ||
| list = slices.DeleteFunc(list, func(f fileset.File) bool { | ||
| return !strings.HasPrefix(f.Relative, prefix) | ||
| }) | ||
| } | ||
| if len(list) == 0 { | ||
| return fmt.Errorf("artifact tgz: no files to pack under %q (empty, gitignored, or no `include` match)", a.Path) | ||
| } | ||
| // Sort so the archive byte stream doesn't depend on walk order. | ||
| slices.SortFunc(list, func(x, y fileset.File) int { | ||
| return strings.Compare(x.Relative, y.Relative) | ||
| }) | ||
|
|
||
| gzw := gzip.NewWriter(w) | ||
| tw := tar.NewWriter(gzw) | ||
| for _, file := range list { | ||
| if err := addFileToTarball(tw, b.SyncRoot, relBase, file); err != nil { | ||
| return err | ||
| } | ||
| } | ||
| if err := tw.Close(); err != nil { | ||
| return err | ||
| } | ||
| return gzw.Close() | ||
| } | ||
|
|
||
| // tarballFromGit snapshots a git ref via `git archive`, so the archive reflects the | ||
| // committed tree at that ref. Entries are named relative to `path` (the tree at | ||
| // `<ref>:<path>`); `include`, when set, scopes the archived pathspecs. Commit wins over | ||
| // Branch. | ||
| func tarballFromGit(ctx context.Context, b *bundle.Bundle, a *config.Artifact, w io.Writer) error { | ||
|
Contributor
There was a problem hiding this comment. have we tested equivalence of tarballs from here vs CLI? Note also the tarball changes that landed in CLI recently to respect gitignore
Contributor
Author
There was a problem hiding this comment. Made them share code instead of testing equivalence |
||
| ref := a.Git.Commit | ||
| if ref == "" { | ||
| ref = a.Git.Branch | ||
| } | ||
| if ref == "" { | ||
| return errors.New("git artifact: specify git.commit or git.branch") | ||
| } | ||
| relBase, err := codeRel(b, a) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| treeish := ref | ||
| if relBase != "." { | ||
| // The tree at <ref>:<path>, so entries come out relative to `path`. | ||
| treeish = ref + ":" + relBase | ||
| } | ||
| args := []string{"-C", b.SyncRootPath, "archive", "--format=tar.gz", treeish} | ||
| if len(a.Include) > 0 { | ||
| args = append(args, "--") | ||
| args = append(args, a.Include...) | ||
| } | ||
| cmd := exec.CommandContext(ctx, "git", args...) | ||
| cmd.Stdout = w | ||
| var stderr strings.Builder | ||
| cmd.Stderr = &stderr | ||
| if err := cmd.Run(); err != nil { | ||
| return fmt.Errorf("git archive %s: %w: %s", treeish, err, stderr.String()) | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // addFileToTarball writes f (a sync-root-relative file) to the archive under a name | ||
| // relative to relBase, so entries are relative to the artifact's `path`. | ||
| func addFileToTarball(tw *tar.Writer, root vfs.Path, relBase string, f fileset.File) error { | ||
| name := f.Relative | ||
| if relBase != "." { | ||
| trimmed, ok := strings.CutPrefix(name, relBase+"/") | ||
| if !ok { | ||
| // Outside the code root; the walk is scoped to it, so this shouldn't | ||
| // happen, but skip defensively rather than mis-place a file. | ||
| return nil | ||
| } | ||
| name = trimmed | ||
| } | ||
|
|
||
| rc, err := root.Open(f.Relative) | ||
| if err != nil { | ||
| return fmt.Errorf("open %s: %w", f.Relative, err) | ||
| } | ||
| defer rc.Close() | ||
|
|
||
| info, err := rc.Stat() | ||
| if err != nil { | ||
| return fmt.Errorf("stat %s: %w", f.Relative, err) | ||
| } | ||
| // Only regular files; the walker never yields directories and symlinks are out | ||
| // of scope for a code snapshot. | ||
| if !info.Mode().IsRegular() { | ||
| return nil | ||
| } | ||
|
|
||
| // Preserve the owner execute bit; normalize the rest. | ||
| mode := int64(0o644) | ||
| if info.Mode().Perm()&0o100 != 0 { | ||
| mode = 0o755 | ||
| } | ||
| hdr := &tar.Header{ | ||
| Typeflag: tar.TypeReg, | ||
| Name: name, | ||
| Size: info.Size(), | ||
| Mode: mode, | ||
| ModTime: tarballEpoch, | ||
| } | ||
| if err := tw.WriteHeader(hdr); err != nil { | ||
| return fmt.Errorf("tar header for %s: %w", name, err) | ||
| } | ||
| if _, err := io.Copy(tw, rc); err != nil { | ||
| return fmt.Errorf("write %s: %w", name, err) | ||
| } | ||
| return nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| package artifacts | ||
|
|
||
| import ( | ||
| "archive/tar" | ||
| "bytes" | ||
| "compress/gzip" | ||
| "io" | ||
| "os" | ||
| "os/exec" | ||
| "path/filepath" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/databricks/cli/bundle" | ||
| "github.com/databricks/cli/bundle/config" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // tarEntries reads a gzipped tarball and returns entry name -> content. | ||
| func tarEntries(t *testing.T, b []byte) map[string]string { | ||
| t.Helper() | ||
| gzr, err := gzip.NewReader(bytes.NewReader(b)) | ||
| require.NoError(t, err) | ||
| tr := tar.NewReader(gzr) | ||
| out := map[string]string{} | ||
| for { | ||
| hdr, err := tr.Next() | ||
| if err == io.EOF { | ||
| break | ||
| } | ||
| require.NoError(t, err) | ||
| body, err := io.ReadAll(tr) | ||
| require.NoError(t, err) | ||
| out[hdr.Name] = string(body) | ||
| } | ||
| return out | ||
| } | ||
|
|
||
| func runGit(t *testing.T, dir string, args ...string) { | ||
| t.Helper() | ||
| cmd := exec.Command("git", args...) | ||
| cmd.Dir = dir | ||
| out, err := cmd.CombinedOutput() | ||
| require.NoError(t, err, "git %v: %s", args, out) | ||
| } | ||
|
|
||
| // TestTarballFromGitArchivesRef verifies git mode archives the committed tree with | ||
| // entries named relative to the artifact's path (here the repo root, so no prefix). | ||
| func TestTarballFromGitArchivesRef(t *testing.T) { | ||
| repo := t.TempDir() | ||
| runGit(t, repo, "init", "-q") | ||
| runGit(t, repo, "config", "user.email", "t@example.com") | ||
| runGit(t, repo, "config", "user.name", "t") | ||
| // Keep line endings verbatim so the content check doesn't depend on the runner's | ||
| // core.autocrlf (true by default on the Windows CI image). | ||
| runGit(t, repo, "config", "core.autocrlf", "false") | ||
| require.NoError(t, os.WriteFile(filepath.Join(repo, "train.py"), []byte("print('x')\n"), 0o644)) | ||
| runGit(t, repo, "add", "-A") | ||
| runGit(t, repo, "commit", "-qm", "init") | ||
|
|
||
| b := &bundle.Bundle{SyncRootPath: repo} | ||
| a := &config.Artifact{Path: repo, Git: &config.ArtifactGit{Commit: "HEAD"}} | ||
|
|
||
| var buf bytes.Buffer | ||
| require.NoError(t, tarballFromGit(t.Context(), b, a, &buf)) | ||
|
|
||
| // path == repo root, so the entry is relative to it (no injected prefix). | ||
| entries := tarEntries(t, buf.Bytes()) | ||
| require.Contains(t, entries, "train.py") | ||
| assert.Equal(t, "print('x')", strings.TrimSpace(entries["train.py"])) | ||
| } | ||
|
|
||
| func TestTarballFromGitRequiresRef(t *testing.T) { | ||
| b := &bundle.Bundle{SyncRootPath: t.TempDir()} | ||
| a := &config.Artifact{Path: b.SyncRootPath, Git: &config.ArtifactGit{}} | ||
| err := tarballFromGit(t.Context(), b, a, io.Discard) | ||
| require.ErrorContains(t, err, "git.commit or git.branch") | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is semantics for
includethe same as in air cli? There it is relative to code source root, is include relative to bundle root in DABs? Can we make sure that is documented well somewhere?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
include is now relative to the code-source root, same as the air CLI. The reason I decided this is so that
convert-to-dabscan map air CLI'sinclude_pathsstraight across later.