Skip to content

Commit 94f3c88

Browse files
authored
fix(shim): handle Windows .exe case + seed shim-map at install (#245)
1 parent bdbf8ab commit 94f3c88

8 files changed

Lines changed: 320 additions & 15 deletions

File tree

src/cmd/shim/main.go

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,15 +155,32 @@ func handleNoConfiguredVersion(shimName, runtimeName string, provider runtime.Sh
155155
return fmt.Errorf("no version configured")
156156
}
157157

158-
// getShimName returns the name of this shim binary
158+
// getShimName returns the name of this shim binary based on os.Args[0].
159159
func getShimName() string {
160-
shimPath := os.Args[0]
161-
shimName := filepath.Base(shimPath)
160+
return shimNameFromPath(os.Args[0])
161+
}
162162

163-
// Remove .exe extension on Windows
164-
shimName = strings.TrimSuffix(shimName, ".exe")
163+
// shimNameFromPath derives the shim name from an invocation path.
164+
//
165+
// On Windows, the filename's .exe extension is stripped case-insensitively.
166+
// This matters because Windows command resolution via PATHEXT can surface
167+
// uppercase extensions (e.g., Python's shutil.which returns "mmdc.EXE"
168+
// when PATHEXT contains ".EXE"). A case-sensitive TrimSuffix would leave
169+
// the uppercase extension attached, breaking every downstream lookup in
170+
// the shim-map cache and the provider registry.
171+
func shimNameFromPath(shimPath string) string {
172+
// Split on both separators so Windows-style paths resolve correctly even
173+
// when this runs on a host where filepath.Base ignores backslashes.
174+
if i := strings.LastIndexAny(shimPath, `/\`); i >= 0 {
175+
shimPath = shimPath[i+1:]
176+
}
165177

166-
return shimName
178+
// Strip .exe / .EXE / any mixed case on Windows-style paths.
179+
if ext := filepath.Ext(shimPath); strings.EqualFold(ext, constants.ExtExe) {
180+
shimPath = shimPath[:len(shimPath)-len(ext)]
181+
}
182+
183+
return shimPath
167184
}
168185

169186
// mapShimToRuntime maps a shim name to its runtime

src/cmd/shim/main_test.go

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
package main
2+
3+
import "testing"
4+
5+
func TestShimNameFromPath(t *testing.T) {
6+
tests := []struct {
7+
name string
8+
shimPath string
9+
want string
10+
}{
11+
{
12+
name: "unix-style bare binary",
13+
shimPath: "/home/user/.dtvem/shims/mmdc",
14+
want: "mmdc",
15+
},
16+
{
17+
name: "windows lowercase .exe",
18+
shimPath: `C:\Users\calvin\.dtvem\shims\mmdc.exe`,
19+
want: "mmdc",
20+
},
21+
{
22+
name: "windows uppercase .EXE (PATHEXT-resolved)",
23+
shimPath: `C:\Users\calvin\.dtvem\shims\mmdc.EXE`,
24+
want: "mmdc",
25+
},
26+
{
27+
name: "windows mixed case .Exe",
28+
shimPath: `C:\Users\calvin\.dtvem\shims\mmdc.Exe`,
29+
want: "mmdc",
30+
},
31+
{
32+
name: "forward-slash path with uppercase extension",
33+
shimPath: "C:/Users/calvin/.dtvem/shims/npm.EXE",
34+
want: "npm",
35+
},
36+
{
37+
name: "bare shim name without extension",
38+
shimPath: "mmdc",
39+
want: "mmdc",
40+
},
41+
{
42+
name: "non-.exe extension is preserved (not stripped)",
43+
shimPath: `C:\tools\something.bat`,
44+
want: "something.bat",
45+
},
46+
}
47+
48+
for _, tt := range tests {
49+
t.Run(tt.name, func(t *testing.T) {
50+
got := shimNameFromPath(tt.shimPath)
51+
if got != tt.want {
52+
t.Errorf("shimNameFromPath(%q) = %q, want %q", tt.shimPath, got, tt.want)
53+
}
54+
})
55+
}
56+
}

src/internal/shim/cache.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,34 @@ func SaveShimMap(shimMap ShimMap) error {
6666
return os.WriteFile(cachePath, data, 0644)
6767
}
6868

69+
// MergeShimMap merges the given entries into the on-disk shim map and persists it.
70+
//
71+
// If the cache does not exist yet (first-time install), a new map is created.
72+
// Existing entries with matching keys are overwritten. The in-memory cache is
73+
// reset so subsequent LoadShimMap calls read the updated state from disk.
74+
//
75+
// This is the preferred path for install-time shim registration, where the
76+
// caller knows only the shims it just created and wants to register them
77+
// without rebuilding the entire map (which would require scanning every
78+
// installed runtime — `Rehash` does that).
79+
func MergeShimMap(entries ShimMap) error {
80+
existing, err := loadShimMapFromDisk()
81+
if err != nil || existing == nil {
82+
// Cache missing, unreadable, or empty — start a fresh map.
83+
existing = make(ShimMap, len(entries))
84+
}
85+
86+
for shim, runtime := range entries {
87+
existing[shim] = runtime
88+
}
89+
90+
// Force the next LoadShimMap to re-read from disk so the merged entries
91+
// are visible to any subsequent caller in the same process.
92+
ResetShimMapCache()
93+
94+
return SaveShimMap(existing)
95+
}
96+
6997
// LookupRuntime looks up the runtime for a given shim name using the cache.
7098
// Returns the runtime name and true if found, or empty string and false if not.
7199
func LookupRuntime(shimName string) (string, bool) {

src/internal/shim/cache_test.go

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,3 +218,179 @@ func TestShimMapCacheOnlyLoadsOnce(t *testing.T) {
218218
t.Errorf("Cache should not have reloaded - 'new' entry should not exist")
219219
}
220220
}
221+
222+
func TestMergeShimMap_CreatesWhenNoExistingCache(t *testing.T) {
223+
tempDir := t.TempDir()
224+
225+
originalRoot := os.Getenv("DTVEM_ROOT")
226+
_ = os.Setenv("DTVEM_ROOT", tempDir)
227+
defer func() { _ = os.Setenv("DTVEM_ROOT", originalRoot) }()
228+
229+
config.ResetPathsCache()
230+
defer config.ResetPathsCache()
231+
232+
ResetShimMapCache()
233+
defer ResetShimMapCache()
234+
235+
// No cache directory pre-existing — MergeShimMap must create it from scratch.
236+
entries := ShimMap{
237+
"node": "node",
238+
"npm": "node",
239+
"npx": "node",
240+
}
241+
242+
if err := MergeShimMap(entries); err != nil {
243+
t.Fatalf("MergeShimMap returned error on fresh install: %v", err)
244+
}
245+
246+
loaded, err := LoadShimMap()
247+
if err != nil {
248+
t.Fatalf("LoadShimMap after MergeShimMap failed: %v", err)
249+
}
250+
251+
if len(loaded) != len(entries) {
252+
t.Errorf("expected %d entries, got %d (%v)", len(entries), len(loaded), loaded)
253+
}
254+
for shim, runtime := range entries {
255+
if got := loaded[shim]; got != runtime {
256+
t.Errorf("entry %q: expected runtime %q, got %q", shim, runtime, got)
257+
}
258+
}
259+
}
260+
261+
func TestMergeShimMap_MergesIntoExistingCache(t *testing.T) {
262+
tempDir := t.TempDir()
263+
264+
originalRoot := os.Getenv("DTVEM_ROOT")
265+
_ = os.Setenv("DTVEM_ROOT", tempDir)
266+
defer func() { _ = os.Setenv("DTVEM_ROOT", originalRoot) }()
267+
268+
config.ResetPathsCache()
269+
defer config.ResetPathsCache()
270+
271+
ResetShimMapCache()
272+
defer ResetShimMapCache()
273+
274+
cacheDir := filepath.Join(tempDir, "cache")
275+
if err := os.MkdirAll(cacheDir, 0755); err != nil {
276+
t.Fatalf("Failed to create cache directory: %v", err)
277+
}
278+
279+
// Seed an existing cache (simulates a prior install).
280+
initial := ShimMap{
281+
"python": "python",
282+
"pip": "python",
283+
}
284+
if err := SaveShimMap(initial); err != nil {
285+
t.Fatalf("seed SaveShimMap failed: %v", err)
286+
}
287+
288+
// Merge in a disjoint set of entries (simulates installing a second runtime).
289+
added := ShimMap{
290+
"node": "node",
291+
"npm": "node",
292+
}
293+
if err := MergeShimMap(added); err != nil {
294+
t.Fatalf("MergeShimMap failed: %v", err)
295+
}
296+
297+
loaded, err := LoadShimMap()
298+
if err != nil {
299+
t.Fatalf("LoadShimMap failed: %v", err)
300+
}
301+
302+
// All four entries should now be present.
303+
wantAll := ShimMap{
304+
"python": "python",
305+
"pip": "python",
306+
"node": "node",
307+
"npm": "node",
308+
}
309+
for shim, runtime := range wantAll {
310+
if got := loaded[shim]; got != runtime {
311+
t.Errorf("entry %q: expected runtime %q, got %q", shim, runtime, got)
312+
}
313+
}
314+
}
315+
316+
func TestMergeShimMap_OverwritesExistingKeys(t *testing.T) {
317+
tempDir := t.TempDir()
318+
319+
originalRoot := os.Getenv("DTVEM_ROOT")
320+
_ = os.Setenv("DTVEM_ROOT", tempDir)
321+
defer func() { _ = os.Setenv("DTVEM_ROOT", originalRoot) }()
322+
323+
config.ResetPathsCache()
324+
defer config.ResetPathsCache()
325+
326+
ResetShimMapCache()
327+
defer ResetShimMapCache()
328+
329+
cacheDir := filepath.Join(tempDir, "cache")
330+
if err := os.MkdirAll(cacheDir, 0755); err != nil {
331+
t.Fatalf("Failed to create cache directory: %v", err)
332+
}
333+
334+
// Seed with a stale mapping (e.g., a shim that was previously attributed
335+
// to the wrong runtime by some prior state).
336+
stale := ShimMap{"corepack": "wrong"}
337+
if err := SaveShimMap(stale); err != nil {
338+
t.Fatalf("seed SaveShimMap failed: %v", err)
339+
}
340+
341+
// Merge should overwrite with the correct runtime.
342+
if err := MergeShimMap(ShimMap{"corepack": "node"}); err != nil {
343+
t.Fatalf("MergeShimMap failed: %v", err)
344+
}
345+
346+
loaded, err := LoadShimMap()
347+
if err != nil {
348+
t.Fatalf("LoadShimMap failed: %v", err)
349+
}
350+
351+
if got := loaded["corepack"]; got != "node" {
352+
t.Errorf("expected corepack remapped to node, got %q", got)
353+
}
354+
}
355+
356+
func TestMergeShimMap_ResetsInMemoryCache(t *testing.T) {
357+
tempDir := t.TempDir()
358+
359+
originalRoot := os.Getenv("DTVEM_ROOT")
360+
_ = os.Setenv("DTVEM_ROOT", tempDir)
361+
defer func() { _ = os.Setenv("DTVEM_ROOT", originalRoot) }()
362+
363+
config.ResetPathsCache()
364+
defer config.ResetPathsCache()
365+
366+
ResetShimMapCache()
367+
defer ResetShimMapCache()
368+
369+
cacheDir := filepath.Join(tempDir, "cache")
370+
if err := os.MkdirAll(cacheDir, 0755); err != nil {
371+
t.Fatalf("Failed to create cache directory: %v", err)
372+
}
373+
374+
// Prime the in-memory cache with an initial map.
375+
if err := SaveShimMap(ShimMap{"node": "node"}); err != nil {
376+
t.Fatalf("SaveShimMap failed: %v", err)
377+
}
378+
if _, err := LoadShimMap(); err != nil {
379+
t.Fatalf("initial LoadShimMap failed: %v", err)
380+
}
381+
382+
// Without ResetShimMapCache, the next Load would return the cached copy.
383+
// MergeShimMap is supposed to reset it so callers see merged state.
384+
if err := MergeShimMap(ShimMap{"npm": "node"}); err != nil {
385+
t.Fatalf("MergeShimMap failed: %v", err)
386+
}
387+
388+
loaded, err := LoadShimMap()
389+
if err != nil {
390+
t.Fatalf("post-merge LoadShimMap failed: %v", err)
391+
}
392+
393+
if _, ok := loaded["npm"]; !ok {
394+
t.Error("expected in-memory cache to be reset so the merged 'npm' entry is visible")
395+
}
396+
}

src/internal/shim/manager.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,28 @@ func (m *Manager) CreateShims(shimNames []string) error {
108108
return nil
109109
}
110110

111+
// CreateShimsForRuntime creates shim files for the given names and registers
112+
// them in the shim map under the given runtime name.
113+
//
114+
// This is the preferred path for install-time shim creation (e.g., from a
115+
// runtime provider's post-install hook). Bare CreateShims only writes the
116+
// shim binaries to disk — it does not update the shim-map cache, which means
117+
// subsequent shim invocations have to fall back to the provider registry
118+
// lookup instead of the O(1) cache hit. Calling CreateShimsForRuntime keeps
119+
// the shim files and the cache in sync from the moment they are created.
120+
func (m *Manager) CreateShimsForRuntime(runtimeName string, shimNames []string) error {
121+
if err := m.CreateShims(shimNames); err != nil {
122+
return err
123+
}
124+
125+
entries := make(ShimMap, len(shimNames))
126+
for _, name := range shimNames {
127+
entries[name] = runtimeName
128+
}
129+
130+
return MergeShimMap(entries)
131+
}
132+
111133
// RemoveShim removes a shim
112134
func (m *Manager) RemoveShim(shimName string) error {
113135
shimPath := config.ShimPath(shimName)

src/runtimes/node/provider.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,9 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) {
152152
return dl.URL, archiveName, nil
153153
}
154154

155-
// createShims creates shims for Node.js executables
155+
// createShims creates shims for Node.js executables and registers them in the
156+
// shim-map cache so subsequent shim invocations resolve via O(1) lookup rather
157+
// than falling back to the provider registry.
156158
func (p *Provider) createShims() error {
157159
manager, err := shim.NewManager()
158160
if err != nil {
@@ -162,8 +164,8 @@ func (p *Provider) createShims() error {
162164
// Get the list of shims for Node.js
163165
shimNames := shim.RuntimeShims("node")
164166

165-
// Create each shim
166-
return manager.CreateShims(shimNames)
167+
// Create each shim AND record them in the shim map cache
168+
return manager.CreateShimsForRuntime("node", shimNames)
167169
}
168170

169171
// Uninstall removes an installed version

src/runtimes/python/provider.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,9 @@ func (p *Provider) getDownloadURL(version string) (string, string, error) {
209209
return dl.URL, archiveName, nil
210210
}
211211

212-
// createShims creates shims for Python executables
212+
// createShims creates shims for Python executables and registers them in the
213+
// shim-map cache so subsequent shim invocations resolve via O(1) lookup rather
214+
// than falling back to the provider registry.
213215
func (p *Provider) createShims() error {
214216
manager, err := shim.NewManager()
215217
if err != nil {
@@ -219,8 +221,8 @@ func (p *Provider) createShims() error {
219221
// Get the list of shims for Python
220222
shimNames := shim.RuntimeShims("python")
221223

222-
// Create each shim
223-
return manager.CreateShims(shimNames)
224+
// Create each shim AND record them in the shim map cache
225+
return manager.CreateShimsForRuntime("python", shimNames)
224226
}
225227

226228
// installPip ensures pip is properly installed with working executables.

0 commit comments

Comments
 (0)