From 1c476e551a8d62d4d5a932480e63b5ea908cd3e3 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Mon, 12 Oct 2026 13:08:15 +0200 Subject: [PATCH 1/5] localenv: pass the venv folder to PostProvision and Validate Co-authored-by: Isaac --- libs/localenv/pipeline.go | 21 +++++++--- libs/localenv/pipeline_test.go | 44 +++++++++++++++++++++ libs/localenv/pkgmanager.go | 13 +++--- libs/localenv/provision_integration_test.go | 2 +- libs/localenv/uv.go | 20 +++++----- libs/localenv/uv_test.go | 2 +- 6 files changed, 78 insertions(+), 24 deletions(-) diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index bbde0ad12cf..50fca677b4e 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -73,6 +73,10 @@ type Pipeline struct { // res accumulates phase statuses and result fields as the run progresses. res *Result + // venv is the absolute virtualenv folder that provision seeds, validate + // inspects, and Result.VenvPath reports. Set in preflight. + venv string + // nowFn supplies the time for backup filenames; nil means time.Now. Injected // in tests so backup names are deterministic. nowFn func() time.Time @@ -173,6 +177,7 @@ func (p *Pipeline) run(ctx context.Context) error { if m := detectManager(p.ProjectDir); m != managerUv { return p.fail(PhasePreflight, false, NewError(ErrManagerUnsupported, nil, "%s", managerGuidance(m))) } + p.venv = filepath.Join(p.ProjectDir, venvDir) // Under --dry-run the pipeline only reads and reports a plan, so it must not // mutate anything at preflight. Two preflight steps can write: // - ensureWritable creates and removes a temp file (and would fail a @@ -539,7 +544,7 @@ func (p *Pipeline) provision(ctx context.Context, pyMinor string) error { if err := p.PM.Provision(ctx, p.ProjectDir, selection.Executable); err != nil { return p.fail(PhaseProvision, true, asPipelineError(err, ErrProvision, "provision failed")) } - if err := p.PM.PostProvision(ctx, p.ProjectDir); err != nil { + if err := p.PM.PostProvision(ctx, p.venv); err != nil { return p.fail(PhaseProvision, true, asPipelineError(err, ErrProvision, "post-provision failed")) } p.markOK(PhaseProvision, "") @@ -564,7 +569,7 @@ func (p *Pipeline) hasConstraintConflictWarning() bool { // is not managed (opts.SkipDBConnect) the databricks-connect assertion and version // reporting are skipped. func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, opts MergeOptions) error { - info, err := p.PM.Validate(ctx, p.ProjectDir) + info, err := p.PM.Validate(ctx, p.venv) if err != nil { return p.fail(PhaseValidate, true, asPipelineError(err, ErrValidate, "validation failed")) } @@ -622,10 +627,14 @@ func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, p.markOK(PhaseValidate, detail) // venvPath is reported relative to the project root (spec §6.1), not as an - // absolute path: the value names the ".venv" the command provisions inside - // ProjectDir, and the VS Code consumer already knows the project root (it - // sets the working directory when it shells out). venvDir is already ".venv". - p.res.VenvPath = venvDir + // absolute path: the VS Code consumer already knows the project root (it sets + // the working directory when it shells out). For a standalone project this is + // ".venv". + rel, err := filepath.Rel(p.ProjectDir, p.venv) + if err != nil { + return p.fail(PhaseValidate, true, NewError(ErrValidate, err, "venv %s is not relative to the project", filepath.ToSlash(p.venv))) + } + p.res.VenvPath = filepath.ToSlash(rel) if p.res.Resolved != nil { if managed { p.res.Resolved.DBConnectVersion = dbcVer diff --git a/libs/localenv/pipeline_test.go b/libs/localenv/pipeline_test.go index bc12cfb4cb9..2193db8a255 100644 --- a/libs/localenv/pipeline_test.go +++ b/libs/localenv/pipeline_test.go @@ -1420,3 +1420,47 @@ func TestPipelineNoConstraintsGreenfieldOmitsPins(t *testing.T) { // databricks-connect (orthogonal) is still added. assert.Contains(t, s, "databricks-connect~=17.2.0") } + +// newServerlessTestPipeline returns a pipeline for dir against a test artifact +// server (serverless v4) that is closed when the test ends. +func newServerlessTestPipeline(t *testing.T, dir string) *Pipeline { + srv := newTestServer(t) + t.Cleanup(srv.Close) + return &Pipeline{ + Mode: ModeDefault, ProjectDir: dir, + ConstraintBaseURL: srv.URL, CacheDir: t.TempDir(), + Flags: ComputeFlags{Serverless: "v4"}, Compute: stubCompute{}, + PM: fakePM{py: "3.12", dbc: "17.2.0"}, + } +} + +// venvRecordingPM records the venv folder the pipeline hands to PostProvision +// and Validate. +type venvRecordingPM struct { + fakePM + postProvisionVenv, validateVenv *string +} + +func (f venvRecordingPM) PostProvision(_ context.Context, venv string) error { + *f.postProvisionVenv = venv + return nil +} + +func (f venvRecordingPM) Validate(ctx context.Context, venv string) (VenvInfo, error) { + *f.validateVenv = venv + return f.fakePM.Validate(ctx, venv) +} + +func TestPipelinePassesProjectVenvToPackageManager(t *testing.T) { + dir := writeProject(t) + var post, val string + p := newServerlessTestPipeline(t, dir) + p.PM = venvRecordingPM{fakePM: fakePM{py: "3.12", dbc: "17.2.0"}, postProvisionVenv: &post, validateVenv: &val} + + res, err := p.Run(t.Context()) + require.NoError(t, err) + want := filepath.Join(dir, ".venv") + assert.Equal(t, want, post) + assert.Equal(t, want, val) + assert.Equal(t, ".venv", res.VenvPath) +} diff --git a/libs/localenv/pkgmanager.go b/libs/localenv/pkgmanager.go index 774bc11e569..c474f9963ea 100644 --- a/libs/localenv/pkgmanager.go +++ b/libs/localenv/pkgmanager.go @@ -21,17 +21,18 @@ type PackageManager interface { // exact interpreter path selected by EnsurePython. Provision(ctx context.Context, projectDir, python string) error - // PostProvision seeds pip into the virtual environment inside projectDir. + // PostProvision seeds pip into the virtual environment at venv (an absolute + // path to the venv folder). // This step is required because VS Code's ms-python.vscode-python-envs // extension falls back to `python -m pip list` when its `uv --version` // probe fails on the GUI PATH; uv venvs contain no pip; and `uv sync` // strips pip, so seeding must run after every sync. - PostProvision(ctx context.Context, projectDir string) error + PostProvision(ctx context.Context, venv string) error - // Validate inspects the provisioned virtual environment inside projectDir and - // returns what it observed (see VenvInfo). The caller decides which observations - // are acceptable. - Validate(ctx context.Context, projectDir string) (VenvInfo, error) + // Validate inspects the provisioned virtual environment at venv and returns + // what it observed (see VenvInfo). The caller decides which observations are + // acceptable. + Validate(ctx context.Context, venv string) (VenvInfo, error) } // PythonResolution records which preparation path supplied the interpreter. diff --git a/libs/localenv/provision_integration_test.go b/libs/localenv/provision_integration_test.go index 7e21f9c8fb8..71420c20a49 100644 --- a/libs/localenv/provision_integration_test.go +++ b/libs/localenv/provision_integration_test.go @@ -90,7 +90,7 @@ func TestProvisionCreatesAndValidatesVenv(t *testing.T) { } // A real interpreter exists in the created venv. - assert.FileExists(t, venvPython(dir)) + assert.FileExists(t, venvPython(filepath.Join(dir, venvDir))) // pyproject.toml + uv.lock were written; the env-owned pin was applied. pyproject, err := os.ReadFile(filepath.Join(dir, "pyproject.toml")) diff --git a/libs/localenv/uv.go b/libs/localenv/uv.go index f41755f281c..9e78350e2c9 100644 --- a/libs/localenv/uv.go +++ b/libs/localenv/uv.go @@ -194,13 +194,13 @@ func isUvResolutionConflict(stderr string) bool { return strings.Contains(s, "no solution found") || strings.Contains(s, "are unsatisfiable") } -// venvPython returns the path to the virtualenv's Python interpreter, +// venvPython returns the path to the Python interpreter of the virtualenv at venv, // accounting for the Windows (Scripts/python.exe) vs Unix (bin/python) layout. -func venvPython(projectDir string) string { +func venvPython(venv string) string { if runtime.GOOS == "windows" { - return filepath.Join(projectDir, venvDir, "Scripts", "python.exe") + return filepath.Join(venv, "Scripts", "python.exe") } - return filepath.Join(projectDir, venvDir, "bin", "python") + return filepath.Join(venv, "bin", "python") } // PostProvision seeds pip into the project's virtual environment. @@ -211,9 +211,9 @@ func venvPython(projectDir string) string { // pip if it was previously present. Seeding pip after every sync ensures the // VS Code integration works correctly regardless of how the environment was // activated. -func (m *uvManager) PostProvision(ctx context.Context, projectDir string) error { - args := append([]string{m.bin}, m.pipSeedArgs(venvPython(projectDir))...) - if err := m.runUv(ctx, args, projectDir); err != nil { +func (m *uvManager) PostProvision(ctx context.Context, venv string) error { + args := append([]string{m.bin}, m.pipSeedArgs(venvPython(venv))...) + if err := m.runUv(ctx, args, filepath.Dir(venv)); err != nil { return uvFailure(ErrProvision, err, "uv pip seed") } return nil @@ -231,7 +231,7 @@ func (m *uvManager) PostProvision(ctx context.Context, projectDir string) error // on top of it. Metadata alone cannot tell a live collision from a stale dist-info that // no longer describes the importable module, so the probe also attempts `import // databricks.connect`: a live collision raises there, a harmless leftover does not. -func (m *uvManager) Validate(ctx context.Context, projectDir string) (VenvInfo, error) { +func (m *uvManager) Validate(ctx context.Context, venv string) (VenvInfo, error) { // Each value is printed with a unique prefix so parsing greps for the prefix rather // than relying on line position: any stray line uv or the interpreter writes to // stdout (e.g. a warning) would otherwise shift a positional parse. @@ -255,8 +255,8 @@ except BaseException as e: // active instead of the .venv we just provisioned. The direct path is exactly // what was installed, so validation observes the real target. out, err := process.Background(ctx, - []string{venvPython(projectDir), "-c", pyCode}, - process.WithDir(projectDir), + []string{venvPython(venv), "-c", pyCode}, + process.WithDir(filepath.Dir(venv)), process.WithProcessGroup(), ) if err != nil { diff --git a/libs/localenv/uv_test.go b/libs/localenv/uv_test.go index 9e6f85c7618..152bd27c360 100644 --- a/libs/localenv/uv_test.go +++ b/libs/localenv/uv_test.go @@ -191,7 +191,7 @@ func TestEnsurePythonFailsWhenFallbackFindsNothing(t *testing.T) { func TestVenvPythonPath(t *testing.T) { // Validate invokes this interpreter directly (not via `uv run`) so it observes // exactly the .venv that was provisioned, ignoring any active VIRTUAL_ENV. - got := venvPython(filepath.Join("p", "proj")) + got := venvPython(filepath.Join("p", "proj", ".venv")) var want string if runtime.GOOS == "windows" { want = filepath.Join("p", "proj", ".venv", "Scripts", "python.exe") From 2099b2921a61ac310e9c46d8459ad6dae55e63af Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Mon, 12 Oct 2026 13:10:16 +0200 Subject: [PATCH 2/5] localenv: split the pyproject backup, write, and diff into per-file helpers Co-authored-by: Isaac --- libs/localenv/pipeline.go | 100 ++++++++++++++++++++------------- libs/localenv/pipeline_test.go | 40 +++++++++++++ 2 files changed, 100 insertions(+), 40 deletions(-) diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index 50fca677b4e..18ddf591db2 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -321,23 +321,23 @@ func (p *Pipeline) pyprojectPath() string { return filepath.Join(p.ProjectDir, pyprojectFile) } -// backupPath returns the path to the canonical pyproject.toml backup file. -func (p *Pipeline) backupPath() string { - return filepath.Join(p.ProjectDir, backupFile) +// backupPath returns the path to the canonical pyproject.toml backup file in dir. +func (p *Pipeline) backupPath(dir string) string { + return filepath.Join(dir, backupFile) } -// timestampedBackupBase is the /pyproject.toml. stem +// timestampedBackupBase is the /pyproject.toml. stem // a non-first backup name is built from. -func (p *Pipeline) timestampedBackupBase() string { - return filepath.Join(p.ProjectDir, pyprojectFile+"."+p.clock().Format(backupTimestampLayout)) +func (p *Pipeline) timestampedBackupBase(dir string) string { + return filepath.Join(dir, pyprojectFile+"."+p.clock().Format(backupTimestampLayout)) } -// backupCurrent writes content to a backup of pyproject.toml and returns its -// path, never overwriting an existing backup (invariant 2). The canonical +// backupCurrent writes content to a backup of the pyproject.toml in dir and +// returns its path, never overwriting an existing backup (invariant 2). The canonical // pyproject.toml.bak is written once and kept as the pristine original; later // backups are pyproject.toml..bak. mode is preserved onto the file. -func (p *Pipeline) backupCurrent(content []byte, mode os.FileMode) (string, error) { - canonical := p.backupPath() +func (p *Pipeline) backupCurrent(dir string, content []byte, mode os.FileMode) (string, error) { + canonical := p.backupPath(dir) switch err := writeNew(canonical, content, mode); { case err == nil: return canonical, nil @@ -345,7 +345,7 @@ func (p *Pipeline) backupCurrent(content []byte, mode os.FileMode) (string, erro return "", err } - base := p.timestampedBackupBase() + base := p.timestampedBackupBase(dir) candidate := base + ".bak" for i := 1; ; i++ { // Only a name collision advances the suffix, so the loop terminates. @@ -362,14 +362,14 @@ func (p *Pipeline) backupCurrent(content []byte, mode os.FileMode) (string, erro // plannedBackupName previews the backup name a real run would create — canonical // .bak if none exists yet, else a timestamped name — for --dry-run only. Best-effort: // an unstattable .bak returns an error the caller treats as "nothing to report". -func (p *Pipeline) plannedBackupName() (string, error) { - canonical := p.backupPath() +func (p *Pipeline) plannedBackupName(dir string) (string, error) { + canonical := p.backupPath(dir) _, statErr := os.Stat(canonical) switch { case errors.Is(statErr, os.ErrNotExist): return canonical, nil case statErr == nil: - return p.timestampedBackupBase() + ".bak", nil + return p.timestampedBackupBase(dir) + ".bak", nil default: return "", statErr } @@ -450,27 +450,16 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, // Under --dry-run, build the plan (with a diff) for reporting. A real run does // not need the diff. if p.Check { - oldStr := "" - newStr := string(merged) - oldName := pyprojectFile - newName := pyprojectFile - if !greenfield { - oldStr = string(baseBytes) - newName = pyprojectFile + ".new" - } - edits := myers.ComputeEdits(span.URIFromPath(oldName), oldStr, newStr) - diff := fmt.Sprint(gotextdiff.ToUnified(oldName, newName, oldStr, edits)) - plan := &Plan{ WouldWrite: filepath.ToSlash(pyproject), - Diff: diff, + Diff: pyprojectDiff(pyprojectFile, baseBytes, merged, greenfield), ChangedRegions: changedRegions, WouldInstallPython: pyMinor, } // Report a backup only when the run would actually write one (i.e. it changes // the file); a no-op re-run writes none. if !greenfield && !bytes.Equal(merged, baseBytes) { - if backupName, statErr := p.plannedBackupName(); statErr == nil { + if backupName, statErr := p.plannedBackupName(p.ProjectDir); statErr == nil { plan.WouldBackup = filepath.ToSlash(backupName) } } @@ -479,22 +468,53 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, return merged, greenfield, nil } +// pyprojectDiff renders the unified diff the --dry-run plan reports for one +// pyproject.toml. label is the file name shown in the diff headers; an existing +// file diffs label against label+".new", a greenfield file diffs "" against label. +func pyprojectDiff(label string, base, merged []byte, greenfield bool) string { + oldStr := "" + newStr := string(merged) + oldName := label + newName := label + if !greenfield { + oldStr = string(base) + newName = label + ".new" + } + edits := myers.ComputeEdits(span.URIFromPath(oldName), oldStr, newStr) + return fmt.Sprint(gotextdiff.ToUnified(oldName, newName, oldStr, edits)) +} + // applyMerge writes the merged bytes to disk, backing up the current // pyproject.toml first. From the backup copy onward, disk has been mutated. func (p *Pipeline) applyMerge(_ context.Context, mergedBytes []byte, greenfield bool) error { - pyproject := p.pyprojectPath() + backup, mutated, perr := p.writePyproject(p.pyprojectPath(), mergedBytes, greenfield) + // A backup written before a failed write is still reported, so the consumer + // can restore from it. + if backup != "" { + p.res.BackupPath = filepath.ToSlash(backup) + } + if perr != nil { + return p.fail(PhaseMerge, mutated, perr) + } + return nil +} +// writePyproject writes merged over the pyproject.toml at path, backing up the +// current file first (into the same folder) unless greenfield. A no-op merge +// writes nothing and returns no backup. diskMutated reports whether disk changed +// before a failure; the caller passes it to p.fail. +func (p *Pipeline) writePyproject(path string, merged []byte, greenfield bool) (backup string, diskMutated bool, perr *PipelineError) { if !greenfield { // Stat+read up front: mode is preserved onto the backup, content is the // no-op base and the backup source. Fail before any write (no mutation yet) // rather than swallow a stat/read error on an existing pyproject.toml. - info, statErr := os.Stat(pyproject) + info, statErr := os.Stat(path) if statErr != nil { - return p.fail(PhaseMerge, false, NewError(ErrMerge, statErr, "stat pyproject.toml %s failed", filepath.ToSlash(pyproject))) + return "", false, NewError(ErrMerge, statErr, "stat pyproject.toml %s failed", filepath.ToSlash(path)) } - current, readErr := os.ReadFile(pyproject) + current, readErr := os.ReadFile(path) if readErr != nil { - return p.fail(PhaseMerge, false, NewError(ErrMerge, readErr, "read pyproject.toml %s failed", filepath.ToSlash(pyproject))) + return "", false, NewError(ErrMerge, readErr, "read pyproject.toml %s failed", filepath.ToSlash(path)) } // No-op: the merged output already matches disk. On an idempotent re-run @@ -502,27 +522,27 @@ func (p *Pipeline) applyMerge(_ context.Context, mergedBytes []byte, greenfield // only advance the mtime — spuriously invalidating file watchers and uv.lock // freshness checks — without changing content. Skip both the backup and the // write, leaving disk (and every existing backup) exactly as it was. - if bytes.Equal(current, mergedBytes) { - return nil + if bytes.Equal(current, merged) { + return "", false, nil } // Back up before overwriting (invariant 2). A partial backup is possible // mid-write, so report disk as mutated on error. - backup, backupErr := p.backupCurrent(current, info.Mode().Perm()) + b, backupErr := p.backupCurrent(filepath.Dir(path), current, info.Mode().Perm()) if backupErr != nil { - return p.fail(PhaseMerge, true, NewError(ErrMerge, backupErr, "backup pyproject.toml failed")) + return "", true, NewError(ErrMerge, backupErr, "backup pyproject.toml failed") } - p.res.BackupPath = filepath.ToSlash(backup) + backup = b } - if err := atomicfile.Write(pyproject, mergedBytes, 0o644); err != nil { + if err := atomicfile.Write(path, merged, 0o644); err != nil { code := ErrMerge if greenfield { code = ErrWrite } - return p.fail(PhaseMerge, true, NewError(code, err, "write pyproject.toml failed")) + return backup, true, NewError(code, err, "write pyproject.toml failed") } - return nil + return backup, true, nil } // provision ensures the required Python version is installed, runs uv sync, and diff --git a/libs/localenv/pipeline_test.go b/libs/localenv/pipeline_test.go index 2193db8a255..d478348e55b 100644 --- a/libs/localenv/pipeline_test.go +++ b/libs/localenv/pipeline_test.go @@ -1464,3 +1464,43 @@ func TestPipelinePassesProjectVenvToPackageManager(t *testing.T) { assert.Equal(t, want, val) assert.Equal(t, ".venv", res.VenvPath) } + +func TestWritePyprojectNoOpWritesNothing(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, pyprojectFile) + content := []byte("[project]\nname = \"x\"\n") + require.NoError(t, os.WriteFile(path, content, 0o644)) + + p := &Pipeline{ProjectDir: dir, res: NewResult()} + backup, mutated, perr := p.writePyproject(path, content, false) + require.Nil(t, perr) + assert.Empty(t, backup) + assert.False(t, mutated) + assert.NoFileExists(t, filepath.Join(dir, backupFile)) +} + +func TestWritePyprojectBacksUpInItsOwnFolder(t *testing.T) { + project := t.TempDir() + other := t.TempDir() + path := filepath.Join(other, pyprojectFile) + require.NoError(t, os.WriteFile(path, []byte("[project]\nname = \"x\"\n"), 0o644)) + + p := &Pipeline{ProjectDir: project, res: NewResult()} + backup, mutated, perr := p.writePyproject(path, []byte("[project]\nname = \"y\"\n"), false) + require.Nil(t, perr) + assert.True(t, mutated) + assert.Equal(t, filepath.Join(other, backupFile), backup) + assert.NoFileExists(t, filepath.Join(project, backupFile)) + got, err := os.ReadFile(path) + require.NoError(t, err) + assert.Equal(t, "[project]\nname = \"y\"\n", string(got)) +} + +func TestPyprojectDiffLabels(t *testing.T) { + d := pyprojectDiff("pyproject.toml", []byte("a\n"), []byte("b\n"), false) + assert.Contains(t, d, "--- pyproject.toml") + assert.Contains(t, d, "+++ pyproject.toml.new") + g := pyprojectDiff("pyproject.toml", nil, []byte("b\n"), true) + assert.Contains(t, g, "+++ pyproject.toml") + assert.NotContains(t, g, ".new") +} From e7dc88b27484b982e04ccb69aeb0a6141913b098 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Mon, 12 Oct 2026 13:48:24 +0200 Subject: [PATCH 3/5] localenv: keep the project folder as the uv working folder; set venvPath at preflight Co-authored-by: Isaac --- libs/localenv/pipeline.go | 41 +++++++++++++++++----------------- libs/localenv/pipeline_test.go | 40 +++++++++++++++++---------------- libs/localenv/pkgmanager.go | 12 +++++----- libs/localenv/uv.go | 10 ++++----- libs/localenv/uv_test.go | 22 ++++++++++++++++++ 5 files changed, 74 insertions(+), 51 deletions(-) diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index 18ddf591db2..e1c4d14e9a6 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -17,8 +17,8 @@ import ( "github.com/hexops/gotextdiff/span" ) -// Filenames and directories the pipeline reads and writes. venvDir is also the -// virtualenv directory the package manager provisions. pyprojectFile lives in +// Filenames and directories the pipeline reads and writes. venvDir is the name of +// the virtualenv folder the package manager provisions. pyprojectFile lives in // detect.go, the first consumer of it. const ( backupFile = "pyproject.toml.bak" @@ -73,9 +73,10 @@ type Pipeline struct { // res accumulates phase statuses and result fields as the run progresses. res *Result - // venv is the absolute virtualenv folder that provision seeds, validate - // inspects, and Result.VenvPath reports. Set in preflight. - venv string + // venv is the absolute virtualenv folder that provision seeds and validate + // inspects; venvRel is the same folder relative to ProjectDir, reported as + // Result.VenvPath. Both are set in preflight. + venv, venvRel string // nowFn supplies the time for backup filenames; nil means time.Now. Injected // in tests so backup names are deterministic. @@ -177,7 +178,8 @@ func (p *Pipeline) run(ctx context.Context) error { if m := detectManager(p.ProjectDir); m != managerUv { return p.fail(PhasePreflight, false, NewError(ErrManagerUnsupported, nil, "%s", managerGuidance(m))) } - p.venv = filepath.Join(p.ProjectDir, venvDir) + p.venvRel = venvDir + p.venv = filepath.Join(p.ProjectDir, p.venvRel) // Under --dry-run the pipeline only reads and reports a plan, so it must not // mutate anything at preflight. Two preflight steps can write: // - ensureWritable creates and removes a temp file (and would fail a @@ -322,7 +324,7 @@ func (p *Pipeline) pyprojectPath() string { } // backupPath returns the path to the canonical pyproject.toml backup file in dir. -func (p *Pipeline) backupPath(dir string) string { +func backupPath(dir string) string { return filepath.Join(dir, backupFile) } @@ -337,7 +339,7 @@ func (p *Pipeline) timestampedBackupBase(dir string) string { // pyproject.toml.bak is written once and kept as the pristine original; later // backups are pyproject.toml..bak. mode is preserved onto the file. func (p *Pipeline) backupCurrent(dir string, content []byte, mode os.FileMode) (string, error) { - canonical := p.backupPath(dir) + canonical := backupPath(dir) switch err := writeNew(canonical, content, mode); { case err == nil: return canonical, nil @@ -363,7 +365,7 @@ func (p *Pipeline) backupCurrent(dir string, content []byte, mode os.FileMode) ( // .bak if none exists yet, else a timestamped name — for --dry-run only. Best-effort: // an unstattable .bak returns an error the caller treats as "nothing to report". func (p *Pipeline) plannedBackupName(dir string) (string, error) { - canonical := p.backupPath(dir) + canonical := backupPath(dir) _, statErr := os.Stat(canonical) switch { case errors.Is(statErr, os.ErrNotExist): @@ -459,7 +461,7 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, // Report a backup only when the run would actually write one (i.e. it changes // the file); a no-op re-run writes none. if !greenfield && !bytes.Equal(merged, baseBytes) { - if backupName, statErr := p.plannedBackupName(p.ProjectDir); statErr == nil { + if backupName, statErr := p.plannedBackupName(filepath.Dir(pyproject)); statErr == nil { plan.WouldBackup = filepath.ToSlash(backupName) } } @@ -469,8 +471,9 @@ func (p *Pipeline) mergePlan(_ context.Context, pyMinor string, c *Constraints, } // pyprojectDiff renders the unified diff the --dry-run plan reports for one -// pyproject.toml. label is the file name shown in the diff headers; an existing -// file diffs label against label+".new", a greenfield file diffs "" against label. +// pyproject.toml. label is the file name shown in the diff headers: an existing +// file diffs label against label+".new"; a greenfield file has empty old content +// and label in both headers. func pyprojectDiff(label string, base, merged []byte, greenfield bool) string { oldStr := "" newStr := string(merged) @@ -501,8 +504,8 @@ func (p *Pipeline) applyMerge(_ context.Context, mergedBytes []byte, greenfield // writePyproject writes merged over the pyproject.toml at path, backing up the // current file first (into the same folder) unless greenfield. A no-op merge -// writes nothing and returns no backup. diskMutated reports whether disk changed -// before a failure; the caller passes it to p.fail. +// writes nothing and returns no backup. diskMutated reports whether disk changed, +// on success and on failure; on failure the caller passes it to p.fail. func (p *Pipeline) writePyproject(path string, merged []byte, greenfield bool) (backup string, diskMutated bool, perr *PipelineError) { if !greenfield { // Stat+read up front: mode is preserved onto the backup, content is the @@ -564,7 +567,7 @@ func (p *Pipeline) provision(ctx context.Context, pyMinor string) error { if err := p.PM.Provision(ctx, p.ProjectDir, selection.Executable); err != nil { return p.fail(PhaseProvision, true, asPipelineError(err, ErrProvision, "provision failed")) } - if err := p.PM.PostProvision(ctx, p.venv); err != nil { + if err := p.PM.PostProvision(ctx, p.ProjectDir, p.venv); err != nil { return p.fail(PhaseProvision, true, asPipelineError(err, ErrProvision, "post-provision failed")) } p.markOK(PhaseProvision, "") @@ -589,7 +592,7 @@ func (p *Pipeline) hasConstraintConflictWarning() bool { // is not managed (opts.SkipDBConnect) the databricks-connect assertion and version // reporting are skipped. func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, opts MergeOptions) error { - info, err := p.PM.Validate(ctx, p.venv) + info, err := p.PM.Validate(ctx, p.ProjectDir, p.venv) if err != nil { return p.fail(PhaseValidate, true, asPipelineError(err, ErrValidate, "validation failed")) } @@ -650,11 +653,7 @@ func (p *Pipeline) validate(ctx context.Context, expectedPyMinor, dbcPin string, // absolute path: the VS Code consumer already knows the project root (it sets // the working directory when it shells out). For a standalone project this is // ".venv". - rel, err := filepath.Rel(p.ProjectDir, p.venv) - if err != nil { - return p.fail(PhaseValidate, true, NewError(ErrValidate, err, "venv %s is not relative to the project", filepath.ToSlash(p.venv))) - } - p.res.VenvPath = filepath.ToSlash(rel) + p.res.VenvPath = filepath.ToSlash(p.venvRel) if p.res.Resolved != nil { if managed { p.res.Resolved.DBConnectVersion = dbcVer diff --git a/libs/localenv/pipeline_test.go b/libs/localenv/pipeline_test.go index d478348e55b..fda5f63e0ea 100644 --- a/libs/localenv/pipeline_test.go +++ b/libs/localenv/pipeline_test.go @@ -34,9 +34,9 @@ func (fakePM) EnsureAvailable(context.Context) (string, error) { return "fake 1. func (fakePM) EnsurePython(_ context.Context, minor string) (PythonSelection, error) { return PythonSelection{Executable: minor, Resolution: PythonResolutionUVInstallSucceeded}, nil } -func (fakePM) Provision(context.Context, string, string) error { return nil } -func (fakePM) PostProvision(context.Context, string) error { return nil } -func (f fakePM) Validate(context.Context, string) (VenvInfo, error) { +func (fakePM) Provision(context.Context, string, string) error { return nil } +func (fakePM) PostProvision(context.Context, string, string) error { return nil } +func (f fakePM) Validate(context.Context, string, string) (VenvInfo, error) { return VenvInfo{PythonMinor: f.py, DBConnect: f.dbc, Pyspark: f.pyspark, DBConnectImportErr: f.dbcImportErr}, nil } @@ -58,11 +58,11 @@ func (noProvisionPM) Provision(context.Context, string, string) error { return errors.New("Provision must not be called under --dry-run") } -func (noProvisionPM) PostProvision(context.Context, string) error { +func (noProvisionPM) PostProvision(context.Context, string, string) error { return errors.New("PostProvision must not be called under --dry-run") } -func (noProvisionPM) Validate(context.Context, string) (VenvInfo, error) { +func (noProvisionPM) Validate(context.Context, string, string) (VenvInfo, error) { return VenvInfo{}, errors.New("Validate must not be called under --dry-run") } @@ -1434,34 +1434,36 @@ func newServerlessTestPipeline(t *testing.T, dir string) *Pipeline { } } -// venvRecordingPM records the venv folder the pipeline hands to PostProvision -// and Validate. +// venvRecordingPM records the folders the pipeline hands to PostProvision and +// Validate. type venvRecordingPM struct { fakePM - postProvisionVenv, validateVenv *string + postProvisionDir, postProvisionVenv, validateDir, validateVenv string } -func (f venvRecordingPM) PostProvision(_ context.Context, venv string) error { - *f.postProvisionVenv = venv +func (f *venvRecordingPM) PostProvision(_ context.Context, projectDir, venv string) error { + f.postProvisionDir, f.postProvisionVenv = projectDir, venv return nil } -func (f venvRecordingPM) Validate(ctx context.Context, venv string) (VenvInfo, error) { - *f.validateVenv = venv - return f.fakePM.Validate(ctx, venv) +func (f *venvRecordingPM) Validate(ctx context.Context, projectDir, venv string) (VenvInfo, error) { + f.validateDir, f.validateVenv = projectDir, venv + return f.fakePM.Validate(ctx, projectDir, venv) } func TestPipelinePassesProjectVenvToPackageManager(t *testing.T) { dir := writeProject(t) - var post, val string + pm := &venvRecordingPM{fakePM: fakePM{py: "3.12", dbc: "17.2.0"}} p := newServerlessTestPipeline(t, dir) - p.PM = venvRecordingPM{fakePM: fakePM{py: "3.12", dbc: "17.2.0"}, postProvisionVenv: &post, validateVenv: &val} + p.PM = pm res, err := p.Run(t.Context()) require.NoError(t, err) want := filepath.Join(dir, ".venv") - assert.Equal(t, want, post) - assert.Equal(t, want, val) + assert.Equal(t, dir, pm.postProvisionDir) + assert.Equal(t, want, pm.postProvisionVenv) + assert.Equal(t, dir, pm.validateDir) + assert.Equal(t, want, pm.validateVenv) assert.Equal(t, ".venv", res.VenvPath) } @@ -1471,7 +1473,7 @@ func TestWritePyprojectNoOpWritesNothing(t *testing.T) { content := []byte("[project]\nname = \"x\"\n") require.NoError(t, os.WriteFile(path, content, 0o644)) - p := &Pipeline{ProjectDir: dir, res: NewResult()} + p := &Pipeline{ProjectDir: dir} backup, mutated, perr := p.writePyproject(path, content, false) require.Nil(t, perr) assert.Empty(t, backup) @@ -1485,7 +1487,7 @@ func TestWritePyprojectBacksUpInItsOwnFolder(t *testing.T) { path := filepath.Join(other, pyprojectFile) require.NoError(t, os.WriteFile(path, []byte("[project]\nname = \"x\"\n"), 0o644)) - p := &Pipeline{ProjectDir: project, res: NewResult()} + p := &Pipeline{ProjectDir: project} backup, mutated, perr := p.writePyproject(path, []byte("[project]\nname = \"y\"\n"), false) require.Nil(t, perr) assert.True(t, mutated) diff --git a/libs/localenv/pkgmanager.go b/libs/localenv/pkgmanager.go index c474f9963ea..dba3b084fa3 100644 --- a/libs/localenv/pkgmanager.go +++ b/libs/localenv/pkgmanager.go @@ -22,17 +22,17 @@ type PackageManager interface { Provision(ctx context.Context, projectDir, python string) error // PostProvision seeds pip into the virtual environment at venv (an absolute - // path to the venv folder). + // path to the venv folder), running in projectDir. // This step is required because VS Code's ms-python.vscode-python-envs // extension falls back to `python -m pip list` when its `uv --version` // probe fails on the GUI PATH; uv venvs contain no pip; and `uv sync` // strips pip, so seeding must run after every sync. - PostProvision(ctx context.Context, venv string) error + PostProvision(ctx context.Context, projectDir, venv string) error - // Validate inspects the provisioned virtual environment at venv and returns - // what it observed (see VenvInfo). The caller decides which observations are - // acceptable. - Validate(ctx context.Context, venv string) (VenvInfo, error) + // Validate inspects the provisioned virtual environment at venv, running in + // projectDir, and returns what it observed (see VenvInfo). The caller decides + // which observations are acceptable. + Validate(ctx context.Context, projectDir, venv string) (VenvInfo, error) } // PythonResolution records which preparation path supplied the interpreter. diff --git a/libs/localenv/uv.go b/libs/localenv/uv.go index 9e78350e2c9..26cf9270039 100644 --- a/libs/localenv/uv.go +++ b/libs/localenv/uv.go @@ -203,7 +203,7 @@ func venvPython(venv string) string { return filepath.Join(venv, "bin", "python") } -// PostProvision seeds pip into the project's virtual environment. +// PostProvision seeds pip into the virtual environment at venv. // // VS Code's ms-python.vscode-python-envs extension falls back to // `python -m pip list` when its `uv --version` probe fails on the GUI PATH. @@ -211,9 +211,9 @@ func venvPython(venv string) string { // pip if it was previously present. Seeding pip after every sync ensures the // VS Code integration works correctly regardless of how the environment was // activated. -func (m *uvManager) PostProvision(ctx context.Context, venv string) error { +func (m *uvManager) PostProvision(ctx context.Context, projectDir, venv string) error { args := append([]string{m.bin}, m.pipSeedArgs(venvPython(venv))...) - if err := m.runUv(ctx, args, filepath.Dir(venv)); err != nil { + if err := m.runUv(ctx, args, projectDir); err != nil { return uvFailure(ErrProvision, err, "uv pip seed") } return nil @@ -231,7 +231,7 @@ func (m *uvManager) PostProvision(ctx context.Context, venv string) error { // on top of it. Metadata alone cannot tell a live collision from a stale dist-info that // no longer describes the importable module, so the probe also attempts `import // databricks.connect`: a live collision raises there, a harmless leftover does not. -func (m *uvManager) Validate(ctx context.Context, venv string) (VenvInfo, error) { +func (m *uvManager) Validate(ctx context.Context, projectDir, venv string) (VenvInfo, error) { // Each value is printed with a unique prefix so parsing greps for the prefix rather // than relying on line position: any stray line uv or the interpreter writes to // stdout (e.g. a warning) would otherwise shift a positional parse. @@ -256,7 +256,7 @@ except BaseException as e: // what was installed, so validation observes the real target. out, err := process.Background(ctx, []string{venvPython(venv), "-c", pyCode}, - process.WithDir(filepath.Dir(venv)), + process.WithDir(projectDir), process.WithProcessGroup(), ) if err != nil { diff --git a/libs/localenv/uv_test.go b/libs/localenv/uv_test.go index 152bd27c360..04b14685d46 100644 --- a/libs/localenv/uv_test.go +++ b/libs/localenv/uv_test.go @@ -475,3 +475,25 @@ func TestLineWithPrefix(t *testing.T) { _, ok = lineWithPrefix("PYVER:3.12\n", validateDBCPrefix) assert.False(t, ok) } + +func TestPostProvisionAndValidateRunInProjectDir(t *testing.T) { + // The venv can live outside the project (a uv workspace root), but uv and the + // probe interpreter still run in the project folder: uv reads its settings + // from there, and `python -c` puts the working folder first on sys.path. + project := filepath.Join("p", "proj") + venv := filepath.Join("p", ".venv") + ctx, stub := process.WithStub(t.Context()) + var dirs []string + stub.WithCallback(func(cmd *exec.Cmd) error { + dirs = append(dirs, cmd.Dir) + // The validate probe parses its prefixed lines from stdout. + _, err := cmd.Stdout.Write([]byte("PYVER:3.12\nDBCVER:\nPYSPARKVER:\nDBCIMPORT:\n")) + return err + }) + m := &uvManager{bin: "uv"} + + require.NoError(t, m.PostProvision(ctx, project, venv)) + _, err := m.Validate(ctx, project, venv) + require.NoError(t, err) + assert.Equal(t, []string{project, project}, dirs) +} From 29539edb07e0266e4b6537f97235b9cfaed16366 Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Mon, 12 Oct 2026 13:55:14 +0200 Subject: [PATCH 4/5] localenv: split the venv fields and update the Validate doc comment Co-authored-by: Isaac --- libs/localenv/pipeline.go | 9 ++++++--- libs/localenv/uv.go | 2 +- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/libs/localenv/pipeline.go b/libs/localenv/pipeline.go index e1c4d14e9a6..81c55837e2d 100644 --- a/libs/localenv/pipeline.go +++ b/libs/localenv/pipeline.go @@ -74,9 +74,12 @@ type Pipeline struct { res *Result // venv is the absolute virtualenv folder that provision seeds and validate - // inspects; venvRel is the same folder relative to ProjectDir, reported as - // Result.VenvPath. Both are set in preflight. - venv, venvRel string + // inspects. Set in preflight. + venv string + + // venvRel is the venv folder relative to ProjectDir, reported as + // Result.VenvPath. Set in preflight. + venvRel string // nowFn supplies the time for backup filenames; nil means time.Now. Injected // in tests so backup names are deterministic. diff --git a/libs/localenv/uv.go b/libs/localenv/uv.go index 26cf9270039..c62b84429ac 100644 --- a/libs/localenv/uv.go +++ b/libs/localenv/uv.go @@ -219,7 +219,7 @@ func (m *uvManager) PostProvision(ctx context.Context, projectDir, venv string) return nil } -// Validate inspects the project's virtual environment: the Python minor version, the +// Validate inspects the virtual environment at venv: the Python minor version, the // databricks-connect and standalone-pyspark distribution versions, and whether // databricks-connect actually imports. A missing databricks-connect or pyspark yields // an empty version rather than an error (PackageNotFoundError is caught), so the probe From 3bc7249b02f8be6f2124df763f2823f2f4e0ae3e Mon Sep 17 00:00:00 2001 From: Grigory Panov Date: Mon, 12 Oct 2026 18:09:08 +0200 Subject: [PATCH 5/5] localenv: check that the venv interpreter reaches the uv commands Co-authored-by: Isaac --- libs/localenv/uv_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/localenv/uv_test.go b/libs/localenv/uv_test.go index 04b14685d46..3add58cb4ac 100644 --- a/libs/localenv/uv_test.go +++ b/libs/localenv/uv_test.go @@ -484,8 +484,10 @@ func TestPostProvisionAndValidateRunInProjectDir(t *testing.T) { venv := filepath.Join("p", ".venv") ctx, stub := process.WithStub(t.Context()) var dirs []string + var args [][]string stub.WithCallback(func(cmd *exec.Cmd) error { dirs = append(dirs, cmd.Dir) + args = append(args, cmd.Args) // The validate probe parses its prefixed lines from stdout. _, err := cmd.Stdout.Write([]byte("PYVER:3.12\nDBCVER:\nPYSPARKVER:\nDBCIMPORT:\n")) return err @@ -496,4 +498,8 @@ func TestPostProvisionAndValidateRunInProjectDir(t *testing.T) { _, err := m.Validate(ctx, project, venv) require.NoError(t, err) assert.Equal(t, []string{project, project}, dirs) + // Both commands use the interpreter of the venv folder, not of the project. + require.Len(t, args, 2) + assert.Contains(t, args[0], venvPython(venv), "pip seed") + assert.Equal(t, venvPython(venv), args[1][0], "validate probe") }