From 4a1e34d06bd50c9f828a5ca049a19854dc07f063 Mon Sep 17 00:00:00 2001 From: RishiAhuja Date: Sat, 21 Feb 2026 17:11:00 +0530 Subject: [PATCH] feat(installs): implement staging directory for atomic installs and cleanup of incomplete installations --- internal/installs/installs.go | 25 +++++++++++- internal/installs/installs_test.go | 62 +++++++++++++++++++++++++++++ internal/installtest/installtest.go | 17 ++++++-- internal/versions/versions.go | 26 +++++++++--- internal/versions/versions_test.go | 62 +++++++++++++++++++++++++++++ 5 files changed, 183 insertions(+), 9 deletions(-) diff --git a/internal/installs/installs.go b/internal/installs/installs.go index 493fcb6b6..599154015 100644 --- a/internal/installs/installs.go +++ b/internal/installs/installs.go @@ -7,6 +7,7 @@ import ( "io/fs" "os" "path/filepath" + "strings" "github.com/asdf-vm/asdf/internal/config" "github.com/asdf-vm/asdf/internal/data" @@ -14,6 +15,8 @@ import ( "github.com/asdf-vm/asdf/internal/toolversions" ) +const incompleteSuffix = ".incomplete" + // Installed returns a slice of all installed versions for a given plugin func Installed(conf config.Config, plugin plugins.Plugin) (versions []string, err error) { installDirectory := data.InstallDirectory(conf.DataDir, plugin.Name) @@ -31,7 +34,13 @@ func Installed(conf config.Config, plugin plugins.Plugin) (versions []string, er continue } - versions = append(versions, toolversions.VersionStringFromFSFormat(file.Name())) + name := file.Name() + // Skip incomplete installations left behind by interrupted installs + if strings.HasSuffix(name, incompleteSuffix) { + continue + } + + versions = append(versions, toolversions.VersionStringFromFSFormat(name)) } return versions, err @@ -55,6 +64,20 @@ func DownloadPath(conf config.Config, plugin plugins.Plugin, version toolversion return filepath.Join(data.DownloadDirectory(conf.DataDir, plugin.Name), toolversions.FormatForFS(version)) } +// StagingPath returns the temporary staging path used during installation. +// Installations are first performed in this directory and then atomically +// renamed to the final InstallPath on success, so that interrupted installs +// never appear as valid installed versions. +func StagingPath(conf config.Config, plugin plugins.Plugin, version toolversions.Version) string { + return InstallPath(conf, plugin, version) + incompleteSuffix +} + +// CleanIncomplete removes any stale staging directories for a given plugin and +// version. It is safe to call even if no incomplete directory exists. +func CleanIncomplete(conf config.Config, plugin plugins.Plugin, version toolversions.Version) error { + return os.RemoveAll(StagingPath(conf, plugin, version)) +} + // IsInstalled checks if a specific version of a tool is installed func IsInstalled(conf config.Config, plugin plugins.Plugin, version toolversions.Version) bool { installDir := InstallPath(conf, plugin, version) diff --git a/internal/installs/installs_test.go b/internal/installs/installs_test.go index f8e7f22c4..744c1b038 100644 --- a/internal/installs/installs_test.go +++ b/internal/installs/installs_test.go @@ -77,6 +77,68 @@ func TestIsInstalled(t *testing.T) { version := toolversions.Version{Type: "version", Value: "1.0.0"} assert.True(t, IsInstalled(conf, plugin, version)) }) + t.Run("returns false for incomplete installation", func(t *testing.T) { + // Create an .incomplete staging directory (simulating interrupted install) + version := toolversions.Version{Type: "version", Value: "2.0.0"} + stagingPath := InstallPath(conf, plugin, version) + ".incomplete" + err := os.MkdirAll(stagingPath, os.ModePerm) + assert.Nil(t, err) + + assert.False(t, IsInstalled(conf, plugin, version)) + }) +} + +func TestInstalled_FiltersIncomplete(t *testing.T) { + conf, plugin := generateConfig(t) + + t.Run("does not list incomplete installations", func(t *testing.T) { + // Install a real version + mockInstall(t, conf, plugin, "1.0.0") + + // Create an .incomplete staging directory + incompletePath := filepath.Join(conf.DataDir, "installs", testPluginName, "2.0.0.incomplete") + err := os.MkdirAll(incompletePath, os.ModePerm) + assert.Nil(t, err) + + installedVersions, err := Installed(conf, plugin) + assert.Nil(t, err) + assert.Equal(t, []string{"1.0.0"}, installedVersions) + }) +} + +func TestStagingPath(t *testing.T) { + conf, plugin := generateConfig(t) + + t.Run("returns install path with .incomplete suffix", func(t *testing.T) { + version := toolversions.Version{Type: "version", Value: "1.2.3"} + path := StagingPath(conf, plugin, version) + assert.Equal(t, filepath.Join(conf.DataDir, "installs", "lua", "1.2.3.incomplete"), path) + }) +} + +func TestCleanIncomplete(t *testing.T) { + conf, plugin := generateConfig(t) + + t.Run("removes incomplete staging directory", func(t *testing.T) { + version := toolversions.Version{Type: "version", Value: "1.2.3"} + stagingPath := StagingPath(conf, plugin, version) + err := os.MkdirAll(stagingPath, os.ModePerm) + assert.Nil(t, err) + err = os.WriteFile(filepath.Join(stagingPath, "somefile"), []byte("data"), 0o666) + assert.Nil(t, err) + + err = CleanIncomplete(conf, plugin, version) + assert.Nil(t, err) + + _, err = os.Stat(stagingPath) + assert.True(t, os.IsNotExist(err)) + }) + + t.Run("succeeds even when no incomplete directory exists", func(t *testing.T) { + version := toolversions.Version{Type: "version", Value: "9.9.9"} + err := CleanIncomplete(conf, plugin, version) + assert.Nil(t, err) + }) } // helper functions diff --git a/internal/installtest/installtest.go b/internal/installtest/installtest.go index 14b5beffc..ea22923d4 100644 --- a/internal/installtest/installtest.go +++ b/internal/installtest/installtest.go @@ -32,11 +32,15 @@ func InstallOneVersion(conf config.Config, plugin plugins.Plugin, versionType, v downloadDir := DownloadPath(conf, plugin, version) installDir := InstallPath(conf, plugin, version) + stagingDir := installDir + ".incomplete" + + // Clean up any previous incomplete install + os.RemoveAll(stagingDir) env := map[string]string{ "ASDF_INSTALL_TYPE": versionType, "ASDF_INSTALL_VERSION": version, - "ASDF_INSTALL_PATH": installDir, + "ASDF_INSTALL_PATH": stagingDir, "ASDF_DOWNLOAD_PATH": downloadDir, "ASDF_CONCURRENCY": "1", } @@ -51,16 +55,23 @@ func InstallOneVersion(conf config.Config, plugin plugins.Plugin, versionType, v return fmt.Errorf("failed to run download callback: %w", err) } - err = os.MkdirAll(installDir, 0o777) + err = os.MkdirAll(stagingDir, 0o777) if err != nil { - return fmt.Errorf("unable to create install dir: %w", err) + return fmt.Errorf("unable to create staging install dir: %w", err) } err = plugin.RunCallback("install", []string{}, env, &stdOut, &stdErr) if err != nil { + os.RemoveAll(stagingDir) return fmt.Errorf("failed to run install callback: %w", err) } + err = os.Rename(stagingDir, installDir) + if err != nil { + os.RemoveAll(stagingDir) + return fmt.Errorf("failed to move staging install to final path: %w", err) + } + return nil } diff --git a/internal/versions/versions.go b/internal/versions/versions.go index 2cf669928..bce19136e 100644 --- a/internal/versions/versions.go +++ b/internal/versions/versions.go @@ -155,16 +155,22 @@ func InstallOneVersion(conf config.Config, plugin plugins.Plugin, versionStr str } downloadDir := installs.DownloadPath(conf, plugin, version) installDir := installs.InstallPath(conf, plugin, version) + stagingDir := installs.StagingPath(conf, plugin, version) if installs.IsInstalled(conf, plugin, version) { return VersionAlreadyInstalledError{version: version, toolName: plugin.Name} } + // Remove any incomplete staging directory from a previously interrupted install + if err := installs.CleanIncomplete(conf, plugin, version); err != nil { + return fmt.Errorf("unable to clean up incomplete install: %w", err) + } + concurrency, _ := conf.Concurrency() env := map[string]string{ "ASDF_INSTALL_TYPE": version.Type, "ASDF_INSTALL_VERSION": version.Value, - "ASDF_INSTALL_PATH": installDir, + "ASDF_INSTALL_PATH": stagingDir, "ASDF_DOWNLOAD_PATH": downloadDir, "ASDF_CONCURRENCY": concurrency, } @@ -189,19 +195,29 @@ func InstallOneVersion(conf config.Config, plugin plugins.Plugin, versionStr str return fmt.Errorf("failed to run pre-install hook: %w", err) } - err = os.MkdirAll(installDir, 0o777) + err = os.MkdirAll(stagingDir, 0o777) if err != nil { - return fmt.Errorf("unable to create install dir: %w", err) + return fmt.Errorf("unable to create staging install dir: %w", err) } err = plugin.RunCallback("install", []string{}, env, stdOut, stdErr) if err != nil { - if rmErr := os.RemoveAll(installDir); rmErr != nil { - fmt.Fprintf(stdErr, "failed to clean up '%s' due to %s\n", installDir, rmErr) + if rmErr := os.RemoveAll(stagingDir); rmErr != nil { + fmt.Fprintf(stdErr, "failed to clean up staging dir '%s' due to %s\n", stagingDir, rmErr) } return fmt.Errorf("failed to run install callback: %w", err) } + // Atomically move the staging directory to the final install path so that + // interrupted installs never appear as valid installed versions. + err = os.Rename(stagingDir, installDir) + if err != nil { + if rmErr := os.RemoveAll(stagingDir); rmErr != nil { + fmt.Fprintf(stdErr, "failed to clean up staging dir '%s' due to %s\n", stagingDir, rmErr) + } + return fmt.Errorf("failed to move staging install to final path: %w", err) + } + // Reshim err = shims.GenerateAll(conf, stdOut, stdErr) if err != nil { diff --git a/internal/versions/versions_test.go b/internal/versions/versions_test.go index bb5a93874..69c225c51 100644 --- a/internal/versions/versions_test.go +++ b/internal/versions/versions_test.go @@ -319,6 +319,68 @@ func TestInstallOneVersion(t *testing.T) { // no-download install script prints 'install' assert.Equal(t, "install", stdout.String()) }) + + t.Run("staging directory does not exist after successful install", func(t *testing.T) { + conf, plugin := generateConfig(t) + stdout, stderr := buildOutputs() + err := InstallOneVersion(conf, plugin, "1.0.0", false, &stdout, &stderr) + assert.Nil(t, err) + + // The .incomplete staging directory should not remain + stagingPath := filepath.Join(conf.DataDir, "installs", plugin.Name, "1.0.0.incomplete") + _, err = os.Stat(stagingPath) + assert.True(t, os.IsNotExist(err), "staging directory should be renamed away after successful install") + + // But the final install path should exist + assertVersionInstalled(t, conf.DataDir, plugin.Name, "1.0.0") + }) + + t.Run("staging directory is cleaned up when install fails", func(t *testing.T) { + conf, plugin := generateConfig(t) + stdout, stderr := buildOutputs() + + installScript := filepath.Join(conf.DataDir, "plugins", plugin.Name, "bin", "install") + f, err := os.OpenFile(installScript, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o777) + assert.Nil(t, err) + _, err = f.WriteString("\nexit 1") + assert.Nil(t, err) + err = f.Close() + assert.Nil(t, err) + + err = InstallOneVersion(conf, plugin, "1.0.0", false, &stdout, &stderr) + assert.Error(t, err) + + // Both the staging dir and the final dir should not exist + stagingPath := filepath.Join(conf.DataDir, "installs", plugin.Name, "1.0.0.incomplete") + _, err = os.Stat(stagingPath) + assert.True(t, os.IsNotExist(err), "staging directory should be removed after failed install") + + installPath := filepath.Join(conf.DataDir, "installs", plugin.Name, "1.0.0") + _, err = os.Stat(installPath) + assert.True(t, os.IsNotExist(err), "final install directory should not exist after failed install") + }) + + t.Run("cleans up stale incomplete directory from previous failed install", func(t *testing.T) { + conf, plugin := generateConfig(t) + stdout, stderr := buildOutputs() + + // Simulate a stale .incomplete directory left behind by a previous interrupted install + stagingPath := filepath.Join(conf.DataDir, "installs", plugin.Name, "1.0.0.incomplete") + err := os.MkdirAll(stagingPath, 0o777) + assert.Nil(t, err) + err = os.WriteFile(filepath.Join(stagingPath, "stale-file"), []byte("leftover"), 0o666) + assert.Nil(t, err) + + // A new install should succeed, cleaning up the stale dir first + err = InstallOneVersion(conf, plugin, "1.0.0", false, &stdout, &stderr) + assert.Nil(t, err) + + assertVersionInstalled(t, conf.DataDir, plugin.Name, "1.0.0") + + // Staging dir should not exist + _, err = os.Stat(stagingPath) + assert.True(t, os.IsNotExist(err)) + }) } func TestLatest(t *testing.T) {