address PR comments

Author: felipemadero

pkg/localnet/tmpnet.go

@@ -376,13 +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())
-	return utils.SetupExecFile(binaryPath, pluginPath)
+	return utils.SetupExecFile(log, binaryPath, pluginPath)
 }
 
 // Set up blockchain config for all nodes in the network
@@ -583,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

@@ -79,7 +79,7 @@ func TrackSubnetWithLocalMachine(
 	rootDir := app.GetLocalDir(clusterName)
 
 	pluginPath := filepath.Join(rootDir, "node1", "plugins", vmID.String())
-	if err := utils.SetupExecFile(vmBin, pluginPath); 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()&0o0100 != 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.
@@ -79,8 +86,18 @@ func FileCopy(src string, dst string) error {
 
 // 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(src string, dst string) error {
+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

pkg/utils/file_test.go

@@ -8,6 +8,8 @@ import (
 	"testing"
 
 	"github.com/ava-labs/avalanche-cli/pkg/constants"
+	"github.com/ava-labs/avalanchego/utils/logging"
+
 	"github.com/stretchr/testify/require"
 )
 
@@ -110,7 +112,7 @@ func TestSetupExecFile(t *testing.T) {
 		require.Equal(t, false, FileExists(src))
 		require.Equal(t, true, FileExists(dest))
 		require.Equal(t, false, IsExecutable(dest))
-		err = SetupExecFile(src, dest)
+		err = SetupExecFile(logging.NoLog{}, src, dest)
 		require.Error(t, err)
 		content, err := os.ReadFile(dest)
 		require.NoError(t, err)
@@ -125,7 +127,7 @@ func TestSetupExecFile(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, false, FileExists(dest))
 		require.Equal(t, false, IsExecutable(dest))
-		err = SetupExecFile(src, dest)
+		err = SetupExecFile(logging.NoLog{}, src, dest)
 		require.NoError(t, err)
 		content, err := os.ReadFile(dest)
 		require.NoError(t, err)
@@ -138,7 +140,7 @@ func TestSetupExecFile(t *testing.T) {
 		dest := createTemp(t, "testexecfile", destContent)
 		require.Equal(t, true, FileExists(dest))
 		require.Equal(t, false, IsExecutable(dest))
-		err := SetupExecFile(src, dest)
+		err := SetupExecFile(logging.NoLog{}, src, dest)
 		require.NoError(t, err)
 		content, err := os.ReadFile(dest)
 		require.NoError(t, err)
@@ -153,7 +155,7 @@ func TestSetupExecFile(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, true, FileExists(dest))
 		require.Equal(t, true, IsExecutable(dest))
-		err = SetupExecFile(src, dest)
+		err = SetupExecFile(logging.NoLog{}, src, dest)
 		require.NoError(t, err)
 		content, err := os.ReadFile(dest)
 		require.NoError(t, err)