From 86fa4a268f38cadd06588a7a099a1e32435f3401 Mon Sep 17 00:00:00 2001 From: "Calvin A. Allen" Date: Thu, 14 May 2026 10:32:01 -0400 Subject: [PATCH] fix(shim): discover install-time shims from disk, not static provider list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Install previously created shim files and shim-map cache entries from the provider's static Shims() declaration, regardless of whether those executables actually existed on disk for the version being installed. On Windows that created phantom python3 and pip shims for python-build- standalone — which ships only python.exe / pythonw.exe in the root and an empty Scripts/ placeholder — leaving users with shims that errored at invocation time with "secondary executable not found." Reshim's scan already handled this correctly by registering only executables it found on disk, so install and reshim disagreed about which shims existed. This change factors the per-version disk scan out of RehashWithCallback into DiscoverShimsForVersion and routes the node, python, and ruby provider createShims() through it. Install now registers exactly the executables that exist in bin/, root/, and Scripts/ — picking up pythonw on Windows, pip / pip3 / pip3.X after ensurepip succeeds, and nothing extra when it fails. --- src/internal/shim/manager.go | 82 +++++++++++----- src/internal/shim/manager_test.go | 141 +++++++++++++++++++++++++++ src/runtimes/node/provider_full.go | 10 +- src/runtimes/python/provider_full.go | 15 ++- src/runtimes/ruby/provider_full.go | 10 +- 5 files changed, 228 insertions(+), 30 deletions(-) diff --git a/src/internal/shim/manager.go b/src/internal/shim/manager.go index 591b7ed..083e522 100644 --- a/src/internal/shim/manager.go +++ b/src/internal/shim/manager.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime" + "sort" "github.com/CodingWithCalvin/dtvem.cli/src/internal/config" "github.com/CodingWithCalvin/dtvem.cli/src/internal/constants" @@ -282,33 +283,16 @@ func (m *Manager) RehashWithCallback(callback RehashCallback) (*RehashResult, er version := versionEntry.Name() versionDir := filepath.Join(runtimeVersionsDir, version) - // Scan the directories where runtime and package executables - // live. Anything found is recorded as a shim provided by this - // version. We deliberately do NOT pre-populate the provider's - // declared core shims (e.g. python3, pip3) without checking - // the filesystem: an embeddable Windows install may ship only - // python.exe, and asserting that 3.8.9 "provides pip" when it - // doesn't would corrupt the version-coverage data the shim - // uses to give users an informed error. - if execs, err := findExecutables(filepath.Join(versionDir, "bin")); err == nil { - for _, exec := range execs { - recordShim(exec, version) - } - } - - // On Windows, also check the root version directory for .cmd/.bat files - // and Scripts directory for Python packages - if runtime.GOOS == constants.OSWindows { - if execs, err := findExecutables(versionDir); err == nil { - for _, exec := range execs { - recordShim(exec, version) - } - } - if execs, err := findExecutables(filepath.Join(versionDir, "Scripts")); err == nil { - for _, exec := range execs { - recordShim(exec, version) - } - } + // Anything found on disk is recorded as a shim provided by + // this version. We deliberately do NOT pre-populate the + // provider's declared core shims (e.g. python3, pip3) + // without checking the filesystem: an embeddable Windows + // install may ship only python.exe, and asserting that + // 3.8.9 "provides pip" when it doesn't would corrupt the + // version-coverage data the shim uses to give users an + // informed error. + for _, exec := range DiscoverShimsForVersion(versionDir) { + recordShim(exec, version) } } } @@ -391,6 +375,50 @@ func RuntimeShims(runtimeName string) []string { return provider.Shims() } +// DiscoverShimsForVersion scans an installed runtime version's directory tree +// and returns the executable base names that should be shimmed. +// +// versionDir is the absolute path to a single version's install directory +// (e.g., ~/.dtvem/versions/python/3.14.2). The same directories `Rehash` +// walks are scanned here: `bin/` always, plus the root and `Scripts/` on +// Windows. Missing directories are not an error — they just contribute no +// names — so callers can use this on any layout without pre-checking. +// +// The intersection-with-provider-declarations is deliberately omitted: +// reshim registers every executable it finds (pip-installed tools, npm- +// installed binaries, version-suffixed names like pip3.14), and install +// should match that policy so reshim doesn't surprise the user by adding +// or removing shims that the install command didn't. +// +// Returns names sorted alphabetically so callers get stable output across +// runs (filesystem iteration order is not guaranteed). +func DiscoverShimsForVersion(versionDir string) []string { + seen := make(map[string]struct{}) + scan := func(dir string) { + execs, err := findExecutables(dir) + if err != nil { + return + } + for _, e := range execs { + seen[e] = struct{}{} + } + } + + scan(filepath.Join(versionDir, "bin")) + + if runtime.GOOS == constants.OSWindows { + scan(versionDir) + scan(filepath.Join(versionDir, "Scripts")) + } + + out := make([]string, 0, len(seen)) + for name := range seen { + out = append(out, name) + } + sort.Strings(out) + return out +} + // findExecutables scans a directory for executable files and returns their base names func findExecutables(dir string) ([]string, error) { entries, err := os.ReadDir(dir) diff --git a/src/internal/shim/manager_test.go b/src/internal/shim/manager_test.go index e2b818d..a9f3d38 100644 --- a/src/internal/shim/manager_test.go +++ b/src/internal/shim/manager_test.go @@ -438,6 +438,147 @@ func TestListShims_SkipsCmdFiles(t *testing.T) { } } +// writeExecutable writes a file with the executable bit set on Unix. The +// content is irrelevant — findExecutables only looks at extension (Windows) +// or the exec bit (Unix) — but a non-zero body keeps file readers happy. +func writeExecutable(t *testing.T, path string) { + t.Helper() + if err := os.WriteFile(path, []byte("x"), 0755); err != nil { + t.Fatalf("Failed to write %s: %v", path, err) + } +} + +// platformExeName returns name with the OS-appropriate executable extension. +// On Windows the .exe suffix is required for findExecutables to recognize +// the file; on Unix the bare name is used and the exec bit drives detection. +func platformExeName(name string) string { + if runtime.GOOS == constants.OSWindows { + return name + constants.ExtExe + } + return name +} + +func TestDiscoverShimsForVersion_EmptyDir(t *testing.T) { + versionDir := t.TempDir() + + got := DiscoverShimsForVersion(versionDir) + if len(got) != 0 { + t.Errorf("DiscoverShimsForVersion(empty) = %v, want []", got) + } +} + +func TestDiscoverShimsForVersion_MissingVersionDir(t *testing.T) { + // Caller may pass a path that doesn't exist (e.g., when called before + // the install has moved files into place). The helper must not panic + // or surface an error — it just returns no names. + got := DiscoverShimsForVersion(filepath.Join(t.TempDir(), "does-not-exist")) + if len(got) != 0 { + t.Errorf("DiscoverShimsForVersion(missing) = %v, want []", got) + } +} + +func TestDiscoverShimsForVersion_BinDir(t *testing.T) { + versionDir := t.TempDir() + binDir := filepath.Join(versionDir, "bin") + if err := os.MkdirAll(binDir, 0755); err != nil { + t.Fatalf("mkdir bin: %v", err) + } + + writeExecutable(t, filepath.Join(binDir, platformExeName("node"))) + writeExecutable(t, filepath.Join(binDir, platformExeName("npm"))) + + got := DiscoverShimsForVersion(versionDir) + want := []string{"node", "npm"} + if !reflect.DeepEqual(got, want) { + t.Errorf("DiscoverShimsForVersion(bin) = %v, want %v", got, want) + } +} + +// TestDiscoverShimsForVersion_PythonWindowsOnlyRoot models the python-build- +// standalone Windows tarball before ensurepip runs: python.exe and +// pythonw.exe live in the version root and Scripts/ is either absent or +// only contains the upstream .empty placeholder. The discover helper must +// not invent pip / python3 entries that don't exist on disk — that was +// the root cause of issue #269 where install-time shim creation used a +// static provider declaration instead of disk truth. +func TestDiscoverShimsForVersion_PythonWindowsOnlyRoot(t *testing.T) { + if runtime.GOOS != constants.OSWindows { + t.Skip("Windows-specific layout (root .exe files)") + } + + versionDir := t.TempDir() + writeExecutable(t, filepath.Join(versionDir, "python.exe")) + writeExecutable(t, filepath.Join(versionDir, "pythonw.exe")) + // Upstream ships an empty Scripts/.empty placeholder; recreate it + // here so the test reflects the exact layout users see and proves + // the helper does not surface the placeholder as a shim. + if err := os.MkdirAll(filepath.Join(versionDir, "Scripts"), 0755); err != nil { + t.Fatalf("mkdir Scripts: %v", err) + } + if err := os.WriteFile(filepath.Join(versionDir, "Scripts", ".empty"), nil, 0644); err != nil { + t.Fatalf("write .empty: %v", err) + } + + got := DiscoverShimsForVersion(versionDir) + want := []string{"python", "pythonw"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } +} + +// TestDiscoverShimsForVersion_PythonWindowsAfterEnsurepip models the +// post-ensurepip state: root has python.exe/pythonw.exe, Scripts/ has +// pip.exe, pip3.exe, and the versioned pip3.14.exe. All five should be +// discovered and the result should be sorted+deduplicated. +func TestDiscoverShimsForVersion_PythonWindowsAfterEnsurepip(t *testing.T) { + if runtime.GOOS != constants.OSWindows { + t.Skip("Windows-specific layout (Scripts/*.exe)") + } + + versionDir := t.TempDir() + writeExecutable(t, filepath.Join(versionDir, "python.exe")) + writeExecutable(t, filepath.Join(versionDir, "pythonw.exe")) + + scriptsDir := filepath.Join(versionDir, "Scripts") + if err := os.MkdirAll(scriptsDir, 0755); err != nil { + t.Fatalf("mkdir Scripts: %v", err) + } + writeExecutable(t, filepath.Join(scriptsDir, "pip.exe")) + writeExecutable(t, filepath.Join(scriptsDir, "pip3.exe")) + writeExecutable(t, filepath.Join(scriptsDir, "pip3.14.exe")) + + got := DiscoverShimsForVersion(versionDir) + want := []string{"pip", "pip3", "pip3.14", "python", "pythonw"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } +} + +// TestDiscoverShimsForVersion_DedupesAcrossDirs proves the helper unions +// names across bin/ root/ and Scripts/ rather than double-counting. This +// matters because some runtimes (e.g., Ruby on Windows) place the same +// command name in multiple search locations. +func TestDiscoverShimsForVersion_DedupesAcrossDirs(t *testing.T) { + if runtime.GOOS != constants.OSWindows { + t.Skip("Multi-dir Windows scan (root + Scripts)") + } + + versionDir := t.TempDir() + writeExecutable(t, filepath.Join(versionDir, "tool.exe")) + + scriptsDir := filepath.Join(versionDir, "Scripts") + if err := os.MkdirAll(scriptsDir, 0755); err != nil { + t.Fatalf("mkdir Scripts: %v", err) + } + writeExecutable(t, filepath.Join(scriptsDir, "tool.exe")) + + got := DiscoverShimsForVersion(versionDir) + want := []string{"tool"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } +} + func TestRuntimeShims_AllKnownRuntimes(t *testing.T) { // Verify all known runtimes have shim mappings knownRuntimes := []string{"python", "node", "ruby", "go"} diff --git a/src/runtimes/node/provider_full.go b/src/runtimes/node/provider_full.go index 5afcbb9..a2ad6f5 100644 --- a/src/runtimes/node/provider_full.go +++ b/src/runtimes/node/provider_full.go @@ -124,13 +124,21 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) { // than falling back to the provider registry. The version is recorded in the // cache so the shim can detect when an active runtime version is one that // does not provide a given executable. +// +// The shim list is derived from disk (the same scan reshim uses), not from +// the provider's static Shims() declaration, so install and reshim stay in +// sync and only register executables that actually exist for this version. func (p *Provider) createShims(version string) error { manager, err := shim.NewManager() if err != nil { return err } - shimNames := shim.RuntimeShims("node") + versionDir := config.RuntimeVersionPath("node", version) + shimNames := shim.DiscoverShimsForVersion(versionDir) + if len(shimNames) == 0 { + return fmt.Errorf("no executables found in %s", versionDir) + } return manager.CreateShimsForRuntime("node", version, shimNames) } diff --git a/src/runtimes/python/provider_full.go b/src/runtimes/python/provider_full.go index 256e7e8..835edae 100644 --- a/src/runtimes/python/provider_full.go +++ b/src/runtimes/python/provider_full.go @@ -181,13 +181,26 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) { // than falling back to the provider registry. The version is recorded in the // cache so the shim can detect when an active runtime version is one that // does not provide a given executable. +// +// The shim list is derived from disk (the same scan reshim uses), not from +// the provider's static Shims() declaration. That keeps install and reshim +// honest about which executables actually exist: on Windows the upstream +// python-build-standalone tarball ships only python.exe / pythonw.exe — no +// python3.exe, no pip.exe — so the static list would create phantom shims +// (python3, pip3) that error at invocation time. Pip executables only +// appear in Scripts/ after installPipIfNeeded succeeds, and scanning here +// ensures they're picked up when present and silently skipped when not. func (p *Provider) createShims(version string) error { manager, err := shim.NewManager() if err != nil { return err } - shimNames := shim.RuntimeShims("python") + versionDir := config.RuntimeVersionPath("python", version) + shimNames := shim.DiscoverShimsForVersion(versionDir) + if len(shimNames) == 0 { + return fmt.Errorf("no executables found in %s", versionDir) + } return manager.CreateShimsForRuntime("python", version, shimNames) } diff --git a/src/runtimes/ruby/provider_full.go b/src/runtimes/ruby/provider_full.go index 21242ff..73e5747 100644 --- a/src/runtimes/ruby/provider_full.go +++ b/src/runtimes/ruby/provider_full.go @@ -197,13 +197,21 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) { // than falling back to the provider registry. The version is recorded in the // cache so the shim can detect when an active runtime version is one that // does not provide a given executable. +// +// The shim list is derived from disk (the same scan reshim uses), not from +// the provider's static Shims() declaration, so install and reshim stay in +// sync and only register executables that actually exist for this version. func (p *Provider) createShims(version string) error { manager, err := shim.NewManager() if err != nil { return err } - shimNames := shim.RuntimeShims("ruby") + versionDir := config.RuntimeVersionPath("ruby", version) + shimNames := shim.DiscoverShimsForVersion(versionDir) + if len(shimNames) == 0 { + return fmt.Errorf("no executables found in %s", versionDir) + } return manager.CreateShimsForRuntime("ruby", version, shimNames) }