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) }