From 60115f8c2793a07bdda52b9d938b0260f2202114 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Mon, 17 Aug 2026 01:01:30 -0400 Subject: [PATCH 01/10] fix(setup): resolve pip and python from PATH before installing Recent macOS and Homebrew installs ship python3/pip3 with no bare python or pip, so the hardcoded `pip install` failed outright on a common developer box. Probe pip3 then pip, falling back to ` -m pip` for interpreters installed without a pip shim. Every package manager is now checked for existence before being run, so a missing tool reports what to install instead of an exec "not found" error. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/installer.go | 72 ++++++++++++++++++++- internal/setup/installer_test.go | 105 +++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+), 1 deletion(-) diff --git a/internal/setup/installer.go b/internal/setup/installer.go index f538ad0d..e160e322 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -94,6 +94,17 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install }, nil } + // Confirm the tool exists before shelling out, so a missing package manager + // reports what to install instead of surfacing an exec "not found" error. + if reason := missingToolReason(args[0]); reason != "" { + return &InstallResult{ + SDKID: detection.SDKID, + Package: pkg, + Failed: true, + FailureReason: reason, + }, nil + } + if detection.SDKID == "dotnet-server-sdk" { target, reason := dotnetProjectArg(dir) if reason != "" { @@ -151,6 +162,36 @@ func dotnetProjectArg(dir string) (args []string, reason string) { } } +// installHints maps a package-manager executable to how the user can get it. +var installHints = map[string]string{ + "pip": "install Python from https://www.python.org/downloads or your package manager", + "pip3": "install Python from https://www.python.org/downloads or your package manager", + "python": "install Python from https://www.python.org/downloads or your package manager", + "poetry": "see https://python-poetry.org/docs/#installation", + "uv": "see https://docs.astral.sh/uv/getting-started/installation", + "pipenv": "see https://pipenv.pypa.io/en/latest/installation.html", + "npm": "install Node.js from https://nodejs.org", + "yarn": "see https://yarnpkg.com/getting-started/install", + "pnpm": "see https://pnpm.io/installation", + "bun": "see https://bun.sh/docs/installation", + "bundle": "run `gem install bundler`", + "gem": "install Ruby from https://www.ruby-lang.org/en/documentation/installation", + "go": "install Go from https://go.dev/dl", + "dotnet": "install the .NET SDK from https://dotnet.microsoft.com/download", +} + +// missingToolReason returns an explanation when tool is not on PATH, or an empty +// string when it is available. +func missingToolReason(tool string) string { + if onPath(tool) { + return "" + } + if hint, ok := installHints[tool]; ok { + return fmt.Sprintf("%s is not installed or not on your PATH — %s", tool, hint) + } + return fmt.Sprintf("%s is not installed or not on your PATH", tool) +} + func execRun(dir string, args []string) ([]byte, error) { cmd := exec.Command(args[0], args[1:]...) //nolint:gosec cmd.Dir = dir @@ -207,6 +248,15 @@ func InstallArgs(sdkID, packageManager string) (args []string, pkg string) { } } +// lookPath is indirected so tests can control which executables appear to exist. +var lookPath = exec.LookPath + +// onPath reports whether name is an executable on PATH. +func onPath(name string) bool { + _, err := lookPath(name) + return err == nil +} + // pythonInstallCmd returns the install command arguments for a Python package // manager. Anything unrecognised — including the empty string, which IsInstalled // passes — falls back to pip. @@ -219,8 +269,28 @@ func pythonInstallCmd(pm, pkg string) []string { case "pipenv": return []string{"pipenv", "install", pkg} default: - return []string{"pip", "install", pkg} + return pipInstallCmd(pkg) + } +} + +// pipInstallCmd returns the pip install command, choosing the first tool that is +// actually on PATH. Recent macOS and Homebrew installs ship python3/pip3 with no +// bare python/pip, so a hardcoded `pip` fails outright on a common developer box. +// Falling back to `-m pip` covers interpreters installed without a pip shim. +// With nothing on PATH the bare `pip` form is returned so the plan screen still +// shows a sensible command; Install's pre-flight check reports what is missing. +func pipInstallCmd(pkg string) []string { + for _, bin := range []string{"pip3", "pip"} { + if onPath(bin) { + return []string{bin, "install", pkg} + } + } + for _, bin := range []string{"python3", "python"} { + if onPath(bin) { + return []string{bin, "-m", "pip", "install", pkg} + } } + return []string{"pip", "install", pkg} } // nodeInstallCmd returns the install command arguments for a Node.js package manager. diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 59a1b1ed..bbc54f69 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -3,6 +3,7 @@ package setup import ( "errors" "os" + "os/exec" "path/filepath" "testing" @@ -44,6 +45,24 @@ func TestInstallArgs_NodeSDKs(t *testing.T) { } } +// stubPath makes only the named executables appear to exist on PATH for the rest +// of the test. +func stubPath(t *testing.T, available ...string) { + t.Helper() + set := make(map[string]bool, len(available)) + for _, name := range available { + set[name] = true + } + original := lookPath + lookPath = func(name string) (string, error) { + if set[name] { + return "/usr/bin/" + name, nil + } + return "", exec.ErrNotFound + } + t.Cleanup(func() { lookPath = original }) +} + func TestInstallArgs_Python(t *testing.T) { tests := []struct { packageManager string @@ -60,6 +79,7 @@ func TestInstallArgs_Python(t *testing.T) { } for _, tt := range tests { t.Run(tt.packageManager, func(t *testing.T) { + stubPath(t, "pip", "poetry", "uv", "pipenv") args, pkg := InstallArgs("python-server-sdk", tt.packageManager) assert.Equal(t, tt.want, args) assert.Equal(t, "launchdarkly-server-sdk", pkg) @@ -67,6 +87,87 @@ func TestInstallArgs_Python(t *testing.T) { } } +func TestInstallArgs_Python_ResolvesAvailableTool(t *testing.T) { + pkg := "launchdarkly-server-sdk" + tests := []struct { + name string + available []string + want []string + }{ + // Recent macOS and Homebrew ship pip3 with no bare pip. + {"only pip3", []string{"pip3", "python3"}, []string{"pip3", "install", pkg}}, + {"only pip", []string{"pip", "python"}, []string{"pip", "install", pkg}}, + // pip3 wins so a stale python2 pip is never chosen. + {"both pip and pip3", []string{"pip", "pip3"}, []string{"pip3", "install", pkg}}, + // An interpreter with no pip shim still has the module. + {"python3 only", []string{"python3"}, []string{"python3", "-m", "pip", "install", pkg}}, + {"python only", []string{"python"}, []string{"python", "-m", "pip", "install", pkg}}, + {"python3 preferred", []string{"python", "python3"}, []string{"python3", "-m", "pip", "install", pkg}}, + // Nothing available: keep a displayable command; Install reports the problem. + {"nothing available", nil, []string{"pip", "install", pkg}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + stubPath(t, tt.available...) + args, _ := InstallArgs("python-server-sdk", "") + assert.Equal(t, tt.want, args) + }) + } +} + +func TestInstall_MissingToolReportsFailureNotExecError(t *testing.T) { + stubPath(t) // nothing on PATH + dir := t.TempDir() + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + t.Fatal("must not shell out to a tool that does not exist") + return nil, nil + }, + } + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.True(t, result.Failed) + assert.False(t, result.Success) + assert.Contains(t, result.FailureReason, "pip is not installed or not on your PATH") + assert.Contains(t, result.FailureReason, "python.org") +} + +func TestInstall_MissingNodeToolReportsFailure(t *testing.T) { + stubPath(t) // npm absent + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + t.Fatal("must not shell out to a tool that does not exist") + return nil, nil + }, + } + + result, err := installer.Install(t.TempDir(), &DetectResult{SDKID: "node-server"}) + + require.NoError(t, err) + assert.True(t, result.Failed) + assert.Contains(t, result.FailureReason, "npm is not installed") +} + +func TestInstall_PresentToolRuns(t *testing.T) { + stubPath(t, "npm") + var ran []string + installer := PackageInstaller{ + run: func(_ string, args []string) ([]byte, error) { + ran = args + return nil, nil + }, + } + + result, err := installer.Install(t.TempDir(), &DetectResult{SDKID: "node-server"}) + + require.NoError(t, err) + assert.True(t, result.Success) + assert.False(t, result.Failed) + assert.Equal(t, []string{"npm", "install", "@launchdarkly/node-server-sdk"}, ran) +} + func TestInstallArgs_Go(t *testing.T) { args, pkg := InstallArgs("go-server-sdk", "") require.NotEmpty(t, args) @@ -286,6 +387,7 @@ func TestIsInstalled_Dotnet_FindsNestedProject(t *testing.T) { } func TestInstall_Dotnet_SolutionLayout_TargetsTheProject(t *testing.T) { + stubPath(t, "dotnet") dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "MyApp.sln"), []byte(""), 0600)) require.NoError(t, os.MkdirAll(filepath.Join(dir, "src/MyApp"), 0755)) @@ -307,6 +409,7 @@ func TestInstall_Dotnet_SolutionLayout_TargetsTheProject(t *testing.T) { } func TestInstall_Dotnet_SingleRootProject_RunsBareCommand(t *testing.T) { + stubPath(t, "dotnet") dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "MyApp.csproj"), []byte(""), 0600)) @@ -324,6 +427,7 @@ func TestInstall_Dotnet_SingleRootProject_RunsBareCommand(t *testing.T) { // Adding the SDK to an arbitrary assembly is worse than saying which projects exist. func TestInstall_Dotnet_SeveralProjects_ReportsWhyItStopped(t *testing.T) { + stubPath(t, "dotnet") dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "MyApp.sln"), []byte(""), 0600)) for _, p := range []string{"src/Api/Api.csproj", "src/Worker/Worker.csproj"} { @@ -348,6 +452,7 @@ func TestInstall_Dotnet_SeveralProjects_ReportsWhyItStopped(t *testing.T) { } func TestInstall_Dotnet_NoProjectAtAll_ReportsWhyItStopped(t *testing.T) { + stubPath(t, "dotnet") dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "MyApp.sln"), []byte(""), 0600)) From 58888a4fc5cdf7100678712d21eede0317446126 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Mon, 17 Aug 2026 12:45:48 -0400 Subject: [PATCH 02/10] fix(setup): verify pip is importable before using the -m pip form Debian and Ubuntu package pip separately from the interpreter, so a present python3 does not mean `python3 -m pip` can run. The pre-flight check only looked at the executable, so those boxes got "No module named pip" instead of guidance. Stub PATH in the two install tests that reached for a real npm, which the pre-flight check made environment-dependent. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/installer.go | 27 ++++++++++++++++-- internal/setup/installer_test.go | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/internal/setup/installer.go b/internal/setup/installer.go index e160e322..58e1769e 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -94,9 +94,9 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install }, nil } - // Confirm the tool exists before shelling out, so a missing package manager + // Confirm the command can run before shelling out, so a missing package manager // reports what to install instead of surfacing an exec "not found" error. - if reason := missingToolReason(args[0]); reason != "" { + if reason := preflightReason(args); reason != "" { return &InstallResult{ SDKID: detection.SDKID, Package: pkg, @@ -192,6 +192,29 @@ func missingToolReason(tool string) string { return fmt.Sprintf("%s is not installed or not on your PATH", tool) } +// pipModuleAvailable reports whether interpreter can run pip as a module. It is +// indirected so tests need not execute a real interpreter. +var pipModuleAvailable = func(interpreter string) bool { + return exec.Command(interpreter, "-m", "pip", "--version").Run() == nil //nolint:gosec +} + +// preflightReason returns why args cannot run, or an empty string when they can. +// It checks the executable and, for the ` -m pip` form, that pip is +// actually importable: Debian and Ubuntu package pip separately from the +// interpreter, so a present python3 does not imply a usable pip. +func preflightReason(args []string) string { + if reason := missingToolReason(args[0]); reason != "" { + return reason + } + if len(args) > 2 && args[1] == "-m" && args[2] == "pip" && !pipModuleAvailable(args[0]) { + return fmt.Sprintf( + "%s has no pip module — install it with `%s -m ensurepip --upgrade`, or your distribution's python3-pip package", + args[0], args[0], + ) + } + return "" +} + func execRun(dir string, args []string) ([]byte, error) { cmd := exec.Command(args[0], args[1:]...) //nolint:gosec cmd.Dir = dir diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index bbc54f69..9e143015 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -63,6 +63,14 @@ func stubPath(t *testing.T, available ...string) { t.Cleanup(func() { lookPath = original }) } +// stubPipModule controls whether an interpreter appears able to import pip. +func stubPipModule(t *testing.T, available bool) { + t.Helper() + original := pipModuleAvailable + pipModuleAvailable = func(string) bool { return available } + t.Cleanup(func() { pipModuleAvailable = original }) +} + func TestInstallArgs_Python(t *testing.T) { tests := []struct { packageManager string @@ -134,6 +142,44 @@ func TestInstall_MissingToolReportsFailureNotExecError(t *testing.T) { assert.Contains(t, result.FailureReason, "python.org") } +// Debian and Ubuntu package pip separately from the interpreter, so python3 being +// present does not mean `python3 -m pip` can run. +func TestInstall_InterpreterWithoutPipModuleReportsFailure(t *testing.T) { + stubPath(t, "python3") + stubPipModule(t, false) + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + t.Fatal("must not run an interpreter that cannot import pip") + return nil, nil + }, + } + + result, err := installer.Install(t.TempDir(), &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.True(t, result.Failed) + assert.Contains(t, result.FailureReason, "python3 has no pip module") + assert.Contains(t, result.FailureReason, "ensurepip") +} + +func TestInstall_InterpreterWithPipModuleRuns(t *testing.T) { + stubPath(t, "python3") + stubPipModule(t, true) + var ran []string + installer := PackageInstaller{ + run: func(_ string, args []string) ([]byte, error) { + ran = args + return nil, nil + }, + } + + result, err := installer.Install(t.TempDir(), &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.True(t, result.Success) + assert.Equal(t, []string{"python3", "-m", "pip", "install", "launchdarkly-server-sdk"}, ran) +} + func TestInstall_MissingNodeToolReportsFailure(t *testing.T) { stubPath(t) // npm absent installer := PackageInstaller{ @@ -229,6 +275,7 @@ func TestInstallArgs_ManualSDKs(t *testing.T) { } func TestPackageInstaller_Install_Success(t *testing.T) { + stubPath(t, "npm") var capturedDir string var capturedArgs []string @@ -255,6 +302,7 @@ func TestPackageInstaller_Install_Success(t *testing.T) { } func TestPackageInstaller_Install_CommandFailure(t *testing.T) { + stubPath(t, "npm") installer := PackageInstaller{ run: func(dir string, args []string) ([]byte, error) { return []byte("npm ERR! not found"), errors.New("exit status 1") From 7327cb1f8dd2678fca1d1bd4734ca976306cf682 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Mon, 17 Aug 2026 12:58:03 -0400 Subject: [PATCH 03/10] fix(setup): never substitute a Python interpreter for a missing pip Using `python3 -m pip`, or bootstrapping pip with ensurepip, would install tooling onto the user's machine. Setup only ever uses a pip that is already there; when none is found it warns and runs nothing. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/installer.go | 50 +++++++++----------------------- internal/setup/installer_test.go | 49 +++++++------------------------ 2 files changed, 23 insertions(+), 76 deletions(-) diff --git a/internal/setup/installer.go b/internal/setup/installer.go index 58e1769e..68a6ff11 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -94,9 +94,10 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install }, nil } - // Confirm the command can run before shelling out, so a missing package manager - // reports what to install instead of surfacing an exec "not found" error. - if reason := preflightReason(args); reason != "" { + // Confirm the tool exists before shelling out, so a missing package manager + // warns with what to install instead of surfacing an exec "not found" error. + // We never install the tool ourselves. + if reason := missingToolReason(args[0]); reason != "" { return &InstallResult{ SDKID: detection.SDKID, Package: pkg, @@ -166,7 +167,6 @@ func dotnetProjectArg(dir string) (args []string, reason string) { var installHints = map[string]string{ "pip": "install Python from https://www.python.org/downloads or your package manager", "pip3": "install Python from https://www.python.org/downloads or your package manager", - "python": "install Python from https://www.python.org/downloads or your package manager", "poetry": "see https://python-poetry.org/docs/#installation", "uv": "see https://docs.astral.sh/uv/getting-started/installation", "pipenv": "see https://pipenv.pypa.io/en/latest/installation.html", @@ -192,28 +192,6 @@ func missingToolReason(tool string) string { return fmt.Sprintf("%s is not installed or not on your PATH", tool) } -// pipModuleAvailable reports whether interpreter can run pip as a module. It is -// indirected so tests need not execute a real interpreter. -var pipModuleAvailable = func(interpreter string) bool { - return exec.Command(interpreter, "-m", "pip", "--version").Run() == nil //nolint:gosec -} - -// preflightReason returns why args cannot run, or an empty string when they can. -// It checks the executable and, for the ` -m pip` form, that pip is -// actually importable: Debian and Ubuntu package pip separately from the -// interpreter, so a present python3 does not imply a usable pip. -func preflightReason(args []string) string { - if reason := missingToolReason(args[0]); reason != "" { - return reason - } - if len(args) > 2 && args[1] == "-m" && args[2] == "pip" && !pipModuleAvailable(args[0]) { - return fmt.Sprintf( - "%s has no pip module — install it with `%s -m ensurepip --upgrade`, or your distribution's python3-pip package", - args[0], args[0], - ) - } - return "" -} func execRun(dir string, args []string) ([]byte, error) { cmd := exec.Command(args[0], args[1:]...) //nolint:gosec @@ -296,23 +274,21 @@ func pythonInstallCmd(pm, pkg string) []string { } } -// pipInstallCmd returns the pip install command, choosing the first tool that is -// actually on PATH. Recent macOS and Homebrew installs ship python3/pip3 with no -// bare python/pip, so a hardcoded `pip` fails outright on a common developer box. -// Falling back to `-m pip` covers interpreters installed without a pip shim. -// With nothing on PATH the bare `pip` form is returned so the plan screen still -// shows a sensible command; Install's pre-flight check reports what is missing. +// pipInstallCmd returns the pip install command, choosing whichever of pip3 and +// pip is on PATH. Recent macOS and Homebrew installs ship pip3 with no bare pip, +// so a hardcoded `pip` fails outright on a common developer box. +// +// Only an existing pip is used. Reaching past it — to `python3 -m pip`, or to +// bootstrapping pip with ensurepip — would install tooling onto the user's +// machine, which is not ours to do. When no pip is found the bare form is +// returned so the plan screen has something to show, and Install's pre-flight +// check warns instead of running anything. func pipInstallCmd(pkg string) []string { for _, bin := range []string{"pip3", "pip"} { if onPath(bin) { return []string{bin, "install", pkg} } } - for _, bin := range []string{"python3", "python"} { - if onPath(bin) { - return []string{bin, "-m", "pip", "install", pkg} - } - } return []string{"pip", "install", pkg} } diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 9e143015..66309873 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -63,14 +63,6 @@ func stubPath(t *testing.T, available ...string) { t.Cleanup(func() { lookPath = original }) } -// stubPipModule controls whether an interpreter appears able to import pip. -func stubPipModule(t *testing.T, available bool) { - t.Helper() - original := pipModuleAvailable - pipModuleAvailable = func(string) bool { return available } - t.Cleanup(func() { pipModuleAvailable = original }) -} - func TestInstallArgs_Python(t *testing.T) { tests := []struct { packageManager string @@ -107,11 +99,9 @@ func TestInstallArgs_Python_ResolvesAvailableTool(t *testing.T) { {"only pip", []string{"pip", "python"}, []string{"pip", "install", pkg}}, // pip3 wins so a stale python2 pip is never chosen. {"both pip and pip3", []string{"pip", "pip3"}, []string{"pip3", "install", pkg}}, - // An interpreter with no pip shim still has the module. - {"python3 only", []string{"python3"}, []string{"python3", "-m", "pip", "install", pkg}}, - {"python only", []string{"python"}, []string{"python", "-m", "pip", "install", pkg}}, - {"python3 preferred", []string{"python", "python3"}, []string{"python3", "-m", "pip", "install", pkg}}, - // Nothing available: keep a displayable command; Install reports the problem. + // An interpreter is not a stand-in for pip: setup will not bootstrap tooling, + // so the bare form is kept and Install warns rather than running it. + {"interpreters but no pip", []string{"python3", "python"}, []string{"pip", "install", pkg}}, {"nothing available", nil, []string{"pip", "install", pkg}}, } for _, tt := range tests { @@ -142,14 +132,13 @@ func TestInstall_MissingToolReportsFailureNotExecError(t *testing.T) { assert.Contains(t, result.FailureReason, "python.org") } -// Debian and Ubuntu package pip separately from the interpreter, so python3 being -// present does not mean `python3 -m pip` can run. -func TestInstall_InterpreterWithoutPipModuleReportsFailure(t *testing.T) { - stubPath(t, "python3") - stubPipModule(t, false) +// A Python interpreter is not a substitute for pip: using it would mean installing +// tooling onto the user's machine, so setup warns instead. +func TestInstall_InterpreterWithoutPipWarnsAndRunsNothing(t *testing.T) { + stubPath(t, "python3", "python") installer := PackageInstaller{ run: func(string, []string) ([]byte, error) { - t.Fatal("must not run an interpreter that cannot import pip") + t.Fatal("must not install anything when pip is absent") return nil, nil }, } @@ -158,26 +147,8 @@ func TestInstall_InterpreterWithoutPipModuleReportsFailure(t *testing.T) { require.NoError(t, err) assert.True(t, result.Failed) - assert.Contains(t, result.FailureReason, "python3 has no pip module") - assert.Contains(t, result.FailureReason, "ensurepip") -} - -func TestInstall_InterpreterWithPipModuleRuns(t *testing.T) { - stubPath(t, "python3") - stubPipModule(t, true) - var ran []string - installer := PackageInstaller{ - run: func(_ string, args []string) ([]byte, error) { - ran = args - return nil, nil - }, - } - - result, err := installer.Install(t.TempDir(), &DetectResult{SDKID: "python-server-sdk"}) - - require.NoError(t, err) - assert.True(t, result.Success) - assert.Equal(t, []string{"python3", "-m", "pip", "install", "launchdarkly-server-sdk"}, ran) + assert.Contains(t, result.FailureReason, "pip is not installed or not on your PATH") + assert.NotContains(t, result.FailureReason, "ensurepip") } func TestInstall_MissingNodeToolReportsFailure(t *testing.T) { From 939a447b63010dfcfbddd93c1095a854750934c8 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Tue, 18 Aug 2026 11:16:21 -0400 Subject: [PATCH 04/10] fix(setup): install Python packages into the project virtualenv pip refuses to write into an OS-managed Python, which Homebrew and most current distributions now mark, so a project with no active virtualenv could not complete setup at all. A virtualenv's pip is used when the project or the environment has one, and the refusal is explained rather than passed through as pip's raw error. InstallArgs takes the project directory so the previewed command is the one that actually runs. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/setup/commands.go | 2 +- cmd/setup/install.go | 2 +- cmd/setup/update.go | 10 ++- internal/setup/installer.go | 89 +++++++++++++++++++--- internal/setup/installer_test.go | 126 ++++++++++++++++++++++++++++--- 5 files changed, 202 insertions(+), 27 deletions(-) diff --git a/cmd/setup/commands.go b/cmd/setup/commands.go index 994c0709..85bd479e 100644 --- a/cmd/setup/commands.go +++ b/cmd/setup/commands.go @@ -85,7 +85,7 @@ func (m wizardModel) runInstall() tea.Cmd { if err != nil { // Don't dead-end the interactive flow on a failed auto-install (e.g. // Ruby gem perms, no network): surface the command to run by hand. - args, _ := setup.InstallArgs(m.detectResult.SDKID, m.detectResult.PackageManager) + args, _ := setup.InstallArgs(dir, m.detectResult.SDKID, m.detectResult.PackageManager) return installDoneMsg{result: &setup.InstallResult{ SDKID: m.detectResult.SDKID, Command: strings.Join(args, " "), diff --git a/cmd/setup/install.go b/cmd/setup/install.go index 86684322..68a3bb6f 100644 --- a/cmd/setup/install.go +++ b/cmd/setup/install.go @@ -55,7 +55,7 @@ func runInstall(svc setup.Service) func(*cobra.Command, []string) error { var result *setup.InstallResult if dryRun { - args, pkg := setup.InstallArgs(sdkID, pkgMgr) + args, pkg := setup.InstallArgs(dir, sdkID, pkgMgr) result = &setup.InstallResult{ SDKID: sdkID, Package: pkg, diff --git a/cmd/setup/update.go b/cmd/setup/update.go index 0bff503d..669e8096 100644 --- a/cmd/setup/update.go +++ b/cmd/setup/update.go @@ -368,11 +368,13 @@ func (m wizardModel) handleEnter() (tea.Model, tea.Cmd) { } } m.detectResult = &result - // Compute the plan preview shown before any action is taken. - args, _ := setup.InstallArgs(chosen.id, result.PackageManager) + // Compute the plan preview shown before any action is taken. It is resolved + // against the project directory so the previewed command is the one that runs. + planDir, _ := os.Getwd() + args, _ := setup.InstallArgs(planDir, chosen.id, result.PackageManager) m.planInstallCmd = strings.Join(args, " ") - if dir, err := os.Getwd(); err == nil { - m.planAlready = setup.IsInstalled(dir, chosen.id) + if planDir != "" { + m.planAlready = setup.IsInstalled(planDir, chosen.id) } m.step = stepPlan return m, nil diff --git a/internal/setup/installer.go b/internal/setup/installer.go index 68a6ff11..35fb7bc2 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -1,6 +1,7 @@ package setup import ( + "bytes" "errors" "fmt" "io/fs" @@ -72,7 +73,7 @@ var manualInstallSDKs = map[string]bool{ // returns a result with Success=false without returning an error. An unknown SDK // ID returns an error. func (p PackageInstaller) Install(dir string, detection *DetectResult) (*InstallResult, error) { - args, pkg := InstallArgs(detection.SDKID, detection.PackageManager) + args, pkg := InstallArgs(dir, detection.SDKID, detection.PackageManager) if len(args) == 0 { if !manualInstallSDKs[detection.SDKID] { return nil, fmt.Errorf("unknown SDK %q: no install command available; specify a supported --sdk-id", detection.SDKID) @@ -127,6 +128,15 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install out, err := runner(dir, args) command := strings.Join(args, " ") if err != nil { + if reason := externallyManagedReason(dir, out); reason != "" { + return &InstallResult{ + SDKID: detection.SDKID, + Package: pkg, + Command: command, + Failed: true, + FailureReason: reason, + }, nil + } return nil, fmt.Errorf("%s: %w\n%s", command, err, strings.TrimSpace(string(out))) } return &InstallResult{ @@ -163,6 +173,30 @@ func dotnetProjectArg(dir string) (args []string, reason string) { } } +// externallyManagedReason recognises a PEP 668 refusal and says what to do about +// it. Homebrew and most current Linux distributions mark their Python as managed +// by the OS package manager, so pip declines to write into it. A virtualenv is the +// supported way through, and it is the user's to create: installing into their +// system Python, or passing --break-system-packages to force it, risks breaking +// tools that Python came with. +func externallyManagedReason(dir string, out []byte) string { + if !bytes.Contains(out, []byte("externally-managed-environment")) { + return "" + } + target := dir + if target == "" { + target = "your project" + } + return fmt.Sprintf( + "this Python is managed by your operating system, so pip will not install into it. "+ + "Create a virtual environment in %s and run setup again:\n"+ + " python3 -m venv .venv\n"+ + " source .venv/bin/activate\n"+ + "Setup uses .venv automatically once it exists.", + target, + ) +} + // installHints maps a package-manager executable to how the user can get it. var installHints = map[string]string{ "pip": "install Python from https://www.python.org/downloads or your package manager", @@ -192,7 +226,6 @@ func missingToolReason(tool string) string { return fmt.Sprintf("%s is not installed or not on your PATH", tool) } - func execRun(dir string, args []string) ([]byte, error) { cmd := exec.Command(args[0], args[1:]...) //nolint:gosec cmd.Dir = dir @@ -202,7 +235,9 @@ func execRun(dir string, args []string) ([]byte, error) { // InstallArgs returns the command-line arguments and package name for installing the given SDK. // Returns nil args for SDKs that require manual installation (e.g. Java, Android, Swift). // packageManager is used for Node.js SDKs; for other runtimes the appropriate tool is chosen automatically. -func InstallArgs(sdkID, packageManager string) (args []string, pkg string) { +// dir is the project directory, which decides whether a virtualenv's pip is used; +// pass an empty string when there is no project in mind. +func InstallArgs(dir, sdkID, packageManager string) (args []string, pkg string) { switch sdkID { case "react-client-sdk": pkg = "launchdarkly-react-client-sdk" @@ -221,7 +256,7 @@ func InstallArgs(sdkID, packageManager string) (args []string, pkg string) { return nodeInstallCmd(resolveNodePM(packageManager), pkg), pkg case "python-server-sdk": pkg = "launchdarkly-server-sdk" - return pythonInstallCmd(packageManager, pkg), pkg + return pythonInstallCmd(dir, packageManager, pkg), pkg case "go-server-sdk": pkg = "github.com/launchdarkly/go-server-sdk/v7" return []string{"go", "get", pkg}, pkg @@ -261,7 +296,7 @@ func onPath(name string) bool { // pythonInstallCmd returns the install command arguments for a Python package // manager. Anything unrecognised — including the empty string, which IsInstalled // passes — falls back to pip. -func pythonInstallCmd(pm, pkg string) []string { +func pythonInstallCmd(dir, pm, pkg string) []string { switch pm { case "poetry": return []string{"poetry", "add", pkg} @@ -270,20 +305,52 @@ func pythonInstallCmd(pm, pkg string) []string { case "pipenv": return []string{"pipenv", "install", pkg} default: - return pipInstallCmd(pkg) + return pipInstallCmd(dir, pkg) + } +} + +// virtualEnv reports the active virtualenv, indirected so tests are not affected +// by the environment the suite happens to run in. +var virtualEnv = func() string { return os.Getenv("VIRTUAL_ENV") } + +// venvPip returns the pip belonging to the active virtualenv, or to one sitting in +// the project, and an empty string when there is none. A virtualenv is where a +// project's dependencies belong, and it is the only place pip can write on a +// PEP 668 interpreter, so it is preferred over whatever pip is on PATH. +func venvPip(dir string) string { + var roots []string + if active := virtualEnv(); active != "" { + roots = append(roots, active) } + // An empty dir means the caller has no project in mind, so relative lookups + // would search whatever directory the process happens to be in. + if dir != "" { + roots = append(roots, filepath.Join(dir, ".venv"), filepath.Join(dir, "venv")) + } + for _, root := range roots { + for _, rel := range []string{filepath.Join("bin", "pip"), filepath.Join("Scripts", "pip.exe")} { + candidate := filepath.Join(root, rel) + if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + return candidate + } + } + } + return "" } -// pipInstallCmd returns the pip install command, choosing whichever of pip3 and -// pip is on PATH. Recent macOS and Homebrew installs ship pip3 with no bare pip, -// so a hardcoded `pip` fails outright on a common developer box. +// pipInstallCmd returns the pip install command. A virtualenv's pip wins, then +// whichever of pip3 and pip is on PATH: recent macOS and Homebrew installs ship +// pip3 with no bare pip, so a hardcoded `pip` fails outright on a common box. // // Only an existing pip is used. Reaching past it — to `python3 -m pip`, or to // bootstrapping pip with ensurepip — would install tooling onto the user's // machine, which is not ours to do. When no pip is found the bare form is // returned so the plan screen has something to show, and Install's pre-flight // check warns instead of running anything. -func pipInstallCmd(pkg string) []string { +func pipInstallCmd(dir, pkg string) []string { + if pip := venvPip(dir); pip != "" { + return []string{pip, "install", pkg} + } for _, bin := range []string{"pip3", "pip"} { if onPath(bin) { return []string{bin, "install", pkg} @@ -321,7 +388,7 @@ func resolveNodePM(pm string) string { // covers SDKs with an automated install command; returns false for manual SDKs // and unknowns. func IsInstalled(dir, sdkID string) bool { - _, pkg := InstallArgs(sdkID, "") + _, pkg := InstallArgs(dir, sdkID, "") if pkg == "" { return false } diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 66309873..db47b2e4 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -36,7 +36,7 @@ func TestInstallArgs_NodeSDKs(t *testing.T) { for _, tt := range tests { t.Run(tt.sdkID+"/"+tt.pm, func(t *testing.T) { - args, pkg := InstallArgs(tt.sdkID, tt.pm) + args, pkg := InstallArgs("", tt.sdkID, tt.pm) require.NotEmpty(t, args) assert.Equal(t, tt.wantCmd, args[0]) assert.Equal(t, tt.wantPkg, pkg) @@ -80,7 +80,7 @@ func TestInstallArgs_Python(t *testing.T) { for _, tt := range tests { t.Run(tt.packageManager, func(t *testing.T) { stubPath(t, "pip", "poetry", "uv", "pipenv") - args, pkg := InstallArgs("python-server-sdk", tt.packageManager) + args, pkg := InstallArgs("", "python-server-sdk", tt.packageManager) assert.Equal(t, tt.want, args) assert.Equal(t, "launchdarkly-server-sdk", pkg) }) @@ -107,7 +107,7 @@ func TestInstallArgs_Python_ResolvesAvailableTool(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { stubPath(t, tt.available...) - args, _ := InstallArgs("python-server-sdk", "") + args, _ := InstallArgs("", "python-server-sdk", "") assert.Equal(t, tt.want, args) }) } @@ -186,7 +186,7 @@ func TestInstall_PresentToolRuns(t *testing.T) { } func TestInstallArgs_Go(t *testing.T) { - args, pkg := InstallArgs("go-server-sdk", "") + args, pkg := InstallArgs("", "go-server-sdk", "") require.NotEmpty(t, args) assert.Equal(t, "go", args[0]) assert.Equal(t, "get", args[1]) @@ -194,7 +194,7 @@ func TestInstallArgs_Go(t *testing.T) { } func TestInstallArgs_Ruby(t *testing.T) { - args, pkg := InstallArgs("ruby-server-sdk", "") + args, pkg := InstallArgs("", "ruby-server-sdk", "") require.NotEmpty(t, args) assert.Equal(t, "gem", args[0]) assert.Equal(t, "launchdarkly-server-sdk", pkg) @@ -203,14 +203,14 @@ func TestInstallArgs_Ruby(t *testing.T) { // A Gemfile means Bundler owns the project's gems, so the SDK must be added to the // Gemfile; `gem install` would leave the app unable to require it under bundler. func TestInstallArgs_Ruby_Bundler(t *testing.T) { - args, pkg := InstallArgs("ruby-server-sdk", "bundle") + args, pkg := InstallArgs("", "ruby-server-sdk", "bundle") assert.Equal(t, []string{"bundle", "add", "launchdarkly-server-sdk"}, args) assert.Equal(t, "launchdarkly-server-sdk", pkg) } func TestInstallArgs_Android_BothSpellings(t *testing.T) { for _, id := range []string{"android", "android-client-sdk"} { - args, pkg := InstallArgs(id, "gradle") + args, pkg := InstallArgs("", id, "gradle") assert.Nil(t, args, "Android has no automated install command") assert.Equal(t, "com.launchdarkly:launchdarkly-android-client-sdk", pkg) assert.True(t, RequiresManualInstall(id)) @@ -218,7 +218,7 @@ func TestInstallArgs_Android_BothSpellings(t *testing.T) { } func TestInstallArgs_Dotnet(t *testing.T) { - args, pkg := InstallArgs("dotnet-server-sdk", "") + args, pkg := InstallArgs("", "dotnet-server-sdk", "") require.NotEmpty(t, args) assert.Equal(t, "dotnet", args[0]) assert.Equal(t, "LaunchDarkly.ServerSdk", pkg) @@ -238,7 +238,7 @@ func TestInstallArgs_ManualSDKs(t *testing.T) { } for _, tt := range tests { t.Run(tt.sdkID, func(t *testing.T) { - args, pkg := InstallArgs(tt.sdkID, "") + args, pkg := InstallArgs("", tt.sdkID, "") assert.Nil(t, args, "expected nil args for manual SDK %s", tt.sdkID) assert.Equal(t, tt.wantPkg, pkg) }) @@ -512,7 +512,7 @@ func TestInstallArgs_PackageMatchesTemplateImport(t *testing.T) { } for _, tt := range tests { t.Run(tt.sdkID, func(t *testing.T) { - _, pkg := InstallArgs(tt.sdkID, "npm") + _, pkg := InstallArgs("", tt.sdkID, "npm") assert.Equal(t, tt.wantImport, pkg) rendered, err := RenderTemplate(tt.sdkID, InitConfig{}) @@ -527,3 +527,109 @@ func TestInstallArgs_PackageMatchesTemplateImport(t *testing.T) { }) } } + +// stubVirtualEnv controls what looks like an active virtualenv, so the suite is not +// affected by the environment it happens to run in. +func stubVirtualEnv(t *testing.T, dir string) { + t.Helper() + original := virtualEnv + virtualEnv = func() string { return dir } + t.Cleanup(func() { virtualEnv = original }) +} + +// fakeVenv creates the pip executable a virtualenv would have. +func fakeVenv(t *testing.T, root string) string { + t.Helper() + require.NoError(t, os.MkdirAll(filepath.Join(root, "bin"), 0755)) + pip := filepath.Join(root, "bin", "pip") + require.NoError(t, os.WriteFile(pip, []byte("#!/bin/sh\n"), 0700)) + return pip +} + +// A virtualenv is where a project's dependencies belong, and on a PEP 668 +// interpreter it is the only place pip can write at all. +func TestInstallArgs_Python_PrefersProjectVirtualenv(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") // a system pip exists and must still lose + dir := t.TempDir() + pip := fakeVenv(t, filepath.Join(dir, ".venv")) + + args, _ := InstallArgs(dir, "python-server-sdk", "") + + assert.Equal(t, []string{pip, "install", "launchdarkly-server-sdk"}, args) +} + +func TestInstallArgs_Python_AcceptsVenvDirectoryName(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t) + dir := t.TempDir() + pip := fakeVenv(t, filepath.Join(dir, "venv")) + + args, _ := InstallArgs(dir, "python-server-sdk", "") + + assert.Equal(t, []string{pip, "install", "launchdarkly-server-sdk"}, args) +} + +func TestInstallArgs_Python_PrefersActiveVirtualenvOverProjectOne(t *testing.T) { + stubPath(t, "pip3") + active := t.TempDir() + activePip := fakeVenv(t, active) + stubVirtualEnv(t, active) + project := t.TempDir() + fakeVenv(t, filepath.Join(project, ".venv")) + + args, _ := InstallArgs(project, "python-server-sdk", "") + + assert.Equal(t, activePip, args[0], "the environment the user activated wins") +} + +// Without a project directory there is nothing to search, and a relative lookup +// would reach into whatever directory the process happens to be in. +func TestInstallArgs_Python_NoDirDoesNotSearchRelatively(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + + args, _ := InstallArgs("", "python-server-sdk", "") + + assert.Equal(t, []string{"pip3", "install", "launchdarkly-server-sdk"}, args) +} + +// PEP 668: Homebrew and most current distros mark their Python as OS-managed, and +// pip refuses to write into it. The raw refusal tells the user nothing actionable. +func TestInstall_ExternallyManagedEnvironment_ExplainsVirtualenv(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + return []byte("error: externally-managed-environment\n\n× This environment is externally managed"), + errors.New("exit status 1") + }, + } + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err, "a recoverable refusal must not dead-end the flow") + assert.True(t, result.Failed) + assert.Contains(t, result.FailureReason, "managed by your operating system") + assert.Contains(t, result.FailureReason, "python3 -m venv .venv") + assert.Contains(t, result.FailureReason, dir) + // Forcing past the refusal would risk breaking the OS's own Python. + assert.NotContains(t, result.FailureReason, "break-system-packages") +} + +// Any other install failure keeps its existing error path. +func TestInstall_OtherFailuresStillError(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "npm") + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + return []byte("npm ERR! network timeout"), errors.New("exit status 1") + }, + } + + _, err := installer.Install(t.TempDir(), &DetectResult{SDKID: "node-server", PackageManager: "npm"}) + + require.Error(t, err) + assert.Contains(t, err.Error(), "network timeout") +} From 03d06a3fc6f55c1f111582d8f9f54857f54f4bfd Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Tue, 18 Aug 2026 11:28:31 -0400 Subject: [PATCH 05/10] test(setup): neutralise the ambient virtualenv for the package pipInstallCmd prefers VIRTUAL_ENV over anything on PATH, so running the suite inside an activated environment resolved install commands to that environment's pip and failed assertions about pip and pip3. Tests that want an active virtualenv opt in with stubVirtualEnv. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/main_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 internal/setup/main_test.go diff --git a/internal/setup/main_test.go b/internal/setup/main_test.go new file mode 100644 index 00000000..7aa91c94 --- /dev/null +++ b/internal/setup/main_test.go @@ -0,0 +1,16 @@ +package setup + +import ( + "os" + "testing" +) + +// TestMain neutralises the ambient virtualenv for the whole package. pipInstallCmd +// prefers VIRTUAL_ENV over anything on PATH, so a developer running the suite +// inside an activated environment would otherwise see install commands resolve to +// that environment's pip and assertions about pip/pip3 fail. Tests that care about +// an active virtualenv opt in with stubVirtualEnv. +func TestMain(m *testing.M) { + virtualEnv = func() string { return "" } + os.Exit(m.Run()) +} From ef23622a6338f9bdce056d054a6d08528786f341 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Wed, 19 Aug 2026 11:46:59 -0400 Subject: [PATCH 06/10] fix(setup): resolve the virtualenv pip to an absolute path The install runs with its working directory set to the project, and a relative executable path is resolved after that change, so a relative project directory was applied twice: "app/.venv/bin/pip" run in "app" was looked for at "app/app/.venv/bin/pip" and the install failed even though the venv was found. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/installer.go | 12 +++++++++- internal/setup/installer_test.go | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/internal/setup/installer.go b/internal/setup/installer.go index 35fb7bc2..e3ff52eb 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -330,9 +330,19 @@ func venvPip(dir string) string { for _, root := range roots { for _, rel := range []string{filepath.Join("bin", "pip"), filepath.Join("Scripts", "pip.exe")} { candidate := filepath.Join(root, rel) - if info, err := os.Stat(candidate); err == nil && !info.IsDir() { + info, err := os.Stat(candidate) + if err != nil || info.IsDir() { + continue + } + // Absolute, because the command runs with its working directory set to + // dir: a relative executable path is resolved after that change, so + // "app/.venv/bin/pip" run in "app" would be looked for at + // "app/app/.venv/bin/pip" and the install would fail with the venv found. + abs, err := filepath.Abs(candidate) + if err != nil { return candidate } + return abs } } return "" diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index db47b2e4..73f713ca 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -633,3 +633,42 @@ func TestInstall_OtherFailuresStillError(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "network timeout") } + +// The install runs with its working directory set to the project, and a relative +// executable path is resolved after that change — so a relative project dir would +// have the dir applied twice and the install would fail with the venv found. +func TestInstallArgs_Python_VenvPathIsAbsoluteForARelativeDir(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + parent := t.TempDir() + fakeVenv(t, filepath.Join(parent, "app", ".venv")) + chdirTo(t, parent) + + args, _ := InstallArgs("app", "python-server-sdk", "") + + require.NotEmpty(t, args) + assert.True(t, filepath.IsAbs(args[0]), + "a relative pip path is resolved against the command's working directory: %s", args[0]) + assert.FileExists(t, args[0]) +} + +// An absolute project dir must be left as it is. +func TestInstallArgs_Python_VenvPathKeepsAbsoluteDir(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + pip := fakeVenv(t, filepath.Join(dir, ".venv")) + + args, _ := InstallArgs(dir, "python-server-sdk", "") + + assert.Equal(t, pip, args[0]) +} + +// chdirTo moves into dir for the duration of the test. +func chdirTo(t *testing.T, dir string) { + t.Helper() + original, err := os.Getwd() + require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) + t.Cleanup(func() { _ = os.Chdir(original) }) +} From 22d6d61433cdc7d769f285838c4498f27d047526 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Wed, 19 Aug 2026 13:35:52 -0400 Subject: [PATCH 07/10] fix(setup): report a virtualenv that has no pip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `uv venv` creates a virtualenv without pip, which was read as no virtualenv at all: setup then reached for a pip on PATH, installing outside the project the user set up — or being refused by PEP 668 and advising them to create the virtualenv already sitting there. That case is now named, with how to install into it. A PEP 668 refusal no longer carries the command that refused. The done screen offers a non-empty command as "install it yourself with", which contradicted the reason telling them not to run it. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/installer.go | 103 +++++++++++++++++++++++++------ internal/setup/installer_test.go | 68 ++++++++++++++++++++ 2 files changed, 152 insertions(+), 19 deletions(-) diff --git a/internal/setup/installer.go b/internal/setup/installer.go index e3ff52eb..d94d3459 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -95,6 +95,17 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install }, nil } + // A virtualenv we cannot install into is a clearer thing to report than whatever + // the pip outside it would do. + if reason := pipLessVenvReason(dir, pkg, args); reason != "" { + return &InstallResult{ + SDKID: detection.SDKID, + Package: pkg, + Failed: true, + FailureReason: reason, + }, nil + } + // Confirm the tool exists before shelling out, so a missing package manager // warns with what to install instead of surfacing an exec "not found" error. // We never install the tool ourselves. @@ -129,10 +140,11 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install command := strings.Join(args, " ") if err != nil { if reason := externallyManagedReason(dir, out); reason != "" { + // No Command: the reason says not to run this pip, and the done screen + // offers a non-empty Command as "install it yourself with". return &InstallResult{ SDKID: detection.SDKID, Package: pkg, - Command: command, Failed: true, FailureReason: reason, }, nil @@ -197,6 +209,46 @@ func externallyManagedReason(dir string, out []byte) string { ) } +// pipLessVenv returns a virtualenv in dir, or the active one, that exists but has no +// pip. `uv venv` creates exactly this unless asked to seed one, so falling through +// to a pip on PATH would install outside the project the user set up — or be refused +// by PEP 668 and advise creating the very virtualenv already sitting there. +func pipLessVenv(dir string) string { + for _, root := range venvRoots(dir) { + if venvPipIn(root) != "" { + continue + } + // pyvenv.cfg is what marks a directory as a virtualenv. + if _, err := os.Stat(filepath.Join(root, "pyvenv.cfg")); err == nil { + return root + } + } + return "" +} + +// pipLessVenvReason explains that the project's virtualenv cannot be installed into, +// but only when the command would otherwise reach for a pip outside it. +func pipLessVenvReason(dir, pkg string, args []string) string { + if len(args) == 0 || filepath.IsAbs(args[0]) { + return "" // already pointed at a virtualenv's own pip + } + switch filepath.Base(args[0]) { + case "pip", "pip3": + default: + return "" // poetry, uv and pipenv manage their own environment + } + root := pipLessVenv(dir) + if root == "" { + return "" + } + return fmt.Sprintf( + "the virtual environment at %s has no pip, which is how `uv venv` creates one. "+ + "Install into it with `uv pip install %s`, or recreate it with "+ + "`python3 -m venv .venv`, then run setup again.", + root, pkg, + ) +} + // installHints maps a package-manager executable to how the user can get it. var installHints = map[string]string{ "pip": "install Python from https://www.python.org/downloads or your package manager", @@ -318,32 +370,45 @@ var virtualEnv = func() string { return os.Getenv("VIRTUAL_ENV") } // project's dependencies belong, and it is the only place pip can write on a // PEP 668 interpreter, so it is preferred over whatever pip is on PATH. func venvPip(dir string) string { + for _, root := range venvRoots(dir) { + if pip := venvPipIn(root); pip != "" { + return pip + } + } + return "" +} + +// venvRoots lists the virtualenvs to consider for dir, the active one first. An +// empty dir means the caller has no project in mind, so relative lookups would +// search whatever directory the process happens to be in. +func venvRoots(dir string) []string { var roots []string if active := virtualEnv(); active != "" { roots = append(roots, active) } - // An empty dir means the caller has no project in mind, so relative lookups - // would search whatever directory the process happens to be in. if dir != "" { roots = append(roots, filepath.Join(dir, ".venv"), filepath.Join(dir, "venv")) } - for _, root := range roots { - for _, rel := range []string{filepath.Join("bin", "pip"), filepath.Join("Scripts", "pip.exe")} { - candidate := filepath.Join(root, rel) - info, err := os.Stat(candidate) - if err != nil || info.IsDir() { - continue - } - // Absolute, because the command runs with its working directory set to - // dir: a relative executable path is resolved after that change, so - // "app/.venv/bin/pip" run in "app" would be looked for at - // "app/app/.venv/bin/pip" and the install would fail with the venv found. - abs, err := filepath.Abs(candidate) - if err != nil { - return candidate - } - return abs + return roots +} + +// venvPipIn returns root's own pip as an absolute path, or an empty string when the +// virtualenv has none. The path has to be absolute because the command runs with its +// working directory set to the project: a relative executable is resolved after that +// change, so "app/.venv/bin/pip" run in "app" would be looked for at +// "app/app/.venv/bin/pip" and the install would fail with the virtualenv found. +func venvPipIn(root string) string { + for _, rel := range []string{filepath.Join("bin", "pip"), filepath.Join("Scripts", "pip.exe")} { + candidate := filepath.Join(root, rel) + info, err := os.Stat(candidate) + if err != nil || info.IsDir() { + continue + } + abs, err := filepath.Abs(candidate) + if err != nil { + return candidate } + return abs } return "" } diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 73f713ca..5290e879 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -672,3 +672,71 @@ func chdirTo(t *testing.T, dir string) { require.NoError(t, os.Chdir(dir)) t.Cleanup(func() { _ = os.Chdir(original) }) } + +// `uv venv` creates a virtualenv with no pip in it. Falling through to a pip on +// PATH would install outside the project, or be refused by PEP 668 and then advise +// creating the virtualenv already sitting there. +func TestInstall_VenvWithoutPip_ReportsItRatherThanUsingSystemPip(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + root := filepath.Join(dir, ".venv") + require.NoError(t, os.MkdirAll(filepath.Join(root, "bin"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "pyvenv.cfg"), []byte("home = /usr\n"), 0600)) + + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + t.Fatal("must not install with a pip outside the project's virtualenv") + return nil, nil + }, + } + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.True(t, result.Failed) + assert.Contains(t, result.FailureReason, "has no pip") + assert.Contains(t, result.FailureReason, root) + assert.Contains(t, result.FailureReason, "uv pip install launchdarkly-server-sdk") + // Do not offer a command that would install outside the virtualenv. + assert.Empty(t, result.Command) +} + +// A manager that owns its own environment is unaffected by a pip-less virtualenv. +func TestInstall_VenvWithoutPip_LeavesUvAlone(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "uv") + dir := t.TempDir() + root := filepath.Join(dir, ".venv") + require.NoError(t, os.MkdirAll(filepath.Join(root, "bin"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "pyvenv.cfg"), []byte("home = /usr\n"), 0600)) + + var ran []string + installer := PackageInstaller{ + run: func(_ string, args []string) ([]byte, error) { ran = args; return nil, nil }, + } + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk", PackageManager: "uv"}) + + require.NoError(t, err) + assert.True(t, result.Success) + assert.Equal(t, []string{"uv", "add", "launchdarkly-server-sdk"}, ran) +} + +// The PEP 668 reason says not to run that pip, so the done screen must not offer it +// back as "install it yourself with". +func TestInstall_ExternallyManaged_OffersNoCommand(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + installer := PackageInstaller{ + run: func(string, []string) ([]byte, error) { + return []byte("error: externally-managed-environment"), errors.New("exit status 1") + }, + } + + result, err := installer.Install(t.TempDir(), &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.True(t, result.Failed) + assert.Empty(t, result.Command, "the screen would offer the pip the reason says not to run") +} From 65849924f7d956709513d36ae1c4478c45d7e460 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Wed, 19 Aug 2026 14:15:29 -0400 Subject: [PATCH 08/10] fix(setup): name the project virtualenv in the previewed pip command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pip-less virtualenv guard sat only in Install, so the plan screen, --dry-run and the picker still previewed a pip from PATH that Install would refuse to run — and a command shown there is one a reader may run by hand, installing outside the project or hitting a raw PEP 668 error. The command now names the virtualenv's own pip whenever the project has one, present or not, so every surface and the runner agree. Install still explains why an unseeded virtualenv cannot be installed into. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/installer.go | 83 +++++++++++++++----------------- internal/setup/installer_test.go | 42 ++++++++++++++++ 2 files changed, 81 insertions(+), 44 deletions(-) diff --git a/internal/setup/installer.go b/internal/setup/installer.go index d94d3459..9482409e 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -209,43 +209,19 @@ func externallyManagedReason(dir string, out []byte) string { ) } -// pipLessVenv returns a virtualenv in dir, or the active one, that exists but has no -// pip. `uv venv` creates exactly this unless asked to seed one, so falling through -// to a pip on PATH would install outside the project the user set up — or be refused -// by PEP 668 and advise creating the very virtualenv already sitting there. -func pipLessVenv(dir string) string { - for _, root := range venvRoots(dir) { - if venvPipIn(root) != "" { - continue - } - // pyvenv.cfg is what marks a directory as a virtualenv. - if _, err := os.Stat(filepath.Join(root, "pyvenv.cfg")); err == nil { - return root - } - } - return "" -} - -// pipLessVenvReason explains that the project's virtualenv cannot be installed into, -// but only when the command would otherwise reach for a pip outside it. +// pipLessVenvReason explains that the project's virtualenv cannot be installed into. +// It fires only when the command is that virtualenv's own pip and the pip is not +// there — uv, poetry and pipenv own their environments and never reach this. func pipLessVenvReason(dir, pkg string, args []string) string { - if len(args) == 0 || filepath.IsAbs(args[0]) { - return "" // already pointed at a virtualenv's own pip - } - switch filepath.Base(args[0]) { - case "pip", "pip3": - default: - return "" // poetry, uv and pipenv manage their own environment - } - root := pipLessVenv(dir) - if root == "" { + target, exists := venvPipTarget(dir) + if target == "" || exists || len(args) == 0 || args[0] != target { return "" } return fmt.Sprintf( "the virtual environment at %s has no pip, which is how `uv venv` creates one. "+ "Install into it with `uv pip install %s`, or recreate it with "+ "`python3 -m venv .venv`, then run setup again.", - root, pkg, + filepath.Dir(filepath.Dir(target)), pkg, ) } @@ -365,19 +341,6 @@ func pythonInstallCmd(dir, pm, pkg string) []string { // by the environment the suite happens to run in. var virtualEnv = func() string { return os.Getenv("VIRTUAL_ENV") } -// venvPip returns the pip belonging to the active virtualenv, or to one sitting in -// the project, and an empty string when there is none. A virtualenv is where a -// project's dependencies belong, and it is the only place pip can write on a -// PEP 668 interpreter, so it is preferred over whatever pip is on PATH. -func venvPip(dir string) string { - for _, root := range venvRoots(dir) { - if pip := venvPipIn(root); pip != "" { - return pip - } - } - return "" -} - // venvRoots lists the virtualenvs to consider for dir, the active one first. An // empty dir means the caller has no project in mind, so relative lookups would // search whatever directory the process happens to be in. @@ -389,6 +352,13 @@ func venvRoots(dir string) []string { if dir != "" { roots = append(roots, filepath.Join(dir, ".venv"), filepath.Join(dir, "venv")) } + // Absolute, so a root can be compared against the command we build from it and + // so the path we show names the same place whatever the working directory is. + for i, root := range roots { + if abs, err := filepath.Abs(root); err == nil { + roots[i] = abs + } + } return roots } @@ -423,7 +393,11 @@ func venvPipIn(root string) string { // returned so the plan screen has something to show, and Install's pre-flight // check warns instead of running anything. func pipInstallCmd(dir, pkg string) []string { - if pip := venvPip(dir); pip != "" { + // A virtualenv without pip still names the environment the project set up, and + // naming it is more use than a pip from PATH that Install will refuse to run: + // the plan screen, --dry-run and the picker all read this, and a command shown + // there is one a reader may run by hand. + if pip, _ := venvPipTarget(dir); pip != "" { return []string{pip, "install", pkg} } for _, bin := range []string{"pip3", "pip"} { @@ -434,6 +408,27 @@ func pipInstallCmd(dir, pkg string) []string { return []string{"pip", "install", pkg} } +// venvPipTarget returns the pip belonging to the project's virtualenv, and whether it +// is actually there. An empty path means there is no virtualenv to install into. +func venvPipTarget(dir string) (path string, exists bool) { + for _, root := range venvRoots(dir) { + if pip := venvPipIn(root); pip != "" { + return pip, true + } + if isVirtualEnv(root) { + return filepath.Join(root, "bin", "pip"), false + } + } + return "", false +} + +// isVirtualEnv reports whether root is a virtualenv. pyvenv.cfg is what marks one, +// and it is there whether or not the environment was seeded with pip. +func isVirtualEnv(root string) bool { + _, err := os.Stat(filepath.Join(root, "pyvenv.cfg")) + return err == nil +} + // nodeInstallCmd returns the install command arguments for a Node.js package manager. func nodeInstallCmd(pm, pkg string) []string { switch pm { diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 5290e879..3d0b9973 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -740,3 +740,45 @@ func TestInstall_ExternallyManaged_OffersNoCommand(t *testing.T) { assert.True(t, result.Failed) assert.Empty(t, result.Command, "the screen would offer the pip the reason says not to run") } + +// The plan screen, --dry-run and the picker all read InstallArgs, and a command shown +// there is one a reader may run by hand. With a pip-less virtualenv present, none of +// them may name a pip from PATH: running it installs outside the project. +func TestInstallArgs_Python_PipLessVenvNeverPreviewsSystemPip(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3", "pip") + dir := t.TempDir() + root := filepath.Join(dir, ".venv") + require.NoError(t, os.MkdirAll(filepath.Join(root, "bin"), 0755)) + require.NoError(t, os.WriteFile(filepath.Join(root, "pyvenv.cfg"), []byte("home = /usr\n"), 0600)) + + args, _ := InstallArgs(dir, "python-server-sdk", "") + + require.NotEmpty(t, args) + assert.Equal(t, filepath.Join(root, "bin", "pip"), args[0], + "the preview names a pip outside the project's virtualenv") + assert.NotContains(t, filepath.Base(args[0]), "pip3") +} + +// A virtualenv that does have pip is still used, and the preview matches. +func TestInstallArgs_Python_SeededVenvPreviewMatchesRun(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + pip := fakeVenv(t, filepath.Join(dir, ".venv")) + require.NoError(t, os.WriteFile(filepath.Join(dir, ".venv", "pyvenv.cfg"), []byte("home = /usr\n"), 0600)) + + args, _ := InstallArgs(dir, "python-server-sdk", "") + + assert.Equal(t, []string{pip, "install", "launchdarkly-server-sdk"}, args) +} + +// With no virtualenv at all, a pip from PATH is still the right answer. +func TestInstallArgs_Python_NoVenvStillUsesPath(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + + args, _ := InstallArgs(t.TempDir(), "python-server-sdk", "") + + assert.Equal(t, []string{"pip3", "install", "launchdarkly-server-sdk"}, args) +} From ba3d7cccd85171b605811e296b459a3a998d2638 Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Wed, 19 Aug 2026 17:09:38 -0400 Subject: [PATCH 09/10] fix(setup): use the platform's virtualenv layout and flag an unrecorded dependency A virtualenv with no pip was always named bin/pip, so on Windows the plan and the previewed command pointed at a layout that environment never uses. The platform's own layout is named first, and both are still considered. A bare pip install also leaves the project's manifest untouched, so a fresh checkout and CI miss the SDK. poetry, uv and pipenv record it themselves and Ruby has `bundle add`; pip has no equivalent, and editing someone's manifest unasked is not something setup does, so a successful install now says what is still missing. Co-Authored-By: Claude Opus 5 (1M context) --- cmd/setup/install.go | 3 ++ cmd/setup/view.go | 5 +++ internal/setup/installer.go | 70 ++++++++++++++++++++++++++++---- internal/setup/installer_test.go | 67 ++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 8 deletions(-) diff --git a/cmd/setup/install.go b/cmd/setup/install.go index 68a3bb6f..42b85465 100644 --- a/cmd/setup/install.go +++ b/cmd/setup/install.go @@ -95,6 +95,9 @@ func runInstall(svc setup.Service) func(*cobra.Command, []string) error { return nil } fmt.Fprintf(cmd.OutOrStdout(), "Success: %t\n", result.Success) + if result.Warning != "" { + fmt.Fprintf(cmd.OutOrStdout(), "Warning: %s\n", result.Warning) + } switch { case result.FailureReason != "": fmt.Fprintf(cmd.OutOrStdout(), "Reason: %s\n", result.FailureReason) diff --git a/cmd/setup/view.go b/cmd/setup/view.go index 504b1951..0a359b67 100644 --- a/cmd/setup/view.go +++ b/cmd/setup/view.go @@ -133,10 +133,15 @@ func (m wizardModel) View() string { } if m.verifyResult != nil && m.verifyResult.Active && m.detectResult != nil { appHost := strings.TrimRight(m.auth.BaseURI, "/") + warning := "" + if m.installResult != nil && m.installResult.Warning != "" { + warning = "\n" + m.wrap("Note: "+m.installResult.Warning) + "\n" + } return titleStyle.Render("Setup complete!") + "\n\n" + fmt.Sprintf("Your %s SDK is connected to LaunchDarkly.\n", m.detectResult.SDKID) + fmt.Sprintf("Flag %q is ready to use.\n\n", m.flagKey) + fmt.Sprintf("You can now toggle your flag at %s/projects/%s/flags/%s/targeting?env=%s\n", appHost, m.selectedProject, m.flagKey, m.selectedEnv) + + warning + quitHint } return titleStyle.Render("Verification timed out") + "\n\n" + diff --git a/internal/setup/installer.go b/internal/setup/installer.go index 9482409e..3315c8dd 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "sort" "strings" ) @@ -24,7 +25,10 @@ type InstallResult struct { // FailureReason carries the underlying error when Failed is true, so callers // can tell the user why the automatic install did not run. FailureReason string `json:"failure_reason,omitempty"` - Success bool `json:"success"` + // Warning carries something the user has to act on even though the install + // worked, so success is not reported as though nothing were left to do. + Warning string `json:"warning,omitempty"` + Success bool `json:"success"` } // RequiresManualInstall reports whether the SDK has no automated package-manager @@ -155,6 +159,7 @@ func (p PackageInstaller) Install(dir string, detection *DetectResult) (*Install SDKID: detection.SDKID, Package: pkg, Command: command, + Warning: unrecordedDependencyWarning(dir, pkg, args), Success: true, }, nil } @@ -225,6 +230,36 @@ func pipLessVenvReason(dir, pkg string, args []string) string { ) } +// unrecordedDependencyWarning reports that a bare pip install leaves the project's +// manifest untouched. poetry, uv, pipenv and pdm record the dependency themselves, +// and Ruby gets `bundle add` for the same reason, but pip has no equivalent command: +// editing someone's manifest is not something setup does unasked, so it says what is +// missing instead of writing the file. +func unrecordedDependencyWarning(dir, pkg string, args []string) string { + if len(args) == 0 || filepath.Base(args[0]) != "pip" && filepath.Base(args[0]) != "pip3" && + filepath.Base(args[0]) != "pip.exe" { + return "" + } + manifest := "" + for _, name := range []string{"requirements.txt", "requirements/base.txt"} { + if _, err := os.Stat(filepath.Join(dir, name)); err == nil { + manifest = name + break + } + } + if manifest == "" { + return "" + } + if b, err := os.ReadFile(filepath.Join(dir, manifest)); err == nil && bytes.Contains(b, []byte(pkg)) { + return "" // already recorded + } + return fmt.Sprintf( + "pip installed %s but did not record it in %s, so a fresh checkout and CI will not have it. "+ + "Add a line for %s to %s.", + pkg, manifest, pkg, manifest, + ) +} + // installHints maps a package-manager executable to how the user can get it. var installHints = map[string]string{ "pip": "install Python from https://www.python.org/downloads or your package manager", @@ -368,21 +403,39 @@ func venvRoots(dir string) []string { // change, so "app/.venv/bin/pip" run in "app" would be looked for at // "app/app/.venv/bin/pip" and the install would fail with the virtualenv found. func venvPipIn(root string) string { - for _, rel := range []string{filepath.Join("bin", "pip"), filepath.Join("Scripts", "pip.exe")} { + for _, rel := range venvPipLayouts() { candidate := filepath.Join(root, rel) info, err := os.Stat(candidate) if err != nil || info.IsDir() { continue } - abs, err := filepath.Abs(candidate) - if err != nil { - return candidate - } - return abs + return absOrAsGiven(candidate) } return "" } +// venvPipLayouts lists where a virtualenv keeps pip, this platform's layout first. +// Windows uses Scripts\pip.exe; everything else uses bin/pip. Naming the wrong one +// would point the plan and the previewed command at a path the environment on this +// machine never has. +func venvPipLayouts() []string { + unix := filepath.Join("bin", "pip") + windows := filepath.Join("Scripts", "pip.exe") + if runtime.GOOS == "windows" { + return []string{windows, unix} + } + return []string{unix, windows} +} + +// absOrAsGiven makes a path absolute, falling back to the path itself when the +// working directory cannot be read. +func absOrAsGiven(path string) string { + if abs, err := filepath.Abs(path); err == nil { + return abs + } + return path +} + // pipInstallCmd returns the pip install command. A virtualenv's pip wins, then // whichever of pip3 and pip is on PATH: recent macOS and Homebrew installs ship // pip3 with no bare pip, so a hardcoded `pip` fails outright on a common box. @@ -416,7 +469,8 @@ func venvPipTarget(dir string) (path string, exists bool) { return pip, true } if isVirtualEnv(root) { - return filepath.Join(root, "bin", "pip"), false + // No pip to find, so name where this platform would keep one. + return absOrAsGiven(filepath.Join(root, venvPipLayouts()[0])), false } } return "", false diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 3d0b9973..50b399b9 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -5,6 +5,7 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -782,3 +783,69 @@ func TestInstallArgs_Python_NoVenvStillUsesPath(t *testing.T) { assert.Equal(t, []string{"pip3", "install", "launchdarkly-server-sdk"}, args) } + +// A bare pip install leaves requirements.txt untouched, so a fresh checkout and CI +// do not get the SDK. poetry, uv, pipenv and pdm record it themselves and Ruby gets +// `bundle add`; pip has no equivalent, and editing someone's manifest unasked is not +// something setup does — so it says what is missing. +func TestInstall_PipLeavesManifestUnrecorded_Warns(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "requirements.txt"), []byte("flask\n"), 0600)) + installer := PackageInstaller{run: func(string, []string) ([]byte, error) { return nil, nil }} + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.True(t, result.Success) + assert.Contains(t, result.Warning, "did not record it in requirements.txt") + assert.Contains(t, result.Warning, "launchdarkly-server-sdk") +} + +func TestInstall_PipManifestAlreadyRecorded_NoWarning(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "requirements.txt"), + []byte("flask\nlaunchdarkly-server-sdk\n"), 0600)) + installer := PackageInstaller{run: func(string, []string) ([]byte, error) { return nil, nil }} + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.Empty(t, result.Warning) +} + +// The managers that record the dependency themselves must not be nagged about it. +func TestInstall_ManagersThatRecordDependencies_NoWarning(t *testing.T) { + // pdm arrives with the confidence work; these are the managers this build drives. + for _, pm := range []string{"uv", "poetry", "pipenv"} { + t.Run(pm, func(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, pm) + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "requirements.txt"), []byte("flask\n"), 0600)) + installer := PackageInstaller{run: func(string, []string) ([]byte, error) { return nil, nil }} + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk", PackageManager: pm}) + + require.NoError(t, err) + assert.True(t, result.Success) + assert.Empty(t, result.Warning) + }) + } +} + +// Windows keeps a virtualenv's pip in Scripts, so naming bin would point the plan at +// a path the environment never has. +func TestVenvPipLayouts_PlatformFirst(t *testing.T) { + layouts := venvPipLayouts() + require.Len(t, layouts, 2) + if runtime.GOOS == "windows" { + assert.Contains(t, layouts[0], "Scripts") + } else { + assert.Contains(t, layouts[0], "bin") + } + assert.NotEqual(t, layouts[0], layouts[1], "both layouts are still considered") +} From f3b25d17166b592f4b149561fba5e3b225d9387e Mon Sep 17 00:00:00 2001 From: Francisco Fantl Date: Thu, 20 Aug 2026 11:11:06 -0400 Subject: [PATCH 10/10] fix(setup): match the SDK by whole name when checking the manifest A substring match read a related pin such as launchdarkly-server-sdk-otel as the SDK itself, so the warning stayed quiet while the project still lacked the dependency. The whole-name matcher the install-skipping check already uses does the job. Co-Authored-By: Claude Opus 5 (1M context) --- internal/setup/installer.go | 6 ++++-- internal/setup/installer_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/internal/setup/installer.go b/internal/setup/installer.go index 3315c8dd..cab87917 100644 --- a/internal/setup/installer.go +++ b/internal/setup/installer.go @@ -250,8 +250,10 @@ func unrecordedDependencyWarning(dir, pkg string, args []string) string { if manifest == "" { return "" } - if b, err := os.ReadFile(filepath.Join(dir, manifest)); err == nil && bytes.Contains(b, []byte(pkg)) { - return "" // already recorded + // Whole-name matching, so a related pin such as launchdarkly-server-sdk-otel is + // not read as the SDK itself being recorded. + if fileMentionsPackage(filepath.Join(dir, manifest), pkg) { + return "" } return fmt.Sprintf( "pip installed %s but did not record it in %s, so a fresh checkout and CI will not have it. "+ diff --git a/internal/setup/installer_test.go b/internal/setup/installer_test.go index 50b399b9..62a194d1 100644 --- a/internal/setup/installer_test.go +++ b/internal/setup/installer_test.go @@ -849,3 +849,35 @@ func TestVenvPipLayouts_PlatformFirst(t *testing.T) { } assert.NotEqual(t, layouts[0], layouts[1], "both layouts are still considered") } + +// A related package pinned in the manifest is not the SDK. Reading it as one would +// suppress the warning while the project still lacks the dependency. +func TestInstall_RelatedPackagePinned_StillWarns(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "requirements.txt"), + []byte("flask\nlaunchdarkly-server-sdk-otel==1.2.0\n"), 0600)) + installer := PackageInstaller{run: func(string, []string) ([]byte, error) { return nil, nil }} + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.True(t, result.Success) + assert.Contains(t, result.Warning, "did not record it in requirements.txt") +} + +// A pinned version of the SDK itself does count as recorded. +func TestInstall_PinnedSdkVersion_NoWarning(t *testing.T) { + stubVirtualEnv(t, "") + stubPath(t, "pip3") + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "requirements.txt"), + []byte("flask\nlaunchdarkly-server-sdk==9.16.1\n"), 0600)) + installer := PackageInstaller{run: func(string, []string) ([]byte, error) { return nil, nil }} + + result, err := installer.Install(dir, &DetectResult{SDKID: "python-server-sdk"}) + + require.NoError(t, err) + assert.Empty(t, result.Warning) +}