Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

avoid text file busy error on track #2494

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 8 additions & 9 deletions pkg/localnet/tmpnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions pkg/node/local.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
37 changes: 36 additions & 1 deletion pkg/utils/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to error/log if the file isn't executable?

Copy link
Collaborator Author

@felipemadero felipemadero Feb 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to error. This also happens on first install.
We are not going to error on first install, and we prefer to try to
recover if a previous install failed.
Probably we can add log for a failed previous install, while trying to recover.

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)
Expand Down
82 changes: 74 additions & 8 deletions pkg/utils/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -98,3 +100,67 @@ func TestReadGoVersion(t *testing.T) {
}
})
}

func TestSetupExecFile(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love to see this test

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))
})
}
Loading