avoid text file busy error on track (#2494)

* avoid text file busy error on track

* moved exec setup to utils and added unit

* lint

* fix

* address PR comments

Author: felipemadero

pkg/localnet/tmpnet.go

@@ -376,19 +376,18 @@ func TmpNetSetDefaultAliases(ctx context.Context, networkDir string) error {
 
 // Install the given VM binary into the appropriate location with the
 // appropriate name
-func TmpNetInstallVM(network *tmpnet.Network, binaryPath string, vmID ids.ID) error {
+func TmpNetInstallVM(
+	log logging.Logger,
+	network *tmpnet.Network,
+	binaryPath string,
+	vmID ids.ID,
+) error {
 	pluginDir, err := network.DefaultFlags.GetStringVal(config.PluginDirKey)
 	if err != nil {
 		return err
 	}
 	pluginPath := filepath.Join(pluginDir, vmID.String())
-	if err := utils.FileCopy(binaryPath, pluginPath); err != nil {
-		return err
-	}
-	if err := os.Chmod(pluginPath, constants.DefaultPerms755); err != nil {
-		return err
-	}
-	return nil
+	return utils.SetupExecFile(log, binaryPath, pluginPath)
 }
 
 // Set up blockchain config for all nodes in the network
@@ -589,7 +588,7 @@ func TmpNetTrackSubnet(
 	if err != nil {
 		return err
 	}
-	if err := TmpNetInstallVM(network, vmBinaryPath, vmID); err != nil {
+	if err := TmpNetInstallVM(log, network, vmBinaryPath, vmID); err != nil {
 		return err
 	}
 	// Configs

pkg/node/local.go

@@ -77,11 +77,9 @@ func TrackSubnetWithLocalMachine(
 		return fmt.Errorf("unknown vm: %s", sc.VM)
 	}
 	rootDir := app.GetLocalDir(clusterName)
+
 	pluginPath := filepath.Join(rootDir, "node1", "plugins", vmID.String())
-	if err := utils.FileCopy(vmBin, pluginPath); err != nil {
-		return err
-	}
-	if err := os.Chmod(pluginPath, constants.DefaultPerms755); err != nil {
+	if err := utils.SetupExecFile(app.Log, vmBin, pluginPath); err != nil {
 		return err
 	}
 

pkg/utils/file.go

@@ -9,6 +9,9 @@ import (
 
 	"github.com/ava-labs/avalanche-cli/pkg/constants"
 	sdkutils "github.com/ava-labs/avalanche-cli/sdk/utils"
+	"github.com/ava-labs/avalanchego/utils/logging"
+
+	"go.uber.org/zap"
 	"golang.org/x/mod/modfile"
 )
 
@@ -38,7 +41,11 @@ func IsExecutable(filename string) bool {
 		return false
 	}
 	info, _ := os.Stat(filename)
-	return info.Mode()&0x0100 != 0
+	// A file perm can be seen as a 9-bit sequence mapping to: rwxrwxrwx
+	// read-write-execute for owner, group and everybody.
+	// 0o100 is the bit mask that, when applied with the bitwise-AND operator &,
+	// results in != 0 state whenever the file can be executed by the owner.
+	return info.Mode()&0o100 != 0
 }
 
 // UserHomePath returns the absolute path of a file located in the user's home directory.
@@ -77,6 +84,34 @@ func FileCopy(src string, dst string) error {
 	return os.WriteFile(dst, data, constants.WriteReadReadPerms)
 }
 
+// SetupExecFile copies a file into destination and set it to have exec perms,
+// if destination either does not exists, or is not executable
+func SetupExecFile(
+	log logging.Logger,
+	src string,
+	dst string,
+) error {
+	if !IsExecutable(dst) {
+		if FileExists(dst) {
+			log.Error(
+				"binary was not properly installed on a previous CLI execution",
+				zap.String("binary-path", dst),
+			)
+		}
+		// Either it was never installed, or it was partially done (copy or chmod
+		// failure)
+		// As the file is not executable, there is no risk of encountering text file busy
+		// error during copy, because that happens when the binary is being executed.
+		if err := FileCopy(src, dst); err != nil {
+			return err
+		}
+		if err := os.Chmod(dst, constants.DefaultPerms755); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // ReadFile reads a file and returns the contents as a string
 func ReadFile(filePath string) (string, error) {
 	filePath = ExpandHome(filePath)

pkg/utils/file_test.go

@@ -6,6 +6,11 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/ava-labs/avalanche-cli/pkg/constants"
+	"github.com/ava-labs/avalanchego/utils/logging"
+
+	"github.com/stretchr/testify/require"
 )
 
 func TestExpandHome(t *testing.T) {
@@ -45,29 +50,26 @@ func TestExpandHome(t *testing.T) {
 	}
 }
 
-// createTempGoMod creates a temporary go.mod file with the provided content.
-func createTempGoMod(t *testing.T, content string) string {
+// createTemp creates a temporary file with the provided name prefix and content.
+func createTemp(t *testing.T, namePrefix string, content string) string {
 	t.Helper()
-	file, err := os.CreateTemp("", "go.mod")
+	file, err := os.CreateTemp("", namePrefix)
 	if err != nil {
 		t.Fatal(err)
 	}
-
 	if _, err := file.Write([]byte(content)); err != nil {
 		t.Fatal(err)
 	}
-
 	if err := file.Close(); err != nil {
 		t.Fatal(err)
 	}
-
 	return file.Name()
 }
 
 // TestReadGoVersion tests all scenarios in one function using sub-tests.
 func TestReadGoVersion(t *testing.T) {
 	t.Run("Success", func(t *testing.T) {
-		tempFile := createTempGoMod(t, "module example.com/test\n\ngo 1.23\n")
+		tempFile := createTemp(t, "go.mod", "module example.com/test\n\ngo 1.23\n")
 		defer os.Remove(tempFile) // Clean up the temp file
 
 		version, err := ReadGoVersion(tempFile)
@@ -82,7 +84,7 @@ func TestReadGoVersion(t *testing.T) {
 	})
 
 	t.Run("NoVersion", func(t *testing.T) {
-		tempFile := createTempGoMod(t, "module example.com/test\n")
+		tempFile := createTemp(t, "go.mod", "module example.com/test\n")
 		defer os.Remove(tempFile)
 
 		_, err := ReadGoVersion(tempFile)
@@ -98,3 +100,67 @@ func TestReadGoVersion(t *testing.T) {
 		}
 	})
 }
+
+func TestSetupExecFile(t *testing.T) {
+	srcContent := "src content"
+	destContent := "dest content"
+	t.Run("Src does not exists", func(t *testing.T) {
+		src := createTemp(t, "testexecfile", srcContent)
+		dest := createTemp(t, "testexecfile", destContent)
+		err := os.Remove(src)
+		require.NoError(t, err)
+		require.Equal(t, false, FileExists(src))
+		require.Equal(t, true, FileExists(dest))
+		require.Equal(t, false, IsExecutable(dest))
+		err = SetupExecFile(logging.NoLog{}, src, dest)
+		require.Error(t, err)
+		content, err := os.ReadFile(dest)
+		require.NoError(t, err)
+		require.Equal(t, true, FileExists(dest))
+		require.Equal(t, false, IsExecutable(dest))
+		require.Equal(t, destContent, string(content))
+	})
+	t.Run("Dest does not exists", func(t *testing.T) {
+		src := createTemp(t, "testexecfile", srcContent)
+		dest := createTemp(t, "testexecfile", destContent)
+		err := os.Remove(dest)
+		require.NoError(t, err)
+		require.Equal(t, false, FileExists(dest))
+		require.Equal(t, false, IsExecutable(dest))
+		err = SetupExecFile(logging.NoLog{}, src, dest)
+		require.NoError(t, err)
+		content, err := os.ReadFile(dest)
+		require.NoError(t, err)
+		require.Equal(t, true, FileExists(dest))
+		require.Equal(t, true, IsExecutable(dest))
+		require.Equal(t, srcContent, string(content))
+	})
+	t.Run("Dest is not executable", func(t *testing.T) {
+		src := createTemp(t, "testexecfile", srcContent)
+		dest := createTemp(t, "testexecfile", destContent)
+		require.Equal(t, true, FileExists(dest))
+		require.Equal(t, false, IsExecutable(dest))
+		err := SetupExecFile(logging.NoLog{}, src, dest)
+		require.NoError(t, err)
+		content, err := os.ReadFile(dest)
+		require.NoError(t, err)
+		require.Equal(t, true, FileExists(dest))
+		require.Equal(t, true, IsExecutable(dest))
+		require.Equal(t, srcContent, string(content))
+	})
+	t.Run("Dest is already executable", func(t *testing.T) {
+		src := createTemp(t, "testexecfile", srcContent)
+		dest := createTemp(t, "testexecfile", destContent)
+		err := os.Chmod(dest, constants.DefaultPerms755)
+		require.NoError(t, err)
+		require.Equal(t, true, FileExists(dest))
+		require.Equal(t, true, IsExecutable(dest))
+		err = SetupExecFile(logging.NoLog{}, src, dest)
+		require.NoError(t, err)
+		content, err := os.ReadFile(dest)
+		require.NoError(t, err)
+		require.Equal(t, true, FileExists(dest))
+		require.Equal(t, true, IsExecutable(dest))
+		require.Equal(t, destContent, string(content))
+	})
+}