From 91193925c019ccb33752d865925d32d941a63bdb Mon Sep 17 00:00:00 2001 From: Felipe Madero Date: Thu, 19 Dec 2024 23:54:17 -0300 Subject: [PATCH 1/4] avoid text file busy error on track --- cmd/networkcmd/helpers.go | 12 +++++++----- pkg/node/local.go | 13 ++++++++----- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/cmd/networkcmd/helpers.go b/cmd/networkcmd/helpers.go index 7cb366a9e..e091727e7 100644 --- a/cmd/networkcmd/helpers.go +++ b/cmd/networkcmd/helpers.go @@ -63,11 +63,13 @@ func TrackSubnet( } pluginPath := filepath.Join(app.GetPluginsDir(), vmID.String()) - if err := utils.FileCopy(vmBin, pluginPath); err != nil { - return err - } - if err := os.Chmod(pluginPath, constants.DefaultPerms755); err != nil { - return err + if !utils.IsExecutable(pluginPath) { + if err := utils.FileCopy(vmBin, pluginPath); err != nil { + return err + } + if err := os.Chmod(pluginPath, constants.DefaultPerms755); err != nil { + return err + } } cli, err := binutils.NewGRPCClientWithEndpoint( diff --git a/pkg/node/local.go b/pkg/node/local.go index d4238af98..2032a35af 100644 --- a/pkg/node/local.go +++ b/pkg/node/local.go @@ -78,12 +78,15 @@ 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 { - return err + if !utils.IsExecutable(pluginPath) { + if err := utils.FileCopy(vmBin, pluginPath); err != nil { + return err + } + if err := os.Chmod(pluginPath, constants.DefaultPerms755); err != nil { + return err + } } cli, err := binutils.NewGRPCClientWithEndpoint( From 9116cf87483d87eb87239592e538729350deff58 Mon Sep 17 00:00:00 2001 From: Felipe Madero Date: Sat, 18 Jan 2025 11:22:29 -0300 Subject: [PATCH 2/4] moved exec setup to utils and added unit --- cmd/networkcmd/helpers.go | 9 +---- pkg/node/local.go | 9 +---- pkg/utils/file.go | 20 +++++++++- pkg/utils/file_test.go | 80 +++++++++++++++++++++++++++++++++++---- 4 files changed, 95 insertions(+), 23 deletions(-) diff --git a/cmd/networkcmd/helpers.go b/cmd/networkcmd/helpers.go index ae6f3f179..df09a5ac4 100644 --- a/cmd/networkcmd/helpers.go +++ b/cmd/networkcmd/helpers.go @@ -63,13 +63,8 @@ func TrackSubnet( } pluginPath := filepath.Join(app.GetPluginsDir(), vmID.String()) - if !utils.IsExecutable(pluginPath) { - if err := utils.FileCopy(vmBin, pluginPath); err != nil { - return err - } - if err := os.Chmod(pluginPath, constants.DefaultPerms755); err != nil { - return err - } + if err := utils.SetupExecFile(vmBin, pluginPath); err != nil { + return err } cli, err := binutils.NewGRPCClientWithEndpoint( diff --git a/pkg/node/local.go b/pkg/node/local.go index 46b40d185..541cd7cbf 100644 --- a/pkg/node/local.go +++ b/pkg/node/local.go @@ -80,13 +80,8 @@ func TrackSubnetWithLocalMachine( rootDir := app.GetLocalDir(clusterName) pluginPath := filepath.Join(rootDir, "node1", "plugins", vmID.String()) - if !utils.IsExecutable(pluginPath) { - if err := utils.FileCopy(vmBin, pluginPath); err != nil { - return err - } - if err := os.Chmod(pluginPath, constants.DefaultPerms755); err != nil { - return err - } + if err := utils.SetupExecFile(vmBin, pluginPath); err != nil { + return err } cli, err := binutils.NewGRPCClientWithEndpoint( diff --git a/pkg/utils/file.go b/pkg/utils/file.go index a3521d92a..8315daeff 100644 --- a/pkg/utils/file.go +++ b/pkg/utils/file.go @@ -38,7 +38,7 @@ func IsExecutable(filename string) bool { return false } info, _ := os.Stat(filename) - return info.Mode()&0x0100 != 0 + return info.Mode()&0100 != 0 } // UserHomePath returns the absolute path of a file located in the user's home directory. @@ -77,6 +77,24 @@ 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(src string, dst string) error { + if !IsExecutable(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) diff --git a/pkg/utils/file_test.go b/pkg/utils/file_test.go index 04b74afaa..335364bbf 100644 --- a/pkg/utils/file_test.go +++ b/pkg/utils/file_test.go @@ -6,6 +6,9 @@ import ( "os" "path/filepath" "testing" + + "github.com/ava-labs/avalanche-cli/pkg/constants" + "github.com/stretchr/testify/require" ) func TestExpandHome(t *testing.T) { @@ -45,29 +48,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 +82,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 +98,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(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(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(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(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)) + }) +} From 7f33e6d639d44a71aed5cd6550c78544cb89a84c Mon Sep 17 00:00:00 2001 From: Felipe Madero Date: Sat, 18 Jan 2025 11:25:56 -0300 Subject: [PATCH 3/4] lint --- pkg/utils/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/utils/file.go b/pkg/utils/file.go index 8315daeff..827ae654a 100644 --- a/pkg/utils/file.go +++ b/pkg/utils/file.go @@ -38,7 +38,7 @@ func IsExecutable(filename string) bool { return false } info, _ := os.Stat(filename) - return info.Mode()&0100 != 0 + return info.Mode()&0x0100 != 0 } // UserHomePath returns the absolute path of a file located in the user's home directory. From 375199948831827a15062de6b6c613c1da956bbc Mon Sep 17 00:00:00 2001 From: Felipe Madero Date: Sat, 18 Jan 2025 12:10:46 -0300 Subject: [PATCH 4/4] fix --- pkg/utils/file.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/utils/file.go b/pkg/utils/file.go index 827ae654a..7742f7bc3 100644 --- a/pkg/utils/file.go +++ b/pkg/utils/file.go @@ -38,7 +38,7 @@ func IsExecutable(filename string) bool { return false } info, _ := os.Stat(filename) - return info.Mode()&0x0100 != 0 + return info.Mode()&0o0100 != 0 } // UserHomePath returns the absolute path of a file located in the user's home directory.