mirror of
https://github.com/safedep/pmg.git
synced 2026-08-03 07:24:09 +02:00
feat: migrate PMG to use PATH shims for package manager wrapper (#246)
* feat: add FilterPMGFromPath utility for PATH shim recursion prevention * feat: add FilterPMGFromEnv to filter PATH from env slices * feat: filter ~/.pmg/bin from PATH in proxy subprocess env * feat: add PathExport method to Shell interface for shim PATH integration * feat: add ShimManager for PATH shim install/remove lifecycle * feat: wire ShimManager into setup commands with --use-aliases fallback * refactor: add DefaultShimConfig helper to reduce setup boilerplate * fix: resolve real binary path to prevent shim double-invocation exec.CommandContext resolves the binary using the current process PATH, which still contains ~/.pmg/bin. This caused pmg to launch the shim instead of the real package manager, resulting in a second pmg instance with its own proxy — producing duplicate error messages and wasted work. ResolveRealBinary searches a filtered PATH (without ~/.pmg/bin) to find the real package manager binary before execution. * fix: resolve real binary in runner.Execute and expand path resolution tests Ensure guard mode and proxy skip paths also resolve through ResolveRealBinary to prevent infinite shim recursion. Add table-driven tests covering error cases, multi-binary PATH, and PATH restoration. * fix: handle error return values from os.Setenv and file Close calls Address errcheck lint failures: check os.Setenv returns in ResolveRealBinary, and check f.Close/tempFile.Close in ShimManager. * feat: auto-migrate shell aliases to PATH shims on setup install When running `pmg setup install`, detect existing shell aliases and automatically remove them before installing shims. Existing users get a seamless migration with no extra flags or commands needed. * fix: update E2E test to verify shim installation instead of alias RC file Replace the .pmg.rc file check with assertions that ~/.pmg/bin/ exists and contains executable shim scripts for npm and pip. * feat: add FilterPMGFromPath utility for PATH shim recursion prevention * feat: add FilterPMGFromEnv to filter PATH from env slices * feat: filter ~/.pmg/bin from PATH in proxy subprocess env * feat: add PathExport method to Shell interface for shim PATH integration * feat: add ShimManager for PATH shim install/remove lifecycle * feat: wire ShimManager into setup commands with --use-aliases fallback * refactor: add DefaultShimConfig helper to reduce setup boilerplate * fix: resolve real binary path to prevent shim double-invocation exec.CommandContext resolves the binary using the current process PATH, which still contains ~/.pmg/bin. This caused pmg to launch the shim instead of the real package manager, resulting in a second pmg instance with its own proxy — producing duplicate error messages and wasted work. ResolveRealBinary searches a filtered PATH (without ~/.pmg/bin) to find the real package manager binary before execution. * fix: resolve real binary in runner.Execute and expand path resolution tests Ensure guard mode and proxy skip paths also resolve through ResolveRealBinary to prevent infinite shim recursion. Add table-driven tests covering error cases, multi-binary PATH, and PATH restoration. * fix: handle error return values from os.Setenv and file Close calls Address errcheck lint failures: check os.Setenv returns in ResolveRealBinary, and check f.Close/tempFile.Close in ShimManager. * feat: auto-migrate shell aliases to PATH shims on setup install When running `pmg setup install`, detect existing shell aliases and automatically remove them before installing shims. Existing users get a seamless migration with no extra flags or commands needed. * fix: update E2E test to verify shim installation instead of alias RC file Replace the .pmg.rc file check with assertions that ~/.pmg/bin/ exists and contains executable shim scripts for npm and pip. * feat: install both aliases and shims for full coverage Aliases win in interactive shells (including venvs), shims catch non-interactive contexts (IDEs, CI, subprocesses). Remove --use-aliases flag and migration logic since both are always installed together. Update E2E to verify all shim scripts and alias RC file. * feat: address review feedback for shim implementation - Install both aliases and shims together for full coverage - Move homeDir resolution into NewDefaultShimManager (internal concern) - Add mutex to ResolveRealBinary to guard against concurrent PATH mutation - Use filepath.SplitList for platform-correct PATH splitting - Add ResolveRealBinary to runner.Execute and proxy flow to prevent shim recursion in all execution paths - Remove print side-effects from ShimManager.Remove - Update E2E to verify all shim scripts and alias RC file - Expand ResolveRealBinary tests with table-driven cases * fix: restore errcheck handling and add concurrency test for ResolveRealBinary - Restore proper defer with log.Warnf for PATH restoration in ResolveRealBinary - Restore errcheck handling for f.Close() and tempFile.Close() in ShimManager - Add explanatory comment for ResolveRealBinary call in proxy_flow - Add TestResolveRealBinaryConcurrent to verify mutex guards concurrent access * feat: skip shell integration on Windows with informative warning On Windows, pmg setup install now writes only the config file and prints a warning that shell aliases and PATH shims require WSL. * fix: PMG use pre-resolved binary path (#253) --------- Co-authored-by: Abhisek Datta <abhisek.datta@gmail.com>
This commit is contained in:
Sahil Bansal
co-authored by
Abhisek Datta
Abhisek Datta
parent
d993d57e3d
commit
6546116e28
@@ -0,0 +1,256 @@
|
||||
package shim
|
||||
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestFilterPMGFromPath(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
path string
|
||||
expected string
|
||||
}{
|
||||
{
|
||||
name: "removes pmg bin from middle",
|
||||
path: "/usr/local/bin:/home/user/.pmg/bin:/usr/bin",
|
||||
expected: "/usr/local/bin:/usr/bin",
|
||||
},
|
||||
{
|
||||
name: "removes pmg bin from start",
|
||||
path: "/home/user/.pmg/bin:/usr/local/bin:/usr/bin",
|
||||
expected: "/usr/local/bin:/usr/bin",
|
||||
},
|
||||
{
|
||||
name: "removes pmg bin from end",
|
||||
path: "/usr/local/bin:/usr/bin:/home/user/.pmg/bin",
|
||||
expected: "/usr/local/bin:/usr/bin",
|
||||
},
|
||||
{
|
||||
name: "no pmg bin present",
|
||||
path: "/usr/local/bin:/usr/bin",
|
||||
expected: "/usr/local/bin:/usr/bin",
|
||||
},
|
||||
{
|
||||
name: "empty path",
|
||||
path: "",
|
||||
expected: "",
|
||||
},
|
||||
{
|
||||
name: "only pmg bin",
|
||||
path: "/home/user/.pmg/bin",
|
||||
expected: "",
|
||||
},
|
||||
{
|
||||
name: "does not remove partial matches",
|
||||
path: "/usr/local/bin:/home/user/.pmg/binaries:/usr/bin",
|
||||
expected: "/usr/local/bin:/home/user/.pmg/binaries:/usr/bin",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
result := FilterPMGFromPath(tc.path)
|
||||
assert.Equal(t, tc.expected, result)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveRealBinary(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
setupDirs func(t *testing.T, tmpDir string) (pmgBin, realBin string)
|
||||
binary string
|
||||
wantPath func(realBinDir string) string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "skips shim and finds real binary",
|
||||
setupDirs: func(t *testing.T, tmpDir string) (string, string) {
|
||||
pmgBin := filepath.Join(tmpDir, ".pmg", "bin")
|
||||
realBin := filepath.Join(tmpDir, "real-bin")
|
||||
require.NoError(t, os.MkdirAll(pmgBin, 0o755))
|
||||
require.NoError(t, os.MkdirAll(realBin, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755))
|
||||
return pmgBin, realBin
|
||||
},
|
||||
binary: "npm",
|
||||
wantPath: func(realBin string) string { return filepath.Join(realBin, "npm") },
|
||||
},
|
||||
{
|
||||
name: "returns error when binary not found outside shim dir",
|
||||
setupDirs: func(t *testing.T, tmpDir string) (string, string) {
|
||||
pmgBin := filepath.Join(tmpDir, ".pmg", "bin")
|
||||
realBin := filepath.Join(tmpDir, "real-bin")
|
||||
require.NoError(t, os.MkdirAll(pmgBin, 0o755))
|
||||
require.NoError(t, os.MkdirAll(realBin, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755))
|
||||
return pmgBin, realBin
|
||||
},
|
||||
binary: "npm",
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "works when no shim dir exists in PATH",
|
||||
setupDirs: func(t *testing.T, tmpDir string) (string, string) {
|
||||
realBin := filepath.Join(tmpDir, "real-bin")
|
||||
require.NoError(t, os.MkdirAll(realBin, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755))
|
||||
return "", realBin
|
||||
},
|
||||
binary: "npm",
|
||||
wantPath: func(realBin string) string { return filepath.Join(realBin, "npm") },
|
||||
},
|
||||
{
|
||||
name: "resolves correct binary when multiple exist",
|
||||
setupDirs: func(t *testing.T, tmpDir string) (string, string) {
|
||||
pmgBin := filepath.Join(tmpDir, ".pmg", "bin")
|
||||
firstBin := filepath.Join(tmpDir, "first-bin")
|
||||
secondBin := filepath.Join(tmpDir, "second-bin")
|
||||
require.NoError(t, os.MkdirAll(pmgBin, 0o755))
|
||||
require.NoError(t, os.MkdirAll(firstBin, 0o755))
|
||||
require.NoError(t, os.MkdirAll(secondBin, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(firstBin, "npm"), []byte("#!/bin/sh\necho first"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(secondBin, "npm"), []byte("#!/bin/sh\necho second"), 0o755))
|
||||
return pmgBin, firstBin + ":" + secondBin
|
||||
},
|
||||
binary: "npm",
|
||||
wantPath: func(realBin string) string { return filepath.Join(filepath.SplitList(realBin)[0], "npm") },
|
||||
},
|
||||
{
|
||||
name: "restores original PATH after resolution",
|
||||
setupDirs: func(t *testing.T, tmpDir string) (string, string) {
|
||||
pmgBin := filepath.Join(tmpDir, ".pmg", "bin")
|
||||
realBin := filepath.Join(tmpDir, "real-bin")
|
||||
require.NoError(t, os.MkdirAll(pmgBin, 0o755))
|
||||
require.NoError(t, os.MkdirAll(realBin, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755))
|
||||
return pmgBin, realBin
|
||||
},
|
||||
binary: "npm",
|
||||
wantPath: func(realBin string) string { return filepath.Join(realBin, "npm") },
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
pmgBin, realBin := tc.setupDirs(t, tmpDir)
|
||||
|
||||
var pathParts []string
|
||||
if pmgBin != "" {
|
||||
pathParts = append(pathParts, pmgBin)
|
||||
}
|
||||
pathParts = append(pathParts, filepath.SplitList(realBin)...)
|
||||
|
||||
t.Setenv("PATH", strings.Join(pathParts, ":"))
|
||||
originalPath := os.Getenv("PATH")
|
||||
|
||||
resolved, err := ResolveRealBinary(tc.binary)
|
||||
|
||||
if tc.wantErr {
|
||||
assert.Error(t, err)
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, tc.wantPath(realBin), resolved)
|
||||
|
||||
assert.Equal(t, originalPath, os.Getenv("PATH"), "PATH should be restored after ResolveRealBinary")
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResolveRealBinaryConcurrent(t *testing.T) {
|
||||
tmpDir := t.TempDir()
|
||||
|
||||
pmgBin := filepath.Join(tmpDir, ".pmg", "bin")
|
||||
realBin := filepath.Join(tmpDir, "real-bin")
|
||||
require.NoError(t, os.MkdirAll(pmgBin, 0o755))
|
||||
require.NoError(t, os.MkdirAll(realBin, 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(pmgBin, "npm"), []byte("#!/bin/sh\necho shim"), 0o755))
|
||||
require.NoError(t, os.WriteFile(filepath.Join(realBin, "npm"), []byte("#!/bin/sh\necho real"), 0o755))
|
||||
|
||||
t.Setenv("PATH", pmgBin+":"+realBin)
|
||||
|
||||
const goroutines = 10
|
||||
errs := make(chan error, goroutines)
|
||||
paths := make(chan string, goroutines)
|
||||
|
||||
for range goroutines {
|
||||
go func() {
|
||||
resolved, err := ResolveRealBinary("npm")
|
||||
if err != nil {
|
||||
errs <- err
|
||||
paths <- ""
|
||||
return
|
||||
}
|
||||
errs <- nil
|
||||
paths <- resolved
|
||||
}()
|
||||
}
|
||||
|
||||
expectedPath := filepath.Join(realBin, "npm")
|
||||
for i := range goroutines {
|
||||
assert.NoError(t, <-errs, "goroutine %d should not error", i)
|
||||
resolved := <-paths
|
||||
if resolved != "" {
|
||||
assert.Equal(t, expectedPath, resolved, "goroutine %d should resolve to real binary", i)
|
||||
}
|
||||
}
|
||||
|
||||
assert.Equal(t, pmgBin+":"+realBin, os.Getenv("PATH"), "PATH should be restored after concurrent calls")
|
||||
}
|
||||
|
||||
func TestFilterPMGFromEnv(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
env []string
|
||||
expected []string
|
||||
}{
|
||||
{
|
||||
name: "filters PATH entry",
|
||||
env: []string{
|
||||
"HOME=/home/user",
|
||||
"PATH=/home/user/.pmg/bin:/usr/local/bin:/usr/bin",
|
||||
"SHELL=/bin/zsh",
|
||||
},
|
||||
expected: []string{
|
||||
"HOME=/home/user",
|
||||
"PATH=/usr/local/bin:/usr/bin",
|
||||
"SHELL=/bin/zsh",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "no PATH entry",
|
||||
env: []string{
|
||||
"HOME=/home/user",
|
||||
"SHELL=/bin/zsh",
|
||||
},
|
||||
expected: []string{
|
||||
"HOME=/home/user",
|
||||
"SHELL=/bin/zsh",
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "empty env",
|
||||
env: []string{},
|
||||
expected: []string{},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
result := FilterPMGFromEnv(tc.env)
|
||||
assert.Equal(t, tc.expected, result)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user