diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 48f296b82..56e4ba1b4 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -18,6 +18,8 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi - Go API +- [libs/os] Kill() and {Must,}{Read,Write}File() functions have been removed. (@alessio) + - Blockchain Protocol ### FEATURES diff --git a/config/toml.go b/config/toml.go index 0dce3e6dc..19973a3fb 100644 --- a/config/toml.go +++ b/config/toml.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "io/ioutil" + "os" "path/filepath" "strings" "text/template" @@ -63,7 +64,7 @@ func WriteConfigFile(configFilePath string, config *Config) { panic(err) } - tmos.MustWriteFile(configFilePath, buffer.Bytes(), 0644) + mustWriteFile(configFilePath, buffer.Bytes(), 0644) } // Note: any changes to the comments/variables/mapstructure @@ -492,16 +493,22 @@ func ResetTestRootWithChainID(testName string, chainID string) *Config { chainID = "tendermint_test" } testGenesis := fmt.Sprintf(testGenesisFmt, chainID) - tmos.MustWriteFile(genesisFilePath, []byte(testGenesis), 0644) + mustWriteFile(genesisFilePath, []byte(testGenesis), 0644) } // we always overwrite the priv val - tmos.MustWriteFile(privKeyFilePath, []byte(testPrivValidatorKey), 0644) - tmos.MustWriteFile(privStateFilePath, []byte(testPrivValidatorState), 0644) + mustWriteFile(privKeyFilePath, []byte(testPrivValidatorKey), 0644) + mustWriteFile(privStateFilePath, []byte(testPrivValidatorState), 0644) config := TestConfig().SetRoot(rootDir) return config } +func mustWriteFile(filePath string, contents []byte, mode os.FileMode) { + if err := ioutil.WriteFile(filePath, contents, mode); err != nil { + tmos.Exit(fmt.Sprintf("failed to write file: %v", err)) + } +} + var testGenesisFmt = `{ "genesis_time": "2018-10-10T08:20:13.695936996Z", "chain_id": "%s", diff --git a/libs/autofile/autofile_test.go b/libs/autofile/autofile_test.go index 3e3814d5e..c2442a56f 100644 --- a/libs/autofile/autofile_test.go +++ b/libs/autofile/autofile_test.go @@ -10,8 +10,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - - tmos "github.com/tendermint/tendermint/libs/os" ) func TestSIGHUP(t *testing.T) { @@ -27,10 +25,9 @@ func TestSIGHUP(t *testing.T) { dir, err := ioutil.TempDir("", "sighup_test") require.NoError(t, err) t.Cleanup(func() { - os.RemoveAll(dir) + _ = os.RemoveAll(dir) }) - err = os.Chdir(dir) - require.NoError(t, err) + require.NoError(t, os.Chdir(dir)) // Create an AutoFile in the temporary directory name := "sighup_test" @@ -45,19 +42,16 @@ func TestSIGHUP(t *testing.T) { require.NoError(t, err) // Move the file over - err = os.Rename(name, name+"_old") - require.NoError(t, err) + require.NoError(t, os.Rename(name, name+"_old")) // Move into a different temporary directory otherDir, err := ioutil.TempDir("", "sighup_test_other") require.NoError(t, err) - defer os.RemoveAll(otherDir) - err = os.Chdir(otherDir) - require.NoError(t, err) + t.Cleanup(func() { os.RemoveAll(otherDir) }) + require.NoError(t, os.Chdir(otherDir)) // Send SIGHUP to self. - err = syscall.Kill(syscall.Getpid(), syscall.SIGHUP) - require.NoError(t, err) + require.NoError(t, syscall.Kill(syscall.Getpid(), syscall.SIGHUP)) // Wait a bit... signals are not handled synchronously. time.Sleep(time.Millisecond * 10) @@ -67,14 +61,13 @@ func TestSIGHUP(t *testing.T) { require.NoError(t, err) _, err = af.Write([]byte("Line 4\n")) require.NoError(t, err) - err = af.Close() - require.NoError(t, err) + require.NoError(t, af.Close()) // Both files should exist - if body := tmos.MustReadFile(filepath.Join(dir, name+"_old")); string(body) != "Line 1\nLine 2\n" { + if body := mustReadFile(t, filepath.Join(dir, name+"_old")); string(body) != "Line 1\nLine 2\n" { t.Errorf("unexpected body %s", body) } - if body := tmos.MustReadFile(filepath.Join(dir, name)); string(body) != "Line 3\nLine 4\n" { + if body := mustReadFile(t, filepath.Join(dir, name)); string(body) != "Line 3\nLine 4\n" { t.Errorf("unexpected body %s", body) } @@ -115,8 +108,7 @@ func TestAutoFileSize(t *testing.T) { // First, create an AutoFile writing to a tempfile dir f, err := ioutil.TempFile("", "sighup_test") require.NoError(t, err) - err = f.Close() - require.NoError(t, err) + require.NoError(t, f.Close()) // Here is the actual AutoFile. af, err := OpenAutoFile(f.Name()) @@ -136,14 +128,19 @@ func TestAutoFileSize(t *testing.T) { require.NoError(t, err) // 3. Not existing file - err = af.Close() - require.NoError(t, err) - err = os.Remove(f.Name()) - require.NoError(t, err) + require.NoError(t, af.Close()) + require.NoError(t, os.Remove(f.Name())) size, err = af.Size() require.EqualValues(t, 0, size, "Expected a new file to be empty") require.NoError(t, err) // Cleanup - _ = os.Remove(f.Name()) + t.Cleanup(func() { os.Remove(f.Name()) }) +} + +func mustReadFile(t *testing.T, filePath string) []byte { + fileBytes, err := ioutil.ReadFile(filePath) + require.NoError(t, err) + + return fileBytes } diff --git a/libs/fail/fail.go b/libs/fail/fail.go index 38cec9a29..03a2ca668 100644 --- a/libs/fail/fail.go +++ b/libs/fail/fail.go @@ -32,16 +32,9 @@ func Fail() { } if callIndex == callIndexToFail { - Exit() + fmt.Printf("*** fail-test %d ***\n", callIndex) + os.Exit(1) } callIndex++ } - -func Exit() { - fmt.Printf("*** fail-test %d ***\n", callIndex) - os.Exit(1) - // proc, _ := os.FindProcess(os.Getpid()) - // proc.Signal(os.Interrupt) - // panic(fmt.Sprintf("*** fail-test %d ***", callIndex)) -} diff --git a/libs/os/os.go b/libs/os/os.go index c2f37992a..733f7e942 100644 --- a/libs/os/os.go +++ b/libs/os/os.go @@ -3,7 +3,6 @@ package os import ( "fmt" "io" - "io/ioutil" "os" "os/signal" "syscall" @@ -29,15 +28,6 @@ func TrapSignal(logger logger, cb func()) { }() } -// Kill the running process by sending itself SIGTERM. -func Kill() error { - p, err := os.FindProcess(os.Getpid()) - if err != nil { - return err - } - return p.Signal(syscall.SIGTERM) -} - func Exit(s string) { fmt.Printf(s + "\n") os.Exit(1) @@ -58,30 +48,6 @@ func FileExists(filePath string) bool { return !os.IsNotExist(err) } -func ReadFile(filePath string) ([]byte, error) { - return ioutil.ReadFile(filePath) -} - -func MustReadFile(filePath string) []byte { - fileBytes, err := ioutil.ReadFile(filePath) - if err != nil { - Exit(fmt.Sprintf("MustReadFile failed: %v", err)) - return nil - } - return fileBytes -} - -func WriteFile(filePath string, contents []byte, mode os.FileMode) error { - return ioutil.WriteFile(filePath, contents, mode) -} - -func MustWriteFile(filePath string, contents []byte, mode os.FileMode) { - err := WriteFile(filePath, contents, mode) - if err != nil { - Exit(fmt.Sprintf("MustWriteFile failed: %v", err)) - } -} - // CopyFile copies a file. It truncates the destination file if it exists. func CopyFile(src, dst string) error { info, err := os.Stat(src) diff --git a/libs/os/os_test.go b/libs/os/os_test.go index 70086afad..2b44dc8e7 100644 --- a/libs/os/os_test.go +++ b/libs/os/os_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "os/exec" + "syscall" "testing" "time" @@ -47,10 +48,7 @@ func TestTrapSignal(t *testing.T) { return } - cmd := exec.Command(os.Args[0], "-test.run="+t.Name()) - mockStderr := bytes.NewBufferString("") - cmd.Env = append(os.Environ(), "TM_TRAP_SIGNAL_TEST=1") - cmd.Stderr = mockStderr + cmd, _, mockStderr := newTestProgram(t, "TM_TRAP_SIGNAL_TEST") err := cmd.Run() if err == nil { @@ -67,7 +65,6 @@ func TestTrapSignal(t *testing.T) { } t.Fatal("this error should not be triggered") - } type mockLogger struct{} @@ -80,10 +77,26 @@ func killer() { tmos.TrapSignal(logger, func() { _, _ = fmt.Fprintf(os.Stderr, "exiting") }) time.Sleep(1 * time.Second) - // use Kill() to test SIGTERM - if err := tmos.Kill(); err != nil { + p, err := os.FindProcess(os.Getpid()) + if err != nil { + panic(err) + } + + if err := p.Signal(syscall.SIGTERM); err != nil { panic(err) } time.Sleep(1 * time.Second) } + +func newTestProgram(t *testing.T, environVar string) (cmd *exec.Cmd, stdout *bytes.Buffer, stderr *bytes.Buffer) { + t.Helper() + + cmd = exec.Command(os.Args[0], "-test.run="+t.Name()) + stdout, stderr = bytes.NewBufferString(""), bytes.NewBufferString("") + cmd.Env = append(os.Environ(), fmt.Sprintf("%s=1", environVar)) + cmd.Stdout = stdout + cmd.Stderr = stderr + + return +} diff --git a/p2p/conn/secret_connection_test.go b/p2p/conn/secret_connection_test.go index d4997d81c..e787e1348 100644 --- a/p2p/conn/secret_connection_test.go +++ b/p2p/conn/secret_connection_test.go @@ -6,6 +6,7 @@ import ( "flag" "fmt" "io" + "io/ioutil" "log" "os" "path/filepath" @@ -21,7 +22,6 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/crypto/sr25519" "github.com/tendermint/tendermint/libs/async" - tmos "github.com/tendermint/tendermint/libs/os" tmrand "github.com/tendermint/tendermint/libs/rand" ) @@ -228,8 +228,7 @@ func TestDeriveSecretsAndChallengeGolden(t *testing.T) { if *update { t.Logf("Updating golden test vector file %s", goldenFilepath) data := createGoldenTestVectors(t) - err := tmos.WriteFile(goldenFilepath, []byte(data), 0644) - require.NoError(t, err) + require.NoError(t, ioutil.WriteFile(goldenFilepath, []byte(data), 0644)) } f, err := os.Open(goldenFilepath) if err != nil { diff --git a/proxy/multi_app_conn.go b/proxy/multi_app_conn.go index a7a6f7014..369b685ba 100644 --- a/proxy/multi_app_conn.go +++ b/proxy/multi_app_conn.go @@ -2,10 +2,11 @@ package proxy import ( "fmt" + "os" + "syscall" abcicli "github.com/tendermint/tendermint/abci/client" tmlog "github.com/tendermint/tendermint/libs/log" - tmos "github.com/tendermint/tendermint/libs/os" "github.com/tendermint/tendermint/libs/service" ) @@ -129,8 +130,7 @@ func (app *multiAppConn) killTMOnClientError() { logger.Error( fmt.Sprintf("%s connection terminated. Did the application crash? Please restart tendermint", conn), "err", err) - killErr := tmos.Kill() - if killErr != nil { + if killErr := kill(); killErr != nil { logger.Error("Failed to kill this process - please do so manually", "err", killErr) } } @@ -189,3 +189,12 @@ func (app *multiAppConn) abciClientFor(conn string) (abcicli.Client, error) { } return c, nil } + +func kill() error { + p, err := os.FindProcess(os.Getpid()) + if err != nil { + return err + } + + return p.Signal(syscall.SIGTERM) +} diff --git a/types/genesis.go b/types/genesis.go index 20fc79721..964df923e 100644 --- a/types/genesis.go +++ b/types/genesis.go @@ -11,7 +11,6 @@ import ( "github.com/tendermint/tendermint/crypto" tmbytes "github.com/tendermint/tendermint/libs/bytes" tmjson "github.com/tendermint/tendermint/libs/json" - tmos "github.com/tendermint/tendermint/libs/os" tmproto "github.com/tendermint/tendermint/proto/tendermint/types" tmtime "github.com/tendermint/tendermint/types/time" ) @@ -52,7 +51,8 @@ func (genDoc *GenesisDoc) SaveAs(file string) error { if err != nil { return err } - return tmos.WriteFile(file, genDocBytes, 0644) + + return ioutil.WriteFile(file, genDocBytes, 0644) // nolint:gosec } // ValidatorHash returns the hash of the validator set contained in the GenesisDoc