Skip to content

Commit f0a80bb

Browse files
committed
fix(install): skip atomic rename for Python providers to avoid PE executable corruption
On Windows, Python pip packages generate .exe wrappers that embed the absolute path of the virtual environment. UniRTM's atomic installation strategy (installing into .unirtm-tmp and renaming) caused these embedded paths to break, leading to silent exit code 1 failures (like with pre-commit). Added an opt-out mechanism for atomic rename for Python/Pypi providers.
1 parent 5638fe1 commit f0a80bb

3 files changed

Lines changed: 38 additions & 15 deletions

File tree

internal/provider/pypi.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,12 @@ func (p *PypiProvider) Name() string {
2828
return "pypi"
2929
}
3030

31+
// SkipAtomicRename indicates that this provider requires installing directly into the final path.
32+
// This is necessary because Python virtualenvs hardcode absolute paths in executable PE wrappers on Windows.
33+
func (p *PypiProvider) SkipAtomicRename() bool {
34+
return true
35+
}
36+
3137
func (p *PypiProvider) Install(ctx context.Context, tool string, installPath string, artifactPath string, version string) error {
3238
// Ensure parent dir exists
3339
if err := os.MkdirAll(filepath.Dir(installPath), 0755); err != nil {

internal/provider/python.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ func (p *PythonProvider) Name() string {
3030
return "python"
3131
}
3232

33+
// SkipAtomicRename indicates that this provider requires installing directly into the final path.
34+
// This is necessary because Python virtualenvs hardcode absolute paths in executable PE wrappers on Windows.
35+
func (p *PythonProvider) SkipAtomicRename() bool {
36+
return true
37+
}
38+
3339
// Install performs Python-specific installation.
3440
func (p *PythonProvider) Install(ctx context.Context, tool string, installPath string, artifactPath string, version string) error {
3541
return p.generic.Install(ctx, tool, installPath, artifactPath, version)

internal/service/installation.go

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -673,38 +673,49 @@ func (im *InstallationManager) Install(ctx context.Context, toolKey, tool, versi
673673
installPath := filepath.Join(env.GetInstallsDir(), fsToolName, version)
674674
tmpInstallPath := installPath + ".unirtm-tmp"
675675

676+
p := im.providerRegistry.GetWithBackend(tool, backendName)
677+
678+
var actualInstallPath string
679+
if skippable, ok := p.(interface{ SkipAtomicRename() bool }); ok && skippable.SkipAtomicRename() {
680+
actualInstallPath = installPath
681+
} else {
682+
actualInstallPath = tmpInstallPath
683+
}
684+
676685
// Clean up any stale directories from previous failed attempts
677-
os.RemoveAll(tmpInstallPath)
678-
// If final path exists but we reached here, it means it's not in the database.
679-
// We'll overwrite it to be safe.
680-
os.RemoveAll(installPath)
686+
os.RemoveAll(actualInstallPath)
687+
if actualInstallPath != installPath {
688+
// If final path exists but we reached here, it means it's not in the database.
689+
// We'll overwrite it to be safe.
690+
os.RemoveAll(installPath)
691+
}
681692

682-
if err := os.MkdirAll(filepath.Dir(tmpInstallPath), 0755); err != nil {
693+
if err := os.MkdirAll(filepath.Dir(actualInstallPath), 0755); err != nil {
683694
return fmt.Errorf("failed to create installs directory: %w", err)
684695
}
685696

686-
p := im.providerRegistry.GetWithBackend(tool, backendName)
687-
688697
if !quietProgress {
689698
fmt.Printf("ℹ extracting %s@%s...\n", tool, version)
690699
}
691-
if err := p.Install(ctx, tool, tmpInstallPath, downloadPath, version); err != nil {
692-
os.RemoveAll(tmpInstallPath)
700+
if err := p.Install(ctx, tool, actualInstallPath, downloadPath, version); err != nil {
701+
os.RemoveAll(actualInstallPath)
693702
return fmt.Errorf("installation failed: %w", err)
694703
}
695704

696705
if !quietProgress {
697706
fmt.Printf("ℹ running post-install hooks for %s@%s...\n", tool, version)
698707
}
699-
if err := p.PostInstall(ctx, tool, tmpInstallPath, version); err != nil {
700-
os.RemoveAll(tmpInstallPath)
708+
if err := p.PostInstall(ctx, tool, actualInstallPath, version); err != nil {
709+
os.RemoveAll(actualInstallPath)
701710
return fmt.Errorf("post-install failed: %w", err)
702711
}
703712

704-
// Atomic rename from temp to final path
705-
if err := os.Rename(tmpInstallPath, installPath); err != nil {
706-
os.RemoveAll(tmpInstallPath)
707-
return fmt.Errorf("failed to finalize installation: %w", err)
713+
if actualInstallPath != installPath {
714+
// Atomic rename from temp to final path
715+
if err := os.Rename(actualInstallPath, installPath); err != nil {
716+
os.RemoveAll(actualInstallPath)
717+
return fmt.Errorf("failed to finalize installation: %w", err)
718+
}
708719
}
709720

710721
if !quietProgress {

0 commit comments

Comments
 (0)