diff --git a/cmd/mutagen-agent/synchronizer.go b/cmd/mutagen-agent/synchronizer.go index e20f74915..1b75616aa 100644 --- a/cmd/mutagen-agent/synchronizer.go +++ b/cmd/mutagen-agent/synchronizer.go @@ -29,7 +29,7 @@ func housekeepRegularly(ctx context.Context, logger *logging.Logger) { // Perform an initial housekeeping operation since the ticker won't fire // straight away. logger.Info("Performing initial housekeeping") - housekeeping.Housekeep() + housekeeping.Housekeep(logger) // Create a ticker to regulate housekeeping and defer its shutdown. ticker := time.NewTicker(housekeepingInterval) @@ -42,7 +42,7 @@ func housekeepRegularly(ctx context.Context, logger *logging.Logger) { return case <-ticker.C: logger.Info("Performing regular housekeeping") - housekeeping.Housekeep() + housekeeping.Housekeep(logger) } } } diff --git a/cmd/mutagen/daemon/register.go b/cmd/mutagen/daemon/register.go index 50b8efb2c..f72da8e7f 100644 --- a/cmd/mutagen/daemon/register.go +++ b/cmd/mutagen/daemon/register.go @@ -1,6 +1,9 @@ package daemon import ( + "os" + + "github.com/mutagen-io/mutagen/pkg/logging" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -10,7 +13,8 @@ import ( // registerMain is the entry point for the register command. func registerMain(_ *cobra.Command, _ []string) error { - return daemon.Register() + logger := logging.NewLogger(logging.LevelError, os.Stderr) + return daemon.Register(logger) } // registerCommand is the register command. diff --git a/cmd/mutagen/daemon/run.go b/cmd/mutagen/daemon/run.go index 6917419fb..8ec8550f4 100644 --- a/cmd/mutagen/daemon/run.go +++ b/cmd/mutagen/daemon/run.go @@ -7,6 +7,7 @@ import ( "os" "os/signal" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "google.golang.org/grpc" @@ -27,18 +28,6 @@ import ( // runMain is the entry point for the run command. func runMain(_ *cobra.Command, _ []string) error { - // Attempt to acquire the daemon lock and defer its release. - lock, err := daemon.AcquireLock() - if err != nil { - return fmt.Errorf("unable to acquire daemon lock: %w", err) - } - defer lock.Release() - - // Create a channel to track termination signals. We do this before creating - // and starting other infrastructure so that we can ensure things terminate - // smoothly, not mid-initialization. - signalTermination := make(chan os.Signal, 1) - signal.Notify(signalTermination, cmd.TerminationSignals...) // Create the root logger. logLevel := logging.LevelInfo @@ -51,6 +40,19 @@ func runMain(_ *cobra.Command, _ []string) error { } logger := logging.NewLogger(logLevel, os.Stderr) + // Attempt to acquire the daemon lock and defer its release. + lock, err := daemon.AcquireLock(logger) + if err != nil { + return fmt.Errorf("unable to acquire daemon lock: %w", err) + } + defer must.Release(lock, logger) + + // Create a channel to track termination signals. We do this before creating + // and starting other infrastructure so that we can ensure things terminate + // smoothly, not mid-initialization. + signalTermination := make(chan os.Signal, 1) + signal.Notify(signalTermination, cmd.TerminationSignals...) + // Create a forwarding session manager and defer its shutdown. forwardingManager, err := forwarding.NewManager(logger.Sublogger("forward")) if err != nil { @@ -74,7 +76,7 @@ func runMain(_ *cobra.Command, _ []string) error { defer server.Stop() // Create the daemon server, defer its shutdown, and register it. - daemonServer := daemonsvc.NewServer() + daemonServer := daemonsvc.NewServer(logger) defer daemonServer.Shutdown() daemonsvc.RegisterDaemonServer(server, daemonServer) @@ -101,11 +103,11 @@ func runMain(_ *cobra.Command, _ []string) error { if err := os.Remove(endpoint); err != nil && !errors.Is(err, fs.ErrNotExist) { return fmt.Errorf("unable to remove existing daemon endpoint: %w", err) } - listener, err := ipc.NewListener(endpoint) + listener, err := ipc.NewListener(endpoint, logger) if err != nil { return fmt.Errorf("unable to create daemon listener: %w", err) } - defer listener.Close() + defer must.Close(listener, logger) // Serve incoming requests and watch for server failure. serverErrors := make(chan error, 1) diff --git a/cmd/mutagen/daemon/unregister.go b/cmd/mutagen/daemon/unregister.go index 1fac629ab..3cfaa01ee 100644 --- a/cmd/mutagen/daemon/unregister.go +++ b/cmd/mutagen/daemon/unregister.go @@ -1,16 +1,19 @@ package daemon import ( + "os" + "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" - "github.com/mutagen-io/mutagen/pkg/daemon" + "github.com/mutagen-io/mutagen/pkg/logging" ) // unregisterMain is the entry point for the unregister command. func unregisterMain(_ *cobra.Command, _ []string) error { - return daemon.Unregister() + logger := logging.NewLogger(logging.LevelError, os.Stderr) + return daemon.Unregister(logger) } // unregisterCommand is the unregister command. diff --git a/cmd/mutagen/project/flush.go b/cmd/mutagen/project/flush.go index 7c45dc2c4..85d475938 100644 --- a/cmd/mutagen/project/flush.go +++ b/cmd/mutagen/project/flush.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -22,6 +24,8 @@ import ( // flushMain is the entry point for the flush command. func flushMain(_ *cobra.Command, _ []string) error { + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // Compute the name of the configuration file and ensure that our working // directory is that in which the file resides. This is required for // relative paths (including relative synchronization paths and relative @@ -51,9 +55,9 @@ func flushMain(_ *cobra.Command, _ []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -70,12 +74,12 @@ func flushMain(_ *cobra.Command, _ []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the project identifier from the lock file. If the lock file is @@ -100,7 +104,7 @@ func flushMain(_ *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("unable to connect to daemon: %w", err) } - defer daemonConnection.Close() + defer must.Close(daemonConnection, logger) // Compute the selection that we're going to use to flush sessions. selection := &selection.Selection{ @@ -132,7 +136,7 @@ var flushConfiguration struct { // projectFile is the path to the project file, if non-default. projectFile string // skipWait indicates whether or not the flush operation should block until - // a synchronization cycle completes for each sesion requested. + // a synchronization cycle completes for each session requested. skipWait bool } diff --git a/cmd/mutagen/project/list.go b/cmd/mutagen/project/list.go index 7184f14bb..3cb588355 100644 --- a/cmd/mutagen/project/list.go +++ b/cmd/mutagen/project/list.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -23,6 +25,8 @@ import ( // listMain is the entry point for the list command. func listMain(_ *cobra.Command, _ []string) error { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Compute the name of the configuration file and ensure that our working // directory is that in which the file resides. This is required for // relative paths (including relative synchronization paths and relative @@ -52,9 +56,9 @@ func listMain(_ *cobra.Command, _ []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -71,12 +75,12 @@ func listMain(_ *cobra.Command, _ []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the project identifier from the lock file. If the lock file is @@ -101,7 +105,7 @@ func listMain(_ *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("unable to connect to daemon: %w", err) } - defer daemonConnection.Close() + defer must.Close(daemonConnection, logger) // Compute the selection that we're going to use to list sessions. selection := &selection.Selection{ diff --git a/cmd/mutagen/project/pause.go b/cmd/mutagen/project/pause.go index a875a970d..2fcecbed2 100644 --- a/cmd/mutagen/project/pause.go +++ b/cmd/mutagen/project/pause.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -23,6 +25,8 @@ import ( // pauseMain is the entry point for the pause command. func pauseMain(_ *cobra.Command, _ []string) error { + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // Compute the name of the configuration file and ensure that our working // directory is that in which the file resides. This is required for // relative paths (including relative synchronization paths and relative @@ -52,9 +56,9 @@ func pauseMain(_ *cobra.Command, _ []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -71,12 +75,12 @@ func pauseMain(_ *cobra.Command, _ []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the project identifier from the lock file. If the lock file is @@ -115,7 +119,7 @@ func pauseMain(_ *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("unable to connect to daemon: %w", err) } - defer daemonConnection.Close() + defer must.Close(daemonConnection, logger) // Compute the selection that we're going to use to pause sessions. selection := &selection.Selection{ diff --git a/cmd/mutagen/project/reset.go b/cmd/mutagen/project/reset.go index 526599456..f6972f6f3 100644 --- a/cmd/mutagen/project/reset.go +++ b/cmd/mutagen/project/reset.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -22,6 +24,7 @@ import ( // resetMain is the entry point for the reset command. func resetMain(_ *cobra.Command, _ []string) error { + logger := logging.NewLogger(logging.LevelError, os.Stderr) // Compute the name of the configuration file and ensure that our working // directory is that in which the file resides. This is required for // relative paths (including relative synchronization paths and relative @@ -51,9 +54,9 @@ func resetMain(_ *cobra.Command, _ []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -70,12 +73,12 @@ func resetMain(_ *cobra.Command, _ []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the project identifier from the lock file. If the lock file is @@ -100,7 +103,7 @@ func resetMain(_ *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("unable to connect to daemon: %w", err) } - defer daemonConnection.Close() + defer must.Close(daemonConnection, logger) // Compute the selection that we're going to use to reset sessions. selection := &selection.Selection{ diff --git a/cmd/mutagen/project/resume.go b/cmd/mutagen/project/resume.go index 6ed6ae401..3eeb00a47 100644 --- a/cmd/mutagen/project/resume.go +++ b/cmd/mutagen/project/resume.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -23,6 +25,8 @@ import ( // resumeMain is the entry point for the resume command. func resumeMain(_ *cobra.Command, _ []string) error { + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // Compute the name of the configuration file and ensure that our working // directory is that in which the file resides. This is required for // relative paths (including relative synchronization paths and relative @@ -52,9 +56,9 @@ func resumeMain(_ *cobra.Command, _ []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -71,12 +75,12 @@ func resumeMain(_ *cobra.Command, _ []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the project identifier from the lock file. If the lock file is diff --git a/cmd/mutagen/project/run.go b/cmd/mutagen/project/run.go index f8ef7f74d..7d8ca3862 100644 --- a/cmd/mutagen/project/run.go +++ b/cmd/mutagen/project/run.go @@ -12,11 +12,15 @@ import ( "github.com/mutagen-io/mutagen/pkg/filesystem/locking" "github.com/mutagen-io/mutagen/pkg/identifier" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/project" ) // runMain is the entry point for the run command. func runMain(_ *cobra.Command, arguments []string) error { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Validate arguments. var commandName string if len(arguments) == 0 { @@ -56,9 +60,9 @@ func runMain(_ *cobra.Command, arguments []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -75,12 +79,12 @@ func runMain(_ *cobra.Command, arguments []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the project identifier from the lock file. If the lock file is diff --git a/cmd/mutagen/project/start.go b/cmd/mutagen/project/start.go index 99544fcb8..1897d5bdd 100644 --- a/cmd/mutagen/project/start.go +++ b/cmd/mutagen/project/start.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -29,6 +31,8 @@ import ( // startMain is the entry point for the start command. func startMain(_ *cobra.Command, _ []string) error { + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // Compute the name of the configuration file and ensure that our working // directory is that in which the file resides. This is required for // relative paths (including relative synchronization paths and relative @@ -58,9 +62,9 @@ func startMain(_ *cobra.Command, _ []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -77,12 +81,12 @@ func startMain(_ *cobra.Command, _ []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the full contents of the lock file and ensure that it's empty. @@ -354,7 +358,7 @@ func startMain(_ *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("unable to connect to daemon: %w", err) } - defer daemonConnection.Close() + defer must.Close(daemonConnection, logger) // At this point, we're going to try to create resources, so we need to // maintain the lock file in case even some of them are successful. diff --git a/cmd/mutagen/project/terminate.go b/cmd/mutagen/project/terminate.go index 072372d22..8fdf24a21 100644 --- a/cmd/mutagen/project/terminate.go +++ b/cmd/mutagen/project/terminate.go @@ -8,6 +8,8 @@ import ( "path/filepath" "runtime" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/mutagen-io/mutagen/cmd" @@ -23,6 +25,8 @@ import ( // terminateMain is the entry point for the terminate command. func terminateMain(_ *cobra.Command, _ []string) error { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Compute the name of the configuration file and ensure that our working // directory is that in which the file resides. This is required for // relative paths (including relative synchronization paths and relative @@ -52,9 +56,9 @@ func terminateMain(_ *cobra.Command, _ []string) error { return fmt.Errorf("unable to create project locker: %w", err) } defer func() { - locker.Close() + must.Close(locker, logger) if removeLockFileOnReturn && runtime.GOOS == "windows" { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } }() @@ -71,12 +75,12 @@ func terminateMain(_ *cobra.Command, _ []string) error { defer func() { if removeLockFileOnReturn { if runtime.GOOS == "windows" { - locker.Truncate(0) + must.Truncate(locker, 0, logger) } else { - os.Remove(lockPath) + must.OSRemove(lockPath, logger) } } - locker.Unlock() + must.Unlock(locker, logger) }() // Read the project identifier from the lock file. If the lock file is @@ -115,7 +119,7 @@ func terminateMain(_ *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("unable to connect to daemon: %w", err) } - defer daemonConnection.Close() + defer must.Close(daemonConnection, logger) // Compute the selection that we're going to use to terminate sessions. selection := &selection.Selection{ diff --git a/cmd/mutagen/sync/create.go b/cmd/mutagen/sync/create.go index 2ac6fb22e..94b1b87e5 100644 --- a/cmd/mutagen/sync/create.go +++ b/cmd/mutagen/sync/create.go @@ -7,6 +7,8 @@ import ( "os" "strings" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" "github.com/spf13/pflag" @@ -96,6 +98,11 @@ func CreateWithSpecification( // createMain is the entry point for the create command. func createMain(_ *cobra.Command, arguments []string) error { + + // Set up a logger on the standard error stream. + // TODO: Make this configurable + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // Validate, extract, and parse URLs. if len(arguments) != 2 { return errors.New("invalid number of endpoint URLs provided") @@ -501,7 +508,7 @@ func createMain(_ *cobra.Command, arguments []string) error { if err != nil { return fmt.Errorf("unable to connect to daemon: %w", err) } - defer daemonConnection.Close() + defer must.Close(daemonConnection, logger) // Perform the create operation. identifier, err := CreateWithSpecification(daemonConnection, specification) diff --git a/cmd/mutagen/sync/main.go b/cmd/mutagen/sync/main.go index 65649155e..1c28620f8 100644 --- a/cmd/mutagen/sync/main.go +++ b/cmd/mutagen/sync/main.go @@ -1,16 +1,22 @@ package sync import ( + "os" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/spf13/cobra" ) // syncMain is the entry point for the sync command. func syncMain(command *cobra.Command, arguments []string) error { + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // If no commands were given, then print help information and bail. We don't // have to worry about warning about arguments being present here (which // would be incorrect usage) because arguments can't even reach this point // (they will be mistaken for subcommands and a error will be displayed). - command.Help() + must.CommandHelp(command, logger) // Success. return nil diff --git a/pkg/agent/bundle.go b/pkg/agent/bundle.go index f5ffe4bb9..737c1f717 100644 --- a/pkg/agent/bundle.go +++ b/pkg/agent/bundle.go @@ -10,6 +10,8 @@ import ( "runtime" "github.com/klauspost/compress/gzip" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/filesystem" "github.com/mutagen-io/mutagen/pkg/mutagen" @@ -51,7 +53,7 @@ var ExpectedBundleLocation BundleLocation // and will have the executability bit set if it makes sense. The path to the // extracted file will be returned, and the caller is responsible for cleaning // up the file if this function returns a nil error. -func ExecutableForPlatform(goos, goarch, outputPath string) (string, error) { +func ExecutableForPlatform(goos, goarch, outputPath string, logger *logging.Logger) (string, error) { // Compute the path to the location in which we expect to find the agent // bundle. var bundleSearchPaths []string @@ -89,26 +91,27 @@ func ExecutableForPlatform(goos, goarch, outputPath string) (string, error) { } return "", fmt.Errorf("unable to open agent bundle (%s): %w", bundlePath, err) } else if metadata, err := file.Stat(); err != nil { - file.Close() + must.Close(file, logger) return "", fmt.Errorf("unable to access agent bundle (%s) file metadata: %w", bundlePath, err) } else if metadata.Mode()&os.ModeType != 0 { - file.Close() + must.Close(file, logger) return "", fmt.Errorf("agent bundle (%s) is not a file", bundlePath) } else { bundle = file - defer bundle.Close() + break } } if bundle == nil { return "", fmt.Errorf("unable to locate agent bundle (search paths: %v)", bundleSearchPaths) } + defer must.Close(bundle, logger) // Create a decompressor and defer its closure. bundleDecompressor, err := gzip.NewReader(bundle) if err != nil { return "", fmt.Errorf("unable to decompress agent bundle: %w", err) } - defer bundleDecompressor.Close() + defer must.Close(bundleDecompressor, logger) // Create an archive reader. bundleArchive := tar.NewReader(bundleDecompressor) @@ -146,8 +149,8 @@ func ExecutableForPlatform(goos, goarch, outputPath string) (string, error) { // Copy data into the file. if _, err := io.CopyN(file, bundleArchive, header.Size); err != nil { - file.Close() - os.Remove(file.Name()) + must.Close(file, logger) + must.OSRemove(file.Name(), logger) return "", fmt.Errorf("unable to copy agent data: %w", err) } @@ -159,15 +162,15 @@ func ExecutableForPlatform(goos, goarch, outputPath string) (string, error) { // to use on Windows in any scenario (where executability bits don't exist). if runtime.GOOS != "windows" && goos != "windows" { if err := file.Chmod(0700); err != nil { - file.Close() - os.Remove(file.Name()) + must.Close(file, logger) + must.OSRemove(file.Name(), logger) return "", fmt.Errorf("unable to make agent executable: %w", err) } } // Close the file. if err := file.Close(); err != nil { - os.Remove(file.Name()) + must.OSRemove(file.Name(), logger) return "", fmt.Errorf("unable to close temporary file: %w", err) } diff --git a/pkg/agent/bundle_test.go b/pkg/agent/bundle_test.go index 83442c4e5..291577bfb 100644 --- a/pkg/agent/bundle_test.go +++ b/pkg/agent/bundle_test.go @@ -1,16 +1,20 @@ package agent import ( + "bytes" "os" "path/filepath" "runtime" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" ) // TestExecutableForInvalidOS tests that ExecutableForPlatform fails for an // invalid OS specification. func TestExecutableForInvalidOS(t *testing.T) { - if _, err := ExecutableForPlatform("fakeos", runtime.GOARCH, ""); err == nil { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + if _, err := ExecutableForPlatform("fakeos", runtime.GOARCH, "", logger); err == nil { t.Fatal("extracting agent executable succeeded for invalid OS") } } @@ -18,7 +22,8 @@ func TestExecutableForInvalidOS(t *testing.T) { // TestExecutableForInvalidArchitecture tests that ExecutableForPlatform fails // for an invalid architecture specification. func TestExecutableForInvalidArchitecture(t *testing.T) { - if _, err := ExecutableForPlatform(runtime.GOOS, "fakearch", ""); err == nil { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + if _, err := ExecutableForPlatform(runtime.GOOS, "fakearch", "", logger); err == nil { t.Fatal("extracting agent executable succeeded for invalid architecture") } } @@ -26,7 +31,8 @@ func TestExecutableForInvalidArchitecture(t *testing.T) { // TestExecutableForInvalidPair tests that ExecutableForPlatform fails for an // invalid OS/architecture specification. func TestExecutableForInvalidPair(t *testing.T) { - if _, err := ExecutableForPlatform("fakeos", "fakearch", ""); err == nil { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + if _, err := ExecutableForPlatform("fakeos", "fakearch", "", logger); err == nil { t.Fatal("extracting agent executable succeeded for invalid architecture") } } @@ -34,7 +40,8 @@ func TestExecutableForInvalidPair(t *testing.T) { // TestExecutableForPlatform tests that ExecutableForPlatform succeeds for the // current OS/architecture. func TestExecutableForPlatform(t *testing.T) { - if executable, err := ExecutableForPlatform(runtime.GOOS, runtime.GOARCH, ""); err != nil { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + if executable, err := ExecutableForPlatform(runtime.GOOS, runtime.GOARCH, "", logger); err != nil { t.Fatal("unable to extract agent bundle for current platform:", err) } else if err = os.Remove(executable); err != nil { t.Error("unable to remove agent executable:", err) @@ -44,11 +51,12 @@ func TestExecutableForPlatform(t *testing.T) { // TestExecutableForPlatformWithOutputPath tests that ExecutableForPlatform // functions correctly when an output path is specified. func TestExecutableForPlatformWithOutputPath(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) // Compute the output path. outputPath := filepath.Join(t.TempDir(), "agent_output") // Perform executable extraction. - executable, err := ExecutableForPlatform(runtime.GOOS, runtime.GOARCH, outputPath) + executable, err := ExecutableForPlatform(runtime.GOOS, runtime.GOARCH, outputPath, logger) if err != nil { t.Fatal("unable to extract agent bundle for current platform:", err) } diff --git a/pkg/agent/dial.go b/pkg/agent/dial.go index 62b80b556..98b8515fc 100644 --- a/pkg/agent/dial.go +++ b/pkg/agent/dial.go @@ -12,6 +12,7 @@ import ( transportpkg "github.com/mutagen-io/mutagen/pkg/agent/transport" "github.com/mutagen-io/mutagen/pkg/filesystem" "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/mutagen" "github.com/mutagen-io/mutagen/pkg/platform/terminal" "github.com/mutagen-io/mutagen/pkg/prompting" @@ -108,7 +109,7 @@ func connect(logger *logging.Logger, transport Transport, mode, prompter string, // exit with its natural exit code (instead of an exit code due to forced // termination) and will be able to yield some error output for diagnosing // the issue. - stream, err := transportpkg.NewStream(agentProcess, errorTee) + stream, err := transportpkg.NewStream(agentProcess, errorTee, logger) if err != nil { return nil, false, false, fmt.Errorf("unable to create agent process stream: %w", err) } @@ -128,7 +129,7 @@ func connect(logger *logging.Logger, transport Transport, mode, prompter string, // we don't want to check it, but transport.Stream guarantees that if // Close returns, then the underlying process has fully terminated, // which is all we care about. - stream.Close() + must.Close(stream, logger) // Extract any error output, ensure that it's UTF-8, strip out any // whitespace (primarily trailing newlines), and neutralize any control @@ -168,7 +169,7 @@ func connect(logger *logging.Logger, transport Transport, mode, prompter string, // Perform a version handshake. if err := mutagen.ClientVersionHandshake(stream); err != nil { - stream.Close() + must.Close(stream, logger) return nil, false, false, fmt.Errorf("version handshake error: %w", err) } diff --git a/pkg/agent/install.go b/pkg/agent/install.go index 5245eb6a2..486eab7c2 100644 --- a/pkg/agent/install.go +++ b/pkg/agent/install.go @@ -6,6 +6,7 @@ import ( "runtime" "github.com/google/uuid" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/filesystem" "github.com/mutagen-io/mutagen/pkg/logging" @@ -50,11 +51,11 @@ func install(logger *logging.Logger, transport Transport, prompter string) error if err := prompting.Message(prompter, "Extracting agent..."); err != nil { return fmt.Errorf("unable to message prompter: %w", err) } - agentExecutable, err := ExecutableForPlatform(goos, goarch, "") + agentExecutable, err := ExecutableForPlatform(goos, goarch, "", logger) if err != nil { return fmt.Errorf("unable to get agent for platform: %w", err) } - defer os.Remove(agentExecutable) + defer must.OSRemove(agentExecutable, logger) // Copy the agent to the remote. We use a unique identifier for the // temporary destination. For Windows remotes, we add a ".exe" suffix, which diff --git a/pkg/agent/transport/ssh/transport_test.go b/pkg/agent/transport/ssh/transport_test.go index 3109f8b05..06be64bc9 100644 --- a/pkg/agent/transport/ssh/transport_test.go +++ b/pkg/agent/transport/ssh/transport_test.go @@ -1,6 +1,7 @@ package ssh import ( + "bytes" "os" "os/user" "path/filepath" @@ -10,6 +11,7 @@ import ( "unicode/utf8" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" ) func TestCopy(t *testing.T) { @@ -17,6 +19,7 @@ func TestCopy(t *testing.T) { if os.Getenv("MUTAGEN_TEST_SSH") != "true" { t.Skip() } + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) // Compute source path. source := filepath.Join(t.TempDir(), "source") @@ -25,7 +28,7 @@ func TestCopy(t *testing.T) { contents := []byte{0, 1, 2, 3, 4, 5, 6} // Attempt to write to a temporary file. - if err := filesystem.WriteFileAtomic(source, contents, 0600); err != nil { + if err := filesystem.WriteFileAtomic(source, contents, 0600, logger); err != nil { t.Fatal("atomic file write failed:", err) } @@ -96,12 +99,12 @@ func TestCommandOutput(t *testing.T) { // TODO: Should we also verify that an extracted HOME/USERPROFILE value // matches the expected home directory since we've already queried the user? if command, err := transport.Command(command); err != nil { - t.Fatal("unable to create command:", err) + t.Fatal("unable to create command:", command, err) } else if output, err := command.Output(); err != nil { - t.Fatal("unable to run command:", err) + t.Fatal("unable to run command:", output, err) } else if !strings.Contains(string(output), content) { - t.Error("output does not contain expected content") + t.Error("output does not contain expected content, got:'", string(output), "', Expected: '", content, "'") } else if !utf8.Valid(output) { - t.Error("output not in UTF-8 encoding") + t.Error("output not in UTF-8 encoding: ", string(output)) } } diff --git a/pkg/agent/transport/stream.go b/pkg/agent/transport/stream.go index cb1f6af87..fccc5a5b9 100644 --- a/pkg/agent/transport/stream.go +++ b/pkg/agent/transport/stream.go @@ -8,6 +8,9 @@ import ( "sync" "syscall" "time" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // Stream implements io.ReadWriteCloser using the standard input and output @@ -16,6 +19,7 @@ import ( // guarantees that its Close method unblocks pending Read and Write calls. It // also provides optional forwarding of the process' standard error stream. type Stream struct { + logger *logging.Logger // process is the underlying process. process *exec.Cmd // standardInput is the process' standard input stream. @@ -34,7 +38,7 @@ type Stream struct { // other I/O redirection should be performed for the process. If this function // fails, the command should be considered unusable. If standardErrorReceiver is // non-nil, then the process' standard error output will be forwarded to it. -func NewStream(process *exec.Cmd, standardErrorReceiver io.Writer) (*Stream, error) { +func NewStream(process *exec.Cmd, standardErrorReceiver io.Writer, logger *logging.Logger) (*Stream, error) { // Create a pipe to the process' standard input stream. standardInput, err := process.StdinPipe() if err != nil { @@ -44,7 +48,7 @@ func NewStream(process *exec.Cmd, standardErrorReceiver io.Writer) (*Stream, err // Create a pipe from the process' standard output stream. standardOutput, err := process.StdoutPipe() if err != nil { - standardInput.Close() + must.Close(standardInput, logger) return nil, fmt.Errorf("unable to redirect process output: %w", err) } @@ -57,13 +61,13 @@ func NewStream(process *exec.Cmd, standardErrorReceiver io.Writer) (*Stream, err if standardErrorReceiver != nil { standardError, err := process.StderrPipe() if err != nil { - standardInput.Close() - standardOutput.Close() + must.Close(standardInput, logger) + must.Close(standardOutput, logger) return nil, fmt.Errorf("unable to redirect process error output: %w", err) } go func() { - io.Copy(standardErrorReceiver, standardError) - standardError.Close() + must.IOCopy(standardErrorReceiver, standardError, logger) + must.Close(standardError, logger) }() } @@ -72,6 +76,7 @@ func NewStream(process *exec.Cmd, standardErrorReceiver io.Writer) (*Stream, err process: process, standardInput: standardInput, standardOutput: standardOutput, + logger: logger, }, nil } @@ -192,7 +197,7 @@ func (s *Stream) Close() error { // If this is a POSIX system, then send SIGTERM to the process and wait up // to one second for it to terminate on its own. if runtime.GOOS != "windows" { - s.process.Process.Signal(syscall.SIGTERM) + must.Signal(s.process.Process, syscall.SIGTERM, s.logger) waitTimer.Reset(time.Second) select { case err := <-waitResults: @@ -204,6 +209,6 @@ func (s *Stream) Close() error { // Kill the process (via SIGKILL on POSIX and TerminateProcess on Windows) // and wait for it to exit. - s.process.Process.Kill() + must.Kill(s.process.Process, s.logger) return <-waitResults } diff --git a/pkg/api/models/forwarding/configuration_test.go b/pkg/api/models/forwarding/configuration_test.go index 8e201650e..9444ee37f 100644 --- a/pkg/api/models/forwarding/configuration_test.go +++ b/pkg/api/models/forwarding/configuration_test.go @@ -1,11 +1,14 @@ package forwarding import ( + "bytes" "os" "testing" "github.com/mutagen-io/mutagen/pkg/encoding" "github.com/mutagen-io/mutagen/pkg/forwarding" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) const ( @@ -29,6 +32,8 @@ var expectedConfiguration = &forwarding.Configuration{ // TestLoadConfiguration tests loading a YAML-based session configuration. func TestLoadConfiguration(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Write a valid configuration to a temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_configuration") if err != nil { @@ -38,7 +43,7 @@ func TestLoadConfiguration(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Attempt to load. yamlConfiguration := &Configuration{} diff --git a/pkg/api/models/synchronization/configuration_test.go b/pkg/api/models/synchronization/configuration_test.go index f3286173d..ae6b7f059 100644 --- a/pkg/api/models/synchronization/configuration_test.go +++ b/pkg/api/models/synchronization/configuration_test.go @@ -1,11 +1,14 @@ package synchronization import ( + "bytes" "os" "testing" "github.com/mutagen-io/mutagen/pkg/encoding" "github.com/mutagen-io/mutagen/pkg/filesystem/behavior" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/synchronization" "github.com/mutagen-io/mutagen/pkg/synchronization/core" "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore" @@ -75,6 +78,8 @@ var expectedConfiguration = &synchronization.Configuration{ // TestLoadConfiguration tests loading a YAML-based session configuration. func TestLoadConfiguration(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Write a valid configuration to a temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_configuration") if err != nil { @@ -84,7 +89,7 @@ func TestLoadConfiguration(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Attempt to load. yamlConfiguration := &Configuration{} diff --git a/pkg/daemon/lock.go b/pkg/daemon/lock.go index cb3384c5f..a53d76dfc 100644 --- a/pkg/daemon/lock.go +++ b/pkg/daemon/lock.go @@ -4,6 +4,8 @@ import ( "fmt" "github.com/mutagen-io/mutagen/pkg/filesystem/locking" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // Lock represents the global daemon lock. It is held by a single daemon @@ -11,10 +13,11 @@ import ( type Lock struct { // locker is the underlying file locker. locker *locking.Locker + logger *logging.Logger } // AcquireLock attempts to acquire the global daemon lock. -func AcquireLock() (*Lock, error) { +func AcquireLock(logger *logging.Logger) (*Lock, error) { // Compute the lock path. lockPath, err := subpath(lockName) if err != nil { @@ -26,13 +29,14 @@ func AcquireLock() (*Lock, error) { if err != nil { return nil, fmt.Errorf("unable to create daemon file locker: %w", err) } else if err = locker.Lock(false); err != nil { - locker.Close() + must.Close(locker, logger) return nil, err } // Create the lock. return &Lock{ locker: locker, + logger: logger, }, nil } @@ -40,7 +44,7 @@ func AcquireLock() (*Lock, error) { func (l *Lock) Release() error { // Release the lock. if err := l.locker.Unlock(); err != nil { - l.locker.Close() + must.Close(l.locker, l.logger) return err } diff --git a/pkg/daemon/lock_test.go b/pkg/daemon/lock_test.go index 18162b11d..7fb918901 100644 --- a/pkg/daemon/lock_test.go +++ b/pkg/daemon/lock_test.go @@ -1,13 +1,18 @@ package daemon import ( + "bytes" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" ) // TestLockCycle tests an acquisition/release cycle of the daemon lock. func TestLockCycle(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Attempt to acquire the daemon lock. - lock, err := AcquireLock() + lock, err := AcquireLock(logger) if err != nil { t.Fatal("unable to acquire lock:", err) } diff --git a/pkg/daemon/register_darwin.go b/pkg/daemon/register_darwin.go index ed8bbb429..93f0ff696 100644 --- a/pkg/daemon/register_darwin.go +++ b/pkg/daemon/register_darwin.go @@ -14,6 +14,8 @@ import ( "path/filepath" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // RegistrationSupported indicates whether or not daemon registration is @@ -64,7 +66,7 @@ const ( ) // Register performs automatic daemon startup registration. -func Register() error { +func Register(logger *logging.Logger) error { // If we're already registered, don't do anything. if registered, err := registered(); err != nil { return fmt.Errorf("unable to determine registration status: %w", err) @@ -76,11 +78,11 @@ func Register() error { // start and stop mechanism depending on whether or not we're registered, so // we need to make sure we don't try to stop a daemon started using a // different mechanism. - lock, err := AcquireLock() + lock, err := AcquireLock(logger) if err != nil { return errors.New("unable to alter registration while daemon is running") } - defer lock.Release() + defer must.Release(lock, logger) // Compute the path to the user's home directory. homeDirectory, err := os.UserHomeDir() @@ -111,7 +113,7 @@ func Register() error { // Attempt to write the launchd plist. targetPath = filepath.Join(targetPath, launchdPlistName) - if err := filesystem.WriteFileAtomic(targetPath, []byte(plist), launchdPlistPermissions); err != nil { + if err := filesystem.WriteFileAtomic(targetPath, []byte(plist), launchdPlistPermissions, logger); err != nil { return fmt.Errorf("unable to write launchd agent plist: %w", err) } @@ -120,7 +122,7 @@ func Register() error { } // Unregister performs automatic daemon startup de-registration. -func Unregister() error { +func Unregister(logger *logging.Logger) error { // If we're not registered, don't do anything. if registered, err := registered(); err != nil { return fmt.Errorf("unable to determine registration status: %w", err) @@ -132,11 +134,11 @@ func Unregister() error { // start and stop mechanism depending on whether or not we're registered, so // we need to make sure we don't try to stop a daemon started using a // different mechanism. - lock, err := AcquireLock() + lock, err := AcquireLock(logger) if err != nil { return errors.New("unable to alter registration while daemon is running") } - defer lock.Release() + defer must.Release(lock, logger) // Compute the path to the user's home directory. homeDirectory, err := os.UserHomeDir() diff --git a/pkg/daemon/register_others.go b/pkg/daemon/register_others.go index 4ee17eb9e..c81cd60ae 100644 --- a/pkg/daemon/register_others.go +++ b/pkg/daemon/register_others.go @@ -4,6 +4,8 @@ package daemon import ( "errors" + + "github.com/mutagen-io/mutagen/pkg/logging" ) // RegistrationSupported indicates whether or not daemon registration is @@ -11,12 +13,12 @@ import ( const RegistrationSupported = false // Register performs automatic daemon startup registration. -func Register() error { +func Register(logger *logging.Logger) error { return errors.New("daemon registration not supported on this platform") } // Unregister performs automatic daemon startup de-registration. -func Unregister() error { +func Unregister(logger *logging.Logger) error { return errors.New("daemon deregistration not supported on this platform") } diff --git a/pkg/daemon/register_windows.go b/pkg/daemon/register_windows.go index 2b5f29c40..1d3fc84a8 100644 --- a/pkg/daemon/register_windows.go +++ b/pkg/daemon/register_windows.go @@ -4,6 +4,7 @@ import ( "fmt" "os" + "github.com/mutagen-io/mutagen/pkg/logging" "golang.org/x/sys/windows/registry" ) @@ -21,7 +22,7 @@ const ( ) // Register performs automatic daemon startup registration. -func Register() error { +func Register(logger *logging.Logger) error { // Attempt to open the relevant registry path and ensure it's cleaned up // when we're done. key, err := registry.OpenKey(rootKey, runPath, registry.SET_VALUE) @@ -49,7 +50,7 @@ func Register() error { } // Unregister performs automatic daemon startup de-registration. -func Unregister() error { +func Unregister(logger *logging.Logger) error { // Attempt to open the relevant registry path and ensure it's cleaned up // when we're done. key, err := registry.OpenKey(rootKey, runPath, registry.QUERY_VALUE|registry.SET_VALUE) diff --git a/pkg/encoding/common.go b/pkg/encoding/common.go index 5ccafcac4..372defa12 100644 --- a/pkg/encoding/common.go +++ b/pkg/encoding/common.go @@ -5,6 +5,7 @@ import ( "os" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" ) // LoadAndUnmarshal provides the underlying loading and unmarshaling @@ -34,7 +35,7 @@ func LoadAndUnmarshal(path string, unmarshal func([]byte) error) error { // the encoding package. It invokes the specified marshaling callback (usually a // closure) and writes the result atomically to the specified path. The data is // saved with read/write permissions for the user only. -func MarshalAndSave(path string, marshal func() ([]byte, error)) error { +func MarshalAndSave(path string, logger *logging.Logger, marshal func() ([]byte, error)) error { // Marshal the message. data, err := marshal() if err != nil { @@ -42,7 +43,7 @@ func MarshalAndSave(path string, marshal func() ([]byte, error)) error { } // Write the file atomically with secure file permissions. - if err := filesystem.WriteFileAtomic(path, data, 0600); err != nil { + if err := filesystem.WriteFileAtomic(path, data, 0600, logger); err != nil { return fmt.Errorf("unable to write message data: %w", err) } diff --git a/pkg/encoding/common_test.go b/pkg/encoding/common_test.go index b84a348bc..a89eca5be 100644 --- a/pkg/encoding/common_test.go +++ b/pkg/encoding/common_test.go @@ -1,17 +1,21 @@ package encoding import ( + "bytes" "encoding/json" "errors" "os" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // testMessageJSON is a test structure to use for encoding tests using JSON. type testMessageJSON struct { // Name represents a person's name. Name string - // Age represent's a person's age. + // Age represents a person's age. Age uint } @@ -49,6 +53,8 @@ func TestLoadAndUnmarshalDirectory(t *testing.T) { // TestLoadAndUnmarshalUnmarshalFail tests that unmarshaling fails if the // unmarshaling callback fails. func TestLoadAndUnmarshalUnmarshalFail(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create an empty temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_encoding") if err != nil { @@ -56,7 +62,7 @@ func TestLoadAndUnmarshalUnmarshalFail(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Create a broken unmarshaling function. unmarshal := func(_ []byte) error { @@ -71,6 +77,8 @@ func TestLoadAndUnmarshalUnmarshalFail(t *testing.T) { // TestLoadAndUnmarshal tests that loading and unmarshaling succeed. func TestLoadAndUnmarshal(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Write the test JSON to a temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_encoding") if err != nil { @@ -80,7 +88,7 @@ func TestLoadAndUnmarshal(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Create an unmarshaling function. value := &testMessageJSON{} @@ -105,6 +113,8 @@ func TestLoadAndUnmarshal(t *testing.T) { // TestMarshalAndSaveMarshalFail tests that marshaling fails if the marshaling // callback fails. func TestMarshalAndSaveMarshalFail(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create an empty temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_encoding") if err != nil { @@ -112,7 +122,7 @@ func TestMarshalAndSaveMarshalFail(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Create a broken marshaling function. marshal := func() ([]byte, error) { @@ -120,26 +130,30 @@ func TestMarshalAndSaveMarshalFail(t *testing.T) { } // Attempt to marshal and save using a broken unmarshaling function. - if MarshalAndSave(file.Name(), marshal) == nil { + if MarshalAndSave(file.Name(), logger, marshal) == nil { t.Error("expected MarshalAndSave to return an error") } } // TestMarshalAndSaveOverDirectory tests that saving over a directory fails. func TestMarshalAndSaveOverDirectory(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a marshaling function. marshal := func() ([]byte, error) { return []byte{0}, nil } // Attempt to marshal and save over a directory. - if MarshalAndSave(t.TempDir(), marshal) == nil { + if MarshalAndSave(t.TempDir(), logger, marshal) == nil { t.Error("expected MarshalAndSave to return an error") } } // TestMarshalAndSave tests that marshaling and saving succeed. func TestMarshalAndSave(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create an empty temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_encoding") if err != nil { @@ -147,7 +161,7 @@ func TestMarshalAndSave(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Create a marshaling function. value := &testMessageJSON{Name: testMessageJSONName, Age: testMessageJSONAge} @@ -156,7 +170,7 @@ func TestMarshalAndSave(t *testing.T) { } // Attempt to marshal and save. - if err := MarshalAndSave(file.Name(), marshal); err != nil { + if err := MarshalAndSave(file.Name(), logger, marshal); err != nil { t.Fatal("MarshalAndSave failed:", err) } diff --git a/pkg/encoding/protobuf.go b/pkg/encoding/protobuf.go index 412d171b9..38bf6d8f8 100644 --- a/pkg/encoding/protobuf.go +++ b/pkg/encoding/protobuf.go @@ -6,6 +6,7 @@ import ( "fmt" "io" + "github.com/mutagen-io/mutagen/pkg/logging" "google.golang.org/protobuf/encoding/protowire" "google.golang.org/protobuf/proto" @@ -42,8 +43,8 @@ func LoadAndUnmarshalProtobuf(path string, message proto.Message) error { // MarshalAndSaveProtobuf marshals the specified Protocol Buffers message and // saves it to the specified path. -func MarshalAndSaveProtobuf(path string, message proto.Message) error { - return MarshalAndSave(path, func() ([]byte, error) { +func MarshalAndSaveProtobuf(path string, message proto.Message, logger *logging.Logger) error { + return MarshalAndSave(path, logger, func() ([]byte, error) { return proto.Marshal(message) }) } diff --git a/pkg/encoding/protobuf_test.go b/pkg/encoding/protobuf_test.go index b33e588b3..6d746fb97 100644 --- a/pkg/encoding/protobuf_test.go +++ b/pkg/encoding/protobuf_test.go @@ -5,12 +5,16 @@ import ( "os" "testing" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/url" ) // TestProtocolBuffersCycle tests a Protocol Buffers marshal/save/load/unmarshal // cycle. func TestProtocolBuffersCycle(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create an empty temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_encoding") if err != nil { @@ -18,7 +22,7 @@ func TestProtocolBuffersCycle(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Create a Protocol Buffers message that we can test with. message := &url.URL{ @@ -28,7 +32,7 @@ func TestProtocolBuffersCycle(t *testing.T) { Port: 1776, Path: "/by/land/or/by/sea", } - if err := MarshalAndSaveProtobuf(file.Name(), message); err != nil { + if err := MarshalAndSaveProtobuf(file.Name(), message, logger); err != nil { t.Fatal("unable to marshal and save Protocol Buffers message:", err) } diff --git a/pkg/encoding/yaml_test.go b/pkg/encoding/yaml_test.go index b24e61e00..12f93c017 100644 --- a/pkg/encoding/yaml_test.go +++ b/pkg/encoding/yaml_test.go @@ -1,8 +1,12 @@ package encoding import ( + "bytes" "os" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // testMessageYAML is a test structure to use for encoding tests using YAML. @@ -29,6 +33,8 @@ section: // TestLoadAndUnmarshalYAML tests that loading and unmarshaling YAML data // succeeds. func TestLoadAndUnmarshalYAML(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Write the test YAML to a temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_encoding") if err != nil { @@ -38,7 +44,7 @@ func TestLoadAndUnmarshalYAML(t *testing.T) { } else if err = file.Close(); err != nil { t.Fatal("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Attempt to load and unmarshal. value := &testMessageYAML{} diff --git a/pkg/filesystem/atomic.go b/pkg/filesystem/atomic.go index 2ae955cbd..ea52d3932 100644 --- a/pkg/filesystem/atomic.go +++ b/pkg/filesystem/atomic.go @@ -4,6 +4,9 @@ import ( "fmt" "os" "path/filepath" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) const ( @@ -15,7 +18,7 @@ const ( // WriteFileAtomic writes a file to disk in an atomic fashion by using an // intermediate temporary file that is swapped in place using a rename // operation. -func WriteFileAtomic(path string, data []byte, permissions os.FileMode) error { +func WriteFileAtomic(path string, data []byte, permissions os.FileMode, logger *logging.Logger) error { // Create a temporary file. The os package already uses secure permissions // for creating temporary files, so we don't need to change them. temporary, err := os.CreateTemp(filepath.Dir(path), atomicWriteTemporaryNamePrefix) @@ -25,26 +28,26 @@ func WriteFileAtomic(path string, data []byte, permissions os.FileMode) error { // Write data. if _, err = temporary.Write(data); err != nil { - temporary.Close() - os.Remove(temporary.Name()) + must.Close(temporary, logger) + must.OSRemove(temporary.Name(), logger) return fmt.Errorf("unable to write data to temporary file: %w", err) } // Close out the file. if err = temporary.Close(); err != nil { - os.Remove(temporary.Name()) + must.OSRemove(temporary.Name(), logger) return fmt.Errorf("unable to close temporary file: %w", err) } // Set the file's permissions. if err = os.Chmod(temporary.Name(), permissions); err != nil { - os.Remove(temporary.Name()) + must.OSRemove(temporary.Name(), logger) return fmt.Errorf("unable to change file permissions: %w", err) } // Rename the file. if err = Rename(nil, temporary.Name(), nil, path, true); err != nil { - os.Remove(temporary.Name()) + must.OSRemove(temporary.Name(), logger) return fmt.Errorf("unable to rename file: %w", err) } diff --git a/pkg/filesystem/atomic_test.go b/pkg/filesystem/atomic_test.go index 51a950e5f..98c27deaa 100644 --- a/pkg/filesystem/atomic_test.go +++ b/pkg/filesystem/atomic_test.go @@ -5,15 +5,21 @@ import ( "os" "path/filepath" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" ) func TestWriteFileAtomicNonExistentDirectory(t *testing.T) { - if WriteFileAtomic("/does/not/exist", []byte{}, 0600) == nil { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + + if WriteFileAtomic("/does/not/exist", []byte{}, 0600, logger) == nil { t.Error("atomic file write did not fail for non-existent path") } } func TestWriteFileAtomic(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Compute the target path. target := filepath.Join(t.TempDir(), "file") @@ -21,7 +27,7 @@ func TestWriteFileAtomic(t *testing.T) { contents := []byte{0, 1, 2, 3, 4, 5, 6} // Attempt to write to a temporary file. - if err := WriteFileAtomic(target, contents, 0600); err != nil { + if err := WriteFileAtomic(target, contents, 0600, logger); err != nil { t.Fatal("atomic file write failed:", err) } diff --git a/pkg/filesystem/behavior/executability.go b/pkg/filesystem/behavior/executability.go index eac398a17..cbd5a011a 100644 --- a/pkg/filesystem/behavior/executability.go +++ b/pkg/filesystem/behavior/executability.go @@ -6,6 +6,8 @@ import ( "runtime" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) const ( @@ -19,7 +21,7 @@ const ( // executability bits. It allows for the path leaf to be a symbolic link. The // second value returned by this function indicates whether or not probe files // were used in determining behavior. -func PreservesExecutabilityByPath(path string, probeMode ProbeMode) (bool, bool, error) { +func PreservesExecutabilityByPath(path string, probeMode ProbeMode, logger *logging.Logger) (bool, bool, error) { // Check the filesystem probing mode and see if we can return an assumption. if probeMode == ProbeMode_ProbeModeAssume { return assumeExecutabilityPreservation, false, nil @@ -48,7 +50,7 @@ func PreservesExecutabilityByPath(path string, probeMode ProbeMode) (bool, bool, // Ensure that the file is cleaned up and removed when we're done. defer func() { file.Close() - os.Remove(file.Name()) + must.OSRemove(file.Name(), logger) }() // Mark the file as user-executable. We use the os.File-based Chmod here @@ -75,7 +77,7 @@ func PreservesExecutabilityByPath(path string, probeMode ProbeMode) (bool, bool, // its underlying filesystem) preserves POSIX executability bits. The second // value returned by this function indicates whether or not probe files were // used in determining behavior. -func PreservesExecutability(directory *filesystem.Directory, probeMode ProbeMode) (bool, bool, error) { +func PreservesExecutability(directory *filesystem.Directory, probeMode ProbeMode, logger *logging.Logger) (bool, bool, error) { // Check the filesystem probing mode and see if we can return an assumption. if probeMode == ProbeMode_ProbeModeAssume { return assumeExecutabilityPreservation, false, nil @@ -103,8 +105,8 @@ func PreservesExecutability(directory *filesystem.Directory, probeMode ProbeMode // Ensure that the file is cleaned up and removed when we're done. defer func() { - file.Close() - directory.RemoveFile(name) + must.Close(file, logger) + must.RemoveFile(directory, name, logger) }() // HACK: Convert the file to an os.File object for race-free Chmod and Stat diff --git a/pkg/filesystem/behavior/executability_test.go b/pkg/filesystem/behavior/executability_test.go index 721063fcc..ed6f22ca6 100644 --- a/pkg/filesystem/behavior/executability_test.go +++ b/pkg/filesystem/behavior/executability_test.go @@ -1,11 +1,14 @@ package behavior import ( + "bytes" "os" "runtime" "testing" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // preservesExecutabilityByPathTestCase represents a test case for @@ -18,6 +21,7 @@ type preservesExecutabilityByPathTestCase struct { assume bool // expected is the expected result of the executability preservation test. expected bool + logger *logging.Logger } // run executes the test in the provided test context. @@ -36,7 +40,7 @@ func (c *preservesExecutabilityByPathTestCase) run(t *testing.T) { // TODO: We should perform some validation on the second parameter returned // by PreservesExecutabilityByPath (indicating whether or not probe files // were used). - if preserves, _, err := PreservesExecutabilityByPath(c.path, probeMode); err != nil { + if preserves, _, err := PreservesExecutabilityByPath(c.path, probeMode, c.logger); err != nil { t.Fatal("unable to probe executability preservation:", err) } else if preserves != c.expected { t.Error("executability preservation behavior does not match expected") @@ -57,6 +61,7 @@ func TestPreservesExecutabilityByPathAssumedHomeDirectory(t *testing.T) { path: homeDirectory, assume: true, expected: runtime.GOOS != "windows", + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -76,6 +81,7 @@ func TestPreservesExecutabilityByPathHomeDirectory(t *testing.T) { testCase := &preservesExecutabilityByPathTestCase{ path: homeDirectory, expected: runtime.GOOS != "windows", + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -95,6 +101,7 @@ func TestPreservesExecutabilityByPathFAT32(t *testing.T) { testCase := &preservesExecutabilityByPathTestCase{ path: fat32Root, expected: false, + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -111,6 +118,8 @@ type preservesExecutabilityTestCase struct { assume bool // expected is the expected result of the executability preservation test. expected bool + + logger *logging.Logger } // run executes the test in the provided test context. @@ -119,11 +128,11 @@ func (c *preservesExecutabilityTestCase) run(t *testing.T) { t.Helper() // Open the path, ensure that it's a directory, and defer its closure. - directory, _, err := filesystem.OpenDirectory(c.path, false) + directory, _, err := filesystem.OpenDirectory(c.path, false, c.logger) if err != nil { t.Fatal("unable to open path:", err) } - defer directory.Close() + defer must.Close(directory, c.logger) // Determine the probing mode. probeMode := ProbeMode_ProbeModeProbe @@ -136,7 +145,7 @@ func (c *preservesExecutabilityTestCase) run(t *testing.T) { // TODO: We should perform some validation on the second parameter returned // by PreservesExecutability (indicating whether or not probe files were // used). - if preserves, _, err := PreservesExecutability(directory, probeMode); err != nil { + if preserves, _, err := PreservesExecutability(directory, probeMode, c.logger); err != nil { t.Fatal("unable to probe executability preservation:", err) } else if preserves != c.expected { t.Error("executability preservation behavior does not match expected") @@ -157,6 +166,7 @@ func TestPreservesExecutabilityAssumedHomeDirectory(t *testing.T) { path: homeDirectory, assume: true, expected: runtime.GOOS != "windows", + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -176,6 +186,7 @@ func TestPreservesExecutabilityHomeDirectory(t *testing.T) { testCase := &preservesExecutabilityTestCase{ path: homeDirectory, expected: runtime.GOOS != "windows", + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -195,6 +206,7 @@ func TestPreservesExecutabilityFAT32(t *testing.T) { testCase := &preservesExecutabilityTestCase{ path: fat32Root, expected: false, + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. diff --git a/pkg/filesystem/behavior/unicode.go b/pkg/filesystem/behavior/unicode.go index efff8d428..c092669bd 100644 --- a/pkg/filesystem/behavior/unicode.go +++ b/pkg/filesystem/behavior/unicode.go @@ -9,6 +9,8 @@ import ( "strings" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) const ( @@ -23,7 +25,7 @@ const ( // directory at the specified path resides decomposes Unicode filenames. The // second value returned by this function indicates whether or not probe files // were used in determining behavior. -func DecomposesUnicodeByPath(path string, probeMode ProbeMode) (bool, bool, error) { +func DecomposesUnicodeByPath(path string, probeMode ProbeMode, logger *logging.Logger) (bool, bool, error) { // Check the filesystem probing mode and see if we can return an assumption. if probeMode == ProbeMode_ProbeModeAssume { return assumeUnicodeDecomposition, false, nil @@ -41,9 +43,9 @@ func DecomposesUnicodeByPath(path string, probeMode ProbeMode) (bool, bool, erro // Create and close a temporary file using the composed filename. file, err := os.CreateTemp(path, composedFileNamePrefix) if err != nil { - return false, true, fmt.Errorf("unable to create test file: %w", err) + return false, true, fmt.Errorf("unable to create test file '%s/%s': %w", path, composedFileNamePrefix, err) } else if err = file.Close(); err != nil { - return false, true, fmt.Errorf("unable to close test file: %w", err) + return false, true, fmt.Errorf("unable to close test file '%s/%s': %w", path, composedFileNamePrefix, err) } // Grab the file's name. This is calculated from the parameters passed to @@ -61,7 +63,7 @@ func DecomposesUnicodeByPath(path string, probeMode ProbeMode) (bool, bool, erro // also normalization-insensitive, we try both compositions. defer func() { if os.Remove(filepath.Join(path, composedFilename)) != nil { - os.Remove(filepath.Join(path, decomposedFilename)) + must.OSRemove(filepath.Join(path, decomposedFilename), logger) } }() @@ -90,7 +92,7 @@ func DecomposesUnicodeByPath(path string, probeMode ProbeMode) (bool, bool, erro // underlying filesystem) decomposes Unicode filenames. The second value // returned by this function indicates whether or not probe files were used in // determining behavior. -func DecomposesUnicode(directory *filesystem.Directory, probeMode ProbeMode) (bool, bool, error) { +func DecomposesUnicode(directory *filesystem.Directory, probeMode ProbeMode, logger *logging.Logger) (bool, bool, error) { // Check the filesystem probing mode and see if we can return an assumption. if probeMode == ProbeMode_ProbeModeAssume { return assumeUnicodeDecomposition, false, nil @@ -108,9 +110,9 @@ func DecomposesUnicode(directory *filesystem.Directory, probeMode ProbeMode) (bo // Create and close a temporary file using the composed filename. composedName, file, err := directory.CreateTemporaryFile(composedFileNamePrefix) if err != nil { - return false, true, fmt.Errorf("unable to create test file: %w", err) + return false, true, fmt.Errorf("unable to create test file '%s': %w", composedName, err) } else if err = file.Close(); err != nil { - return false, true, fmt.Errorf("unable to close test file: %w", err) + return false, true, fmt.Errorf("unable to close test file '%s': %w", composedName, err) } // The name returned from CreateTemporaryFile is calculated from the @@ -127,7 +129,7 @@ func DecomposesUnicode(directory *filesystem.Directory, probeMode ProbeMode) (bo // also normalization-insensitive, we try both compositions. defer func() { if directory.RemoveFile(composedName) != nil { - directory.RemoveFile(decomposedName) + must.RemoveFile(directory, decomposedName, logger) } }() @@ -146,11 +148,11 @@ func DecomposesUnicode(directory *filesystem.Directory, probeMode ProbeMode) (bo // hit a fast path above anyway. directoryForContentRead := directory if runtime.GOOS == "linux" { - directoryForContentRead, err = directory.OpenDirectory(".") + directoryForContentRead, err = directory.OpenDirectory(".", logger) if err != nil { return false, true, fmt.Errorf("unable to re-open directory: %w", err) } - defer directoryForContentRead.Close() + defer must.Close(directoryForContentRead, logger) } // Grab the content names in the directory. diff --git a/pkg/filesystem/behavior/unicode_test.go b/pkg/filesystem/behavior/unicode_test.go index 844806f18..7944ff735 100644 --- a/pkg/filesystem/behavior/unicode_test.go +++ b/pkg/filesystem/behavior/unicode_test.go @@ -1,11 +1,14 @@ package behavior import ( + "bytes" "os" "runtime" "testing" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) func TestDecomposesUnicodeByPathAssumedHomeDirectory(t *testing.T) { @@ -15,13 +18,15 @@ func TestDecomposesUnicodeByPathAssumedHomeDirectory(t *testing.T) { t.Fatal("unable to compute home directory:", err) } + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Query the assumed behavior of the home directory and ensure it matches // what's expected. // // TODO: We should perform some validation on the second parameter returned // by DecomposesUnicodeByPath (indicating whether or not probe files were // used). - if decomposes, _, err := DecomposesUnicodeByPath(homeDirectory, ProbeMode_ProbeModeAssume); err != nil { + if decomposes, _, err := DecomposesUnicodeByPath(homeDirectory, ProbeMode_ProbeModeAssume, logger); err != nil { t.Fatal("unable to query Unicode decomposition:", err) } else if decomposes { t.Error("Unicode decomposition behavior does not match expected") @@ -35,6 +40,7 @@ func TestDecomposesUnicodeByPathDarwinHFS(t *testing.T) { if runtime.GOOS != "darwin" { t.Skip() } + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) // If we don't have the separate HFS+ partition, skip this test. hfsRoot := os.Getenv("MUTAGEN_TEST_HFS_ROOT") @@ -47,7 +53,7 @@ func TestDecomposesUnicodeByPathDarwinHFS(t *testing.T) { // TODO: We should perform some validation on the second parameter returned // by DecomposesUnicodeByPath (indicating whether or not probe files were // used). - if decomposes, _, err := DecomposesUnicodeByPath(hfsRoot, ProbeMode_ProbeModeProbe); err != nil { + if decomposes, _, err := DecomposesUnicodeByPath(hfsRoot, ProbeMode_ProbeModeProbe, logger); err != nil { t.Fatal("unable to probe Unicode decomposition:", err) } else if !decomposes { t.Error("Unicode decomposition behavior does not match expected") @@ -60,13 +66,14 @@ func TestDecomposesUnicodeByPathDarwinAPFS(t *testing.T) { if apfsRoot == "" { t.Skip() } + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) // Probe the behavior of the root and ensure it matches what's expected. // // TODO: We should perform some validation on the second parameter returned // by DecomposesUnicodeByPath (indicating whether or not probe files were // used). - if decomposes, _, err := DecomposesUnicodeByPath(apfsRoot, ProbeMode_ProbeModeProbe); err != nil { + if decomposes, _, err := DecomposesUnicodeByPath(apfsRoot, ProbeMode_ProbeModeProbe, logger); err != nil { t.Fatal("unable to probe Unicode decomposition:", err) } else if decomposes { t.Error("Unicode decomposition behavior does not match expected") @@ -81,6 +88,8 @@ func TestDecomposesUnicodeByPathOSPartition(t *testing.T) { t.Skip() } + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Probe the behavior of the root and ensure it matches what's expected. The // only case we expect to decompose is HFS+ on Darwin, which we won't // encounter in this test. @@ -88,7 +97,7 @@ func TestDecomposesUnicodeByPathOSPartition(t *testing.T) { // TODO: We should perform some validation on the second parameter returned // by DecomposesUnicodeByPath (indicating whether or not probe files were // used). - if decomposes, _, err := DecomposesUnicodeByPath(".", ProbeMode_ProbeModeProbe); err != nil { + if decomposes, _, err := DecomposesUnicodeByPath(".", ProbeMode_ProbeModeProbe, logger); err != nil { t.Fatal("unable to probe Unicode decomposition:", err) } else if decomposes { t.Error("Unicode decomposition behavior does not match expected") @@ -104,6 +113,8 @@ type decomposesUnicodeTestCase struct { assume bool // expected is the expected result of the Unicode decomposition test. expected bool + + logger *logging.Logger } // run executes the test in the provided test context. @@ -112,11 +123,11 @@ func (c *decomposesUnicodeTestCase) run(t *testing.T) { t.Helper() // Open the path, ensure that it's a directory, and defer its closure. - directory, _, err := filesystem.OpenDirectory(c.path, false) + directory, _, err := filesystem.OpenDirectory(c.path, false, c.logger) if err != nil { t.Fatal("unable to open path:", err) } - defer directory.Close() + defer must.Close(directory, c.logger) // Determine the probing mode. probeMode := ProbeMode_ProbeModeProbe @@ -128,7 +139,7 @@ func (c *decomposesUnicodeTestCase) run(t *testing.T) { // // TODO: We should perform some validation on the second parameter returned // by DecomposesUnicode (indicating whether or not probe files were used). - if decomposes, _, err := DecomposesUnicode(directory, probeMode); err != nil { + if decomposes, _, err := DecomposesUnicode(directory, probeMode, c.logger); err != nil { t.Fatal("unable to probe Unicode decomposition:", err) } else if decomposes != c.expected { t.Error("Unicode decomposition behavior does not match expected") @@ -149,6 +160,7 @@ func TestDecomposesUnicodeAssumedHomeDirectory(t *testing.T) { path: homeDirectory, assume: true, expected: false, + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -175,6 +187,7 @@ func TestDecomposesUnicodeDarwinHFS(t *testing.T) { testCase := &decomposesUnicodeTestCase{ path: hfsRoot, expected: true, + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -194,6 +207,7 @@ func TestDecomposesUnicodeDarwinAPFS(t *testing.T) { testCase := &decomposesUnicodeTestCase{ path: apfsRoot, expected: false, + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. @@ -220,6 +234,7 @@ func TestDecomposesUnicodeHomeDirectory(t *testing.T) { testCase := &decomposesUnicodeTestCase{ path: homeDirectory, expected: false, + logger: logging.NewLogger(logging.LevelError, &bytes.Buffer{}), } // Run the test case. diff --git a/pkg/filesystem/directory_posix.go b/pkg/filesystem/directory_posix.go index 369e2cfa3..0e83d92cd 100644 --- a/pkg/filesystem/directory_posix.go +++ b/pkg/filesystem/directory_posix.go @@ -16,6 +16,7 @@ import ( "sync" "time" + "github.com/mutagen-io/mutagen/pkg/logging" "golang.org/x/sys/unix" "github.com/mutagen-io/mutagen/pkg/state" @@ -61,6 +62,8 @@ type Directory struct { // renameatNoReplaceRetryingOnEINTR is found to be unsupported with this // directory as a target. renameatNoReplaceUnsupported state.Marker + + logger *logging.Logger } // Close closes the directory. @@ -204,7 +207,7 @@ func (d *Directory) SetPermissions(name string, ownership *OwnershipSpecificatio if f, err := openatRetryingOnEINTR(d.descriptor, name, unix.O_RDONLY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0); err != nil { return fmt.Errorf("unable to open file: %w", err) } else if err = fchmodRetryingOnEINTR(f, uint32(mode)); err != nil { - closeConsideringEINTR(f) + mustCloseConsideringEINTR(f, d.logger) return fmt.Errorf("unable to set permission bits on file: %w", err) } else if err = closeConsideringEINTR(f); err != nil { return fmt.Errorf("unable to close file: %w", err) @@ -258,10 +261,10 @@ func (d *Directory) open(name string, wantDirectory bool) (int, *Metadata, error if !wantDirectory { var rawMetadata unix.Stat_t if err := fstatRetryingOnEINTR(descriptor, &rawMetadata); err != nil { - closeConsideringEINTR(descriptor) + mustCloseConsideringEINTR(descriptor, d.logger) return -1, nil, fmt.Errorf("unable to query file metadata: %w", err) } else if Mode(rawMetadata.Mode)&ModeTypeMask != ModeTypeFile { - closeConsideringEINTR(descriptor) + mustCloseConsideringEINTR(descriptor, d.logger) return -1, nil, errors.New("path is not a file") } metadata = &Metadata{ @@ -282,7 +285,7 @@ func (d *Directory) open(name string, wantDirectory bool) (int, *Metadata, error // POSIX systems, the directory itself can be re-opened (with a different // underlying file handle pointing to the same directory) by passing "." to this // function. -func (d *Directory) OpenDirectory(name string) (*Directory, error) { +func (d *Directory) OpenDirectory(name string, logger *logging.Logger) (*Directory, error) { // Call the underlying open method. descriptor, _, err := d.open(name, true) if err != nil { @@ -293,6 +296,7 @@ func (d *Directory) OpenDirectory(name string) (*Directory, error) { return &Directory{ descriptor: descriptor, file: os.NewFile(uintptr(descriptor), name), + logger: logger, }, nil } diff --git a/pkg/filesystem/directory_test.go b/pkg/filesystem/directory_test.go index 0921b1ca3..101372b07 100644 --- a/pkg/filesystem/directory_test.go +++ b/pkg/filesystem/directory_test.go @@ -1,11 +1,15 @@ package filesystem import ( + "bytes" "os" "path/filepath" "runtime" "testing" "unicode/utf8" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // TestPathSeparatorSingleByte verifies that the platform path separator rune is @@ -19,11 +23,13 @@ func TestPathSeparatorSingleByte(t *testing.T) { func TestDirectoryContentsNotExist(t *testing.T) { if _, err := DirectoryContentsByPath("/does/not/exist"); err == nil { - t.Error("directory listing succeedeed for non-existent path") + t.Error("directory listing succeeded for non-existent path") } } func TestDirectoryContentsFile(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create an empty temporary file and defer its cleanup. file, err := os.CreateTemp("", "mutagen_filesystem") if err != nil { @@ -31,11 +37,11 @@ func TestDirectoryContentsFile(t *testing.T) { } else if err = file.Close(); err != nil { t.Error("unable to close temporary file:", err) } - defer os.Remove(file.Name()) + defer must.OSRemove(file.Name(), logger) // Ensure that directory listing fails. if _, err := DirectoryContentsByPath(file.Name()); err == nil { - t.Error("directory listing succeedeed for non-directory path") + t.Error("directory listing succeeded for non-directory path") } } @@ -50,13 +56,15 @@ func TestDirectoryContentsGOROOT(t *testing.T) { // TestNonEmptyDirectoryRemovalFailure tests that removal of a non-empty // directory results in failure. func TestNonEmptyDirectoryRemovalFailure(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a handle for a temporary directory (that will be removed // automatically) and defer its closure. - directory, _, err := OpenDirectory(t.TempDir(), false) + directory, _, err := OpenDirectory(t.TempDir(), false, logger) if err != nil { t.Fatal("unable to open directory handle:", err) } - defer directory.Close() + defer must.Close(directory, logger) // Create a directory that will serve as our target. if err := directory.CreateDirectory("target"); err != nil { @@ -64,10 +72,10 @@ func TestNonEmptyDirectoryRemovalFailure(t *testing.T) { } // Create content inside the directory. - if target, err := directory.OpenDirectory("target"); err != nil { + if target, err := directory.OpenDirectory("target", logger); err != nil { t.Fatal("unable to open target directory:", err) } else if err = target.CreateDirectory("content"); err != nil { - target.Close() + must.Close(target, logger) t.Fatal("unable to create content in target directory:", err) } else if err = target.Close(); err != nil { t.Fatal("unable to close target directory:", err) @@ -82,15 +90,17 @@ func TestNonEmptyDirectoryRemovalFailure(t *testing.T) { // TestDirectorySymbolicLinkRemoval tests that removal of symbolic links that // point to directories works as expected. func TestDirectorySymbolicLinkRemoval(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a temporary directory (that will be automatically removed). temporaryDirectoryPath := t.TempDir() // Create a handle for the temporary directory and defer its closure. - directory, _, err := OpenDirectory(temporaryDirectoryPath, false) + directory, _, err := OpenDirectory(temporaryDirectoryPath, false, logger) if err != nil { t.Fatal("unable to open directory handle:", err) } - defer directory.Close() + defer must.Close(directory, logger) // Create a directory that will serve as our target. if err := directory.CreateDirectory("target"); err != nil { @@ -98,11 +108,11 @@ func TestDirectorySymbolicLinkRemoval(t *testing.T) { } // Open the target directory and defer its closure. - target, err := directory.OpenDirectory("target") + target, err := directory.OpenDirectory("target", logger) if err != nil { t.Fatal("unable to open target directory:", err) } - defer target.Close() + defer must.Close(target, logger) // Create content within the target. if err := target.CreateDirectory("content"); err != nil { diff --git a/pkg/filesystem/directory_windows.go b/pkg/filesystem/directory_windows.go index e36dce115..170ee1b62 100644 --- a/pkg/filesystem/directory_windows.go +++ b/pkg/filesystem/directory_windows.go @@ -9,6 +9,8 @@ import ( "strings" "syscall" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "golang.org/x/sys/windows" aclapi "github.com/hectane/go-acl/api" @@ -52,14 +54,15 @@ type Directory struct { // function, hence the reason we need to hold open a separate HANDLE. It is // guaranteed that the value returned from the file's Name function will be // an absolute path. - file *os.File + file *os.File + logger *logging.Logger } // Close closes the directory. func (d *Directory) Close() error { // Close the file object. if err := d.file.Close(); err != nil { - windows.CloseHandle(d.handle) + must.CloseWindowsHandle(d.handle, d.logger) return fmt.Errorf("unable to close file object: %w", err) } @@ -207,7 +210,7 @@ func (d *Directory) SetPermissions(name string, ownership *OwnershipSpecificatio // openHandle is the underlying open implementation shared by OpenDirectory and // OpenFile. It returns the full target path, the Windows file handle // corresponding to the target, the target metadata, or any error. -func (d *Directory) openHandle(name string, wantDirectory bool) (string, windows.Handle, *Metadata, error) { +func (d *Directory) openHandle(name string, wantDirectory bool, logger *logging.Logger) (string, windows.Handle, *Metadata, error) { // Verify that the name is valid. if err := ensureValidName(name); err != nil { return "", 0, nil, err @@ -248,19 +251,19 @@ func (d *Directory) openHandle(name string, wantDirectory bool) (string, windows // Query handle metadata. metadata, err := queryHandleMetadata(name, handle) if err != nil { - windows.CloseHandle(handle) + must.CloseWindowsHandle(handle, logger) return "", 0, nil, fmt.Errorf("unable to query file handle metadata: %w", err) } // Verify that we're not dealing with a symbolic link. if metadata.Mode&ModeTypeSymbolicLink != 0 { - windows.CloseHandle(handle) + must.CloseWindowsHandle(handle, logger) return "", 0, nil, errors.New("path pointed to symbolic link") } // Verify that the handle corresponds to a directory (if requested). if wantDirectory && metadata.Mode&ModeTypeDirectory == 0 { - windows.CloseHandle(handle) + must.CloseWindowsHandle(handle, logger) return "", 0, nil, errors.New("path pointed to non-directory location") } @@ -269,9 +272,9 @@ func (d *Directory) openHandle(name string, wantDirectory bool) (string, windows } // OpenDirectory opens the directory within the directory specified by name. -func (d *Directory) OpenDirectory(name string) (*Directory, error) { +func (d *Directory) OpenDirectory(name string, logger *logging.Logger) (*Directory, error) { // Open the directory handle. - path, handle, _, err := d.openHandle(name, true) + path, handle, _, err := d.openHandle(name, true, logger) if err != nil { return nil, fmt.Errorf("unable to open directory handle: %w", err) } @@ -282,7 +285,7 @@ func (d *Directory) OpenDirectory(name string) (*Directory, error) { // this object (and its Name method) isn't exposed anyway. file, err := os.Open(path) if err != nil { - windows.CloseHandle(handle) + must.CloseWindowsHandle(handle, logger) return nil, fmt.Errorf("unable to open file object for directory: %w", err) } @@ -290,6 +293,7 @@ func (d *Directory) OpenDirectory(name string) (*Directory, error) { return &Directory{ handle: handle, file: file, + logger: logger, }, nil } @@ -402,7 +406,7 @@ func (d *Directory) ReadContents() ([]*Metadata, error) { // OpenFile opens the file within the directory specified by name. func (d *Directory) OpenFile(name string) (io.ReadSeekCloser, *Metadata, error) { // Open the file handle. - _, handle, metadata, err := d.openHandle(name, false) + _, handle, metadata, err := d.openHandle(name, false, d.logger) if err != nil { return nil, nil, fmt.Errorf("unable to open file handle: %w", err) } diff --git a/pkg/filesystem/directory_windows_test.go b/pkg/filesystem/directory_windows_test.go index 29b021b80..45947f640 100644 --- a/pkg/filesystem/directory_windows_test.go +++ b/pkg/filesystem/directory_windows_test.go @@ -1,16 +1,22 @@ package filesystem import ( + "bytes" "os" "os/user" "path/filepath" "strings" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // TestDirectoryLongPaths tests a variety of Directory operations on directory // and file names that exceed the default Windows path length limit. func TestDirectoryLongPaths(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a temporary directory (that will be automatically removed). temporaryDirectoryPath := t.TempDir() @@ -28,14 +34,14 @@ func TestDirectoryLongPaths(t *testing.T) { if err != nil { t.Fatal("unable to create test file with long name:", err) } - file.Close() + must.Close(file, logger) // Open the temporary directory for access and defer its closure. - closer, _, err := Open(temporaryDirectoryPath, false) + closer, _, err := Open(temporaryDirectoryPath, false, logger) if err != nil { t.Fatal("unable to open directory:", err) } - defer closer.Close() + defer must.Close(closer, logger) // Extract the directory object. var directory *Directory @@ -46,17 +52,17 @@ func TestDirectoryLongPaths(t *testing.T) { } // Access the internal directory and ensure that doing so succeeds. - if d, err := directory.OpenDirectory(longDirectoryName); err != nil { + if d, err := directory.OpenDirectory(longDirectoryName, logger); err != nil { t.Error("unable to open directory with long name:", err) } else { - d.Close() + must.Close(d, logger) } // Access the internal file and ensure that doing so succeeds. if f, _, err := directory.OpenFile(longFileName); err != nil { t.Error("unable to open file with long name:", err) } else { - f.Close() + must.Close(f, logger) } // Try to set permissions. diff --git a/pkg/filesystem/interrupt_posix.go b/pkg/filesystem/interrupt_posix.go index c8272e2a6..60bb52c5e 100644 --- a/pkg/filesystem/interrupt_posix.go +++ b/pkg/filesystem/interrupt_posix.go @@ -5,6 +5,7 @@ package filesystem import ( "io" + "github.com/mutagen-io/mutagen/pkg/logging" "golang.org/x/sys/unix" "github.com/mutagen-io/mutagen/pkg/filesystem/internal/syscall" @@ -58,6 +59,15 @@ func closeConsideringEINTR(file int) error { return unix.Close(file) } +// mustCloseConsideringEINTR calls closeConsideringEINTR and logs if there is an +// error. +func mustCloseConsideringEINTR(file int, logger *logging.Logger) { + err := unix.Close(file) + if err != nil { + logger.Warnf("Unable to close considering EINTR file %d; %s", file, err.Error()) + } +} + // mkdiratRetryingOnEINTR is a wrapper around the mkdirat system call that // retries on EINTR errors and returns on the first successful call or non-EINTR // error. diff --git a/pkg/filesystem/locking/locker_test.go b/pkg/filesystem/locking/locker_test.go index c9b5aae54..ec60ad434 100644 --- a/pkg/filesystem/locking/locker_test.go +++ b/pkg/filesystem/locking/locker_test.go @@ -7,6 +7,8 @@ import ( "strings" "testing" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/mutagen" ) @@ -31,6 +33,8 @@ func TestLockerFailOnDirectory(t *testing.T) { // TestLockerCycle tests the lifecycle of a Locker. func TestLockerCycle(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a temporary file and defer its removal. lockfile, err := os.CreateTemp("", "mutagen_filesystem_lock") if err != nil { @@ -38,7 +42,7 @@ func TestLockerCycle(t *testing.T) { } else if err = lockfile.Close(); err != nil { t.Error("unable to close temporary lock file:", err) } - defer os.Remove(lockfile.Name()) + defer must.OSRemove(lockfile.Name(), logger) // Create a locker. locker, err := NewLocker(lockfile.Name(), 0600) @@ -70,6 +74,8 @@ func TestLockerCycle(t *testing.T) { // TestLockDuplicateFail tests that an additional attempt to acquire a lock by a // separate process will fail. func TestLockDuplicateFail(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Compute the path to the Mutagen source tree. mutagenSourcePath, err := mutagen.SourceTreePath() if err != nil { @@ -83,7 +89,7 @@ func TestLockDuplicateFail(t *testing.T) { } else if err = lockfile.Close(); err != nil { t.Error("unable to close temporary lock file:", err) } - defer os.Remove(lockfile.Name()) + defer must.OSRemove(lockfile.Name(), logger) // Create a locker for the file, acquire the lock, and defer the release of // the lock and closure of the locker. @@ -94,8 +100,8 @@ func TestLockDuplicateFail(t *testing.T) { t.Fatal("unable to acquire lock:", err) } defer func() { - locker.Unlock() - locker.Close() + must.Unlock(locker, logger) + must.Close(locker, logger) }() // Attempt to run the test executable and ensure that it fails with the diff --git a/pkg/filesystem/open.go b/pkg/filesystem/open.go index 849bb3097..2592adece 100644 --- a/pkg/filesystem/open.go +++ b/pkg/filesystem/open.go @@ -5,6 +5,9 @@ import ( "fmt" "io" "strings" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // ErrUnsupportedOpenType indicates that the filesystem entry at the specified @@ -13,14 +16,14 @@ var ErrUnsupportedOpenType = errors.New("unsupported open type") // OpenDirectory is a convenience wrapper around Open that requires the result // to be a directory. -func OpenDirectory(path string, allowSymbolicLinkLeaf bool) (*Directory, *Metadata, error) { - if d, metadata, err := Open(path, allowSymbolicLinkLeaf); err != nil { +func OpenDirectory(path string, allowSymbolicLinkLeaf bool, logger *logging.Logger) (*Directory, *Metadata, error) { + if d, metadata, err := Open(path, allowSymbolicLinkLeaf, logger); err != nil { return nil, nil, err } else if (metadata.Mode & ModeTypeMask) != ModeTypeDirectory { - d.Close() + must.Close(d, logger) return nil, nil, errors.New("path is not a directory") } else if directory, ok := d.(*Directory); !ok { - d.Close() + must.Close(d, logger) panic("invalid directory object returned from open operation") } else { return directory, metadata, nil @@ -29,14 +32,14 @@ func OpenDirectory(path string, allowSymbolicLinkLeaf bool) (*Directory, *Metada // OpenFile is a convenience wrapper around Open that requires the result to be // a file. -func OpenFile(path string, allowSymbolicLinkLeaf bool) (io.ReadSeekCloser, *Metadata, error) { - if f, metadata, err := Open(path, allowSymbolicLinkLeaf); err != nil { +func OpenFile(path string, allowSymbolicLinkLeaf bool, logger *logging.Logger) (io.ReadSeekCloser, *Metadata, error) { + if f, metadata, err := Open(path, allowSymbolicLinkLeaf, logger); err != nil { return nil, nil, err } else if (metadata.Mode & ModeTypeMask) != ModeTypeFile { - f.Close() + must.Close(f, logger) return nil, nil, errors.New("path is not a file") } else if file, ok := f.(io.ReadSeekCloser); !ok { - f.Close() + must.Close(f, logger) panic("invalid file object returned from open operation") } else { return file, metadata, nil @@ -65,11 +68,16 @@ type Opener struct { // correspond to openParentNames, and likewise it will be empty if // rootDirectory is nil. openParentDirectories []*Directory + + logger *logging.Logger } // NewOpener creates a new Opener for the specified root path. -func NewOpener(root string) *Opener { - return &Opener{root: root} +func NewOpener(root string, logger *logging.Logger) *Opener { + return &Opener{ + root: root, + logger: logger, + } } // OpenFile opens the file at the specified path (relative to the root). On all @@ -93,7 +101,7 @@ func (o *Opener) OpenFile(path string) (io.ReadSeekCloser, *Metadata, error) { } // Attempt to open the file. - if file, metadata, err := OpenFile(o.root, false); err != nil { + if file, metadata, err := OpenFile(o.root, false, o.logger); err != nil { return nil, nil, fmt.Errorf("unable to open root file: %w", err) } else { return file, metadata, nil @@ -107,7 +115,7 @@ func (o *Opener) OpenFile(path string) (io.ReadSeekCloser, *Metadata, error) { // If it's not already open, open the root directory. if o.rootDirectory == nil { - if directory, _, err := OpenDirectory(o.root, false); err != nil { + if directory, _, err := OpenDirectory(o.root, false, o.logger); err != nil { return nil, nil, fmt.Errorf("unable to open root directory: %w", err) } else { o.rootDirectory = directory @@ -144,7 +152,7 @@ func (o *Opener) OpenFile(path string) (io.ReadSeekCloser, *Metadata, error) { } // Open the directory ourselves and add it to the parent stacks. - if directory, err := parent.OpenDirectory(component); err != nil { + if directory, err := parent.OpenDirectory(component, o.logger); err != nil { return nil, nil, fmt.Errorf("unable to open parent directory: %w", err) } else { parent = directory diff --git a/pkg/filesystem/open_posix.go b/pkg/filesystem/open_posix.go index 31825ea39..199fd46d4 100644 --- a/pkg/filesystem/open_posix.go +++ b/pkg/filesystem/open_posix.go @@ -9,6 +9,7 @@ import ( "path/filepath" "time" + "github.com/mutagen-io/mutagen/pkg/logging" "golang.org/x/sys/unix" ) @@ -25,7 +26,7 @@ import ( // link. In this case, the referenced object must still be a directory or // regular file, and the returned object will still be either a Directory or an // io.ReadSeekCloser. -func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) { +func Open(path string, allowSymbolicLinkLeaf bool, logger *logging.Logger) (io.Closer, *Metadata, error) { // Open the file. Unless explicitly allowed, we disable resolution of // symbolic links at the leaf position of the path by specifying O_NOFOLLOW. // Note that this flag only affects the leaf component of the path - @@ -41,7 +42,7 @@ func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) // too many symbolic links have been encountered, and thus there's no way to // differentiate the two cases and figure out whether or not we should // return ErrUnsupportedType. Even openat doesn't provide a solution to this - // problem since it doens't support AT_SYMLINK_NOFOLLOW. Essentially, + // problem since it doesn't support AT_SYMLINK_NOFOLLOW. Essentially, // there's no way to "open" a symbolic link - it can only be read with // readlink and its ilk. Since ELOOP still sort of makes sense (we've // encountered too many symbolic links at the path leaf), we return it @@ -58,7 +59,7 @@ func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) // Grab metadata for the file. var rawMetadata unix.Stat_t if err := fstatRetryingOnEINTR(descriptor, &rawMetadata); err != nil { - closeConsideringEINTR(descriptor) + mustCloseConsideringEINTR(descriptor, logger) return nil, nil, fmt.Errorf("unable to query file metadata: %w", err) } @@ -82,7 +83,7 @@ func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) case ModeTypeFile: return file(descriptor), metadata, nil default: - closeConsideringEINTR(descriptor) + mustCloseConsideringEINTR(descriptor, logger) return nil, nil, ErrUnsupportedOpenType } } diff --git a/pkg/filesystem/open_windows.go b/pkg/filesystem/open_windows.go index e495c47c8..1e049e970 100644 --- a/pkg/filesystem/open_windows.go +++ b/pkg/filesystem/open_windows.go @@ -7,6 +7,8 @@ import ( "os" "path/filepath" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "golang.org/x/sys/windows" osvendor "github.com/mutagen-io/mutagen/pkg/filesystem/internal/third_party/os" @@ -24,7 +26,7 @@ import ( // link. In this case, the referenced object must still be a directory or // regular file, and the returned object will still be either a Directory or an // io.ReadSeekCloser. -func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) { +func Open(path string, allowSymbolicLinkLeaf bool, logger *logging.Logger) (io.Closer, *Metadata, error) { // Verify that the provided path is absolute. This is a requirement on // Windows, where all of our operations are path-based. if !filepath.IsAbs(path) { @@ -67,14 +69,14 @@ func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) // Query handle metadata. metadata, err := queryHandleMetadata(filepath.Base(path), handle) if err != nil { - windows.CloseHandle(handle) + must.CloseWindowsHandle(handle, logger) return nil, nil, fmt.Errorf("unable to query file handle metadata: %w", err) } // Verify that we're not dealing with a symbolic link. If we are allowing // symbolic links, then they should have been resolved by CreateFile. if metadata.Mode&ModeTypeSymbolicLink != 0 { - windows.CloseHandle(handle) + must.CloseWindowsHandle(handle, logger) return nil, nil, ErrUnsupportedOpenType } @@ -84,7 +86,7 @@ func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) if isDirectory { file, err = os.Open(path) if err != nil { - windows.CloseHandle(handle) + must.CloseWindowsHandle(handle, logger) return nil, nil, fmt.Errorf("unable to open file object for directory: %w", err) } } else { @@ -96,6 +98,7 @@ func Open(path string, allowSymbolicLinkLeaf bool) (io.Closer, *Metadata, error) return &Directory{ handle: handle, file: file, + logger: logger, }, metadata, nil } else { return file, metadata, nil diff --git a/pkg/filesystem/open_windows_test.go b/pkg/filesystem/open_windows_test.go index 126d27c98..a030da2c0 100644 --- a/pkg/filesystem/open_windows_test.go +++ b/pkg/filesystem/open_windows_test.go @@ -1,15 +1,19 @@ package filesystem import ( + "bytes" "os" "path/filepath" "strings" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" ) // TestOpenLongPath verifies that calling Open succeeds on a directory whose // path length exceeds the default path length limit on Windows. func TestOpenLongPath(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) // Create a directory (in a temporary directory that will be automatically // removed) with a name that will exceed the Windows path length limit. longDirectoryName := strings.Repeat("d", windowsLongPathTestingLength) @@ -19,7 +23,7 @@ func TestOpenLongPath(t *testing.T) { } // Attempt to open the directory and ensure doing so succeeds. - directory, _, err := Open(longtemporaryDirectoryPath, false) + directory, _, err := Open(longtemporaryDirectoryPath, false, logger) if err != nil { t.Fatal("unable to open directory with long path:", err) } diff --git a/pkg/filesystem/visibility_test.go b/pkg/filesystem/visibility_test.go index 749813ce8..cb9a96fa1 100644 --- a/pkg/filesystem/visibility_test.go +++ b/pkg/filesystem/visibility_test.go @@ -1,18 +1,24 @@ package filesystem import ( + "bytes" "os" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) func TestMarkHidden(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a temporary file and defer its removal. hiddenFile, err := os.CreateTemp("", ".mutagen_filesystem_hidden") if err != nil { t.Fatal("unable to create temporary hiddenFile file:", err) } - hiddenFile.Close() - defer os.Remove(hiddenFile.Name()) + must.Close(hiddenFile, logger) + defer must.OSRemove(hiddenFile.Name(), logger) // Ensure that we can mark it as hidden. if err := MarkHidden(hiddenFile.Name()); err != nil { diff --git a/pkg/filesystem/watching/watch_test.go b/pkg/filesystem/watching/watch_test.go index 1d5adb52c..c866874f1 100644 --- a/pkg/filesystem/watching/watch_test.go +++ b/pkg/filesystem/watching/watch_test.go @@ -1,11 +1,15 @@ package watching import ( + "bytes" "os" "path/filepath" "runtime" "testing" "time" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) const ( @@ -47,6 +51,8 @@ func TestRecursiveWatcher(t *testing.T) { t.Skip() } + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a temporary directory (that will be automatically removed). directory := t.TempDir() @@ -55,7 +61,7 @@ func TestRecursiveWatcher(t *testing.T) { if err != nil { t.Fatal("unable to establish watch:", err) } - defer watcher.Terminate() + defer must.Terminate(watcher, logger) // Create a subdirectory. subdirectoryRelative := "subdirectory" @@ -101,6 +107,7 @@ func TestNonRecursiveWatcher(t *testing.T) { if !NonRecursiveWatchingSupported { t.Skip() } + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) // Create a temporary directory (that will be automatically removed). directory := t.TempDir() @@ -111,7 +118,7 @@ func TestNonRecursiveWatcher(t *testing.T) { t.Fatal("unable to create watcher:", err) } watcher.Watch(directory) - defer watcher.Terminate() + defer must.Terminate(watcher, logger) // Create a subdirectory. subdirectoryPath := filepath.Join(directory, "subdirectory") diff --git a/pkg/forwarding/connect.go b/pkg/forwarding/connect.go index 8e507c650..8f8feb100 100644 --- a/pkg/forwarding/connect.go +++ b/pkg/forwarding/connect.go @@ -41,6 +41,10 @@ func connect( configuration *Configuration, source bool, ) (Endpoint, error) { + + // Added trace to bypass "unused parameter `session` warning + logger.Tracef("Connecting to %s for session %s", url.String(), session) + // Local the appropriate protocol handler. handler, ok := ProtocolHandlers[url.Protocol] if !ok { diff --git a/pkg/forwarding/controller.go b/pkg/forwarding/controller.go index 7e4a31555..426943d10 100644 --- a/pkg/forwarding/controller.go +++ b/pkg/forwarding/controller.go @@ -13,6 +13,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/encoding" "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/mutagen" "github.com/mutagen-io/mutagen/pkg/prompting" "github.com/mutagen-io/mutagen/pkg/state" @@ -85,7 +86,7 @@ func newSession( prompter string, ) (*controller, error) { // Update status. - prompting.Message(prompter, "Creating session...") + prompting.MustMessage(prompter, "Creating session...", logger) // Set the session version. version := DefaultVersion @@ -107,11 +108,11 @@ func newSession( var err error defer func() { if sourceEndpoint != nil { - sourceEndpoint.Shutdown() + must.Shutdown(sourceEndpoint, logger) sourceEndpoint = nil } if destinationEndpoint != nil { - destinationEndpoint.Shutdown() + must.Shutdown(destinationEndpoint, logger) destinationEndpoint = nil } }() @@ -173,7 +174,7 @@ func newSession( } // Save the session to disk. - if err := encoding.MarshalAndSaveProtobuf(sessionPath, session); err != nil { + if err := encoding.MarshalAndSaveProtobuf(sessionPath, session, logger); err != nil { return nil, fmt.Errorf("unable to save session: %w", err) } @@ -276,7 +277,7 @@ func (c *controller) currentState() *State { // connected and forwarding. func (c *controller) resume(ctx context.Context, prompter string) error { // Update status. - prompting.Message(prompter, fmt.Sprintf("Resuming session %s...", c.session.Identifier)) + prompting.MustMessage(prompter, fmt.Sprintf("Resuming session %s...", c.session.Identifier), c.logger) // Lock the controller's lifecycle and defer its release. c.lifecycleLock.Lock() @@ -327,7 +328,7 @@ func (c *controller) resume(ctx context.Context, prompter string) error { // Mark the session as unpaused and save it to disk. c.stateLock.Lock() c.session.Paused = false - saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session) + saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session, c.logger) c.stateLock.Unlock() // Attempt to connect to source. @@ -420,7 +421,10 @@ func (m controllerHaltMode) description() string { // halt halts the session with the specified behavior. func (c *controller) halt(_ context.Context, mode controllerHaltMode, prompter string) error { // Update status. - prompting.Message(prompter, fmt.Sprintf("%s session %s...", mode.description(), c.session.Identifier)) + prompting.MustMessage(prompter, + fmt.Sprintf("%s session %s...", mode.description(), c.session.Identifier), + c.logger, + ) // Lock the controller's lifecycle and defer its release. c.lifecycleLock.Lock() @@ -452,7 +456,7 @@ func (c *controller) halt(_ context.Context, mode controllerHaltMode, prompter s // Mark the session as paused and save it. c.stateLock.Lock() c.session.Paused = true - saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session) + saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session, c.logger) c.stateLock.Unlock() if saveErr != nil { return fmt.Errorf("unable to save session: %w", saveErr) @@ -489,10 +493,10 @@ func (c *controller) run(ctx context.Context, source, destination Endpoint) { // cancelled while partially connected rather than after forwarding // failure. if source != nil { - source.Shutdown() + must.Shutdown(source, c.logger) } if destination != nil { - destination.Shutdown() + must.Shutdown(destination, c.logger) } // Reset the state. @@ -607,8 +611,8 @@ func (c *controller) run(ctx context.Context, source, destination Endpoint) { shutdownComplete := make(chan struct{}) go func() { <-shutdownCtx.Done() - source.Shutdown() - destination.Shutdown() + must.Shutdown(source, c.logger) + must.Shutdown(destination, c.logger) close(shutdownComplete) }() @@ -733,7 +737,7 @@ func (c *controller) forward(source, destination Endpoint) error { // Open the outgoing connection to which we should forward. outgoing, err := destination.Open() if err != nil { - incoming.Close() + must.Close(incoming, c.logger) return fmt.Errorf("unable to open forwarding connection: %w", err) } @@ -746,7 +750,7 @@ func (c *controller) forward(source, destination Endpoint) error { // Perform forwarding and update state in a background Goroutine. go func() { // Perform forwarding. - ForwardAndClose(ctx, incoming, outgoing, incomingAuditor, outgoingAuditor) + ForwardAndClose(ctx, incoming, outgoing, incomingAuditor, outgoingAuditor, c.logger) // Decrement open connection counts. c.stateLock.Lock() diff --git a/pkg/forwarding/endpoint/remote/client.go b/pkg/forwarding/endpoint/remote/client.go index 4efce232d..17342f0ec 100644 --- a/pkg/forwarding/endpoint/remote/client.go +++ b/pkg/forwarding/endpoint/remote/client.go @@ -11,6 +11,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/forwarding" "github.com/mutagen-io/mutagen/pkg/logging" "github.com/mutagen-io/mutagen/pkg/multiplexing" + "github.com/mutagen-io/mutagen/pkg/must" ) // client is a client for a remote forwarding.Endpoint and implements @@ -52,7 +53,7 @@ func NewEndpoint( var initializationSuccessful bool defer func() { if !initializationSuccessful { - carrier.Close() + must.Close(carrier, logger) } }() @@ -83,7 +84,7 @@ func NewEndpoint( initializationSuccessful = true // Multiplex the carrier. - multiplexer := multiplexing.Multiplex(carrier, false, nil) + multiplexer := multiplexing.Multiplex(carrier, false, nil, logger) // Create a channel to monitor for transport errors and a Goroutine to // populate it. diff --git a/pkg/forwarding/endpoint/remote/server.go b/pkg/forwarding/endpoint/remote/server.go index 2745816ec..a0d764f07 100644 --- a/pkg/forwarding/endpoint/remote/server.go +++ b/pkg/forwarding/endpoint/remote/server.go @@ -12,6 +12,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/forwarding/endpoint/local" "github.com/mutagen-io/mutagen/pkg/logging" "github.com/mutagen-io/mutagen/pkg/multiplexing" + "github.com/mutagen-io/mutagen/pkg/must" ) // initializeEndpoint initializes the underlying endpoint based on the provided @@ -64,7 +65,7 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { var initializationError error defer func() { if initializationError != nil { - carrier.Close() + must.Close(carrier, logger) } }() @@ -96,8 +97,8 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { } // Multiplex the carrier and defer closure of the multiplexer. - multiplexer := multiplexing.Multiplex(carrier, true, nil) - defer multiplexer.Close() + multiplexer := multiplexing.Multiplex(carrier, true, nil, logger) + defer must.Close(multiplexer, logger) // Start a Goroutine couple the lifetime of the underlying endpoint to the // lifetime of the multiplexer. This will cause the underlying endpoint to @@ -106,7 +107,7 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { // important for preempting local accept operations. go func() { <-multiplexer.Closed() - underlying.Shutdown() + must.Shutdown(underlying, logger) }() // Receive and forward connections indefinitely. @@ -134,17 +135,17 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { if request.Listener { outgoing, err = multiplexer.OpenStream(context.Background()) if err != nil { - incoming.Close() + must.Close(incoming, logger) return fmt.Errorf("multiplexer failure: %w", err) } } else { if outgoing, err = underlying.Open(); err != nil { - incoming.Close() + must.Close(incoming, logger) continue } } // Perform forwarding. - go forwarding.ForwardAndClose(context.Background(), incoming, outgoing, nil, nil) + go forwarding.ForwardAndClose(context.Background(), incoming, outgoing, nil, nil, logger) } } diff --git a/pkg/forwarding/forwarding.go b/pkg/forwarding/forwarding.go index e5d96e8a4..18b3cbf9c 100644 --- a/pkg/forwarding/forwarding.go +++ b/pkg/forwarding/forwarding.go @@ -5,6 +5,8 @@ import ( "io" "net" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/stream" ) @@ -15,11 +17,11 @@ import ( // Both connections must implement CloseWriter or this function will panic. If // the caller passes non-nil values for firstAuditor and/or secondAuditor, then // auditing will be performed on the write end of the respective connection. -func ForwardAndClose(ctx context.Context, first, second net.Conn, firstAuditor, secondAuditor stream.Auditor) { +func ForwardAndClose(ctx context.Context, first, second net.Conn, firstAuditor, secondAuditor stream.Auditor, logger *logging.Logger) { // Defer closure of the connections. defer func() { - first.Close() - second.Close() + must.Close(first, logger) + must.Close(second, logger) }() // Extract write closure interfaces. @@ -42,14 +44,14 @@ func ForwardAndClose(ctx context.Context, first, second net.Conn, firstAuditor, go func() { _, err := io.Copy(stream.NewAuditWriter(first, firstAuditor), second) if err == nil { - firstCloseWriter.CloseWrite() + must.CloseWrite(firstCloseWriter, logger) } copyErrors <- err }() go func() { _, err := io.Copy(stream.NewAuditWriter(second, secondAuditor), first) if err == nil { - secondCloseWriter.CloseWrite() + must.CloseWrite(secondCloseWriter, logger) } copyErrors <- err }() diff --git a/pkg/forwarding/protocols/docker/protocol.go b/pkg/forwarding/protocols/docker/protocol.go index 6093249a4..cf82640c2 100644 --- a/pkg/forwarding/protocols/docker/protocol.go +++ b/pkg/forwarding/protocols/docker/protocol.go @@ -10,6 +10,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/forwarding" "github.com/mutagen-io/mutagen/pkg/forwarding/endpoint/remote" "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" urlpkg "github.com/mutagen-io/mutagen/pkg/url" forwardingurlpkg "github.com/mutagen-io/mutagen/pkg/url/forwarding" ) @@ -38,6 +39,10 @@ func (p *protocolHandler) Connect( configuration *forwarding.Configuration, source bool, ) (forwarding.Endpoint, error) { + + // Added trace to bypass "unused parameter `session` warning + logger.Tracef("Connecting to %s for session %s", url.String(), session) + // Verify that the URL is of the correct kind and protocol. if url.Kind != urlpkg.Kind_Forwarding { panic("non-forwarding URL dispatched to forwarding protocol handler") @@ -71,7 +76,7 @@ func (p *protocolHandler) Connect( case results <- dialResult{stream, err}: case <-ctx.Done(): if stream != nil { - stream.Close() + must.Close(stream, logger) } } }() diff --git a/pkg/forwarding/protocols/ssh/protocol.go b/pkg/forwarding/protocols/ssh/protocol.go index 67a32b9ad..0bebb1f5a 100644 --- a/pkg/forwarding/protocols/ssh/protocol.go +++ b/pkg/forwarding/protocols/ssh/protocol.go @@ -10,6 +10,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/forwarding" "github.com/mutagen-io/mutagen/pkg/forwarding/endpoint/remote" "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" urlpkg "github.com/mutagen-io/mutagen/pkg/url" forwardingurlpkg "github.com/mutagen-io/mutagen/pkg/url/forwarding" ) @@ -38,6 +39,10 @@ func (p *protocolHandler) Connect( configuration *forwarding.Configuration, source bool, ) (forwarding.Endpoint, error) { + + // Added trace to bypass "unused parameter `session` warning + logger.Tracef("Connecting to %s for session %s", url.String(), session) + // Verify that the URL is of the correct kind and protocol. if url.Kind != urlpkg.Kind_Forwarding { panic("non-forwarding URL dispatched to forwarding protocol handler") @@ -71,7 +76,7 @@ func (p *protocolHandler) Connect( case results <- dialResult{stream, err}: case <-ctx.Done(): if stream != nil { - stream.Close() + must.Close(stream, logger) } } }() diff --git a/pkg/housekeeping/housekeep.go b/pkg/housekeeping/housekeep.go index 27012ae3c..8079d9ff0 100644 --- a/pkg/housekeeping/housekeep.go +++ b/pkg/housekeeping/housekeep.go @@ -1,12 +1,15 @@ package housekeeping import ( + "fmt" "os" "path/filepath" "runtime" "time" "github.com/mutagen-io/extstat" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/agent" "github.com/mutagen-io/mutagen/pkg/filesystem" @@ -25,25 +28,25 @@ const ( ) // Housekeep invokes housekeeping functions on the Mutagen data directory. -func Housekeep() { +func Housekeep(logger *logging.Logger) { // Perform housekeeping on agent binaries, but only if we're not in a // Mutagen sidecar container. Sidecar containers are particularly // susceptible to stale agent access times due to the fact that the agent is // baked into the sidecar image and the sidecar image is typically unpacked // via OverlayFS on top of ext4 with either relatime or noatime. if !sidecar.EnvironmentIsSidecar() { - housekeepAgents() + housekeepAgents(logger) } // Perform housekeeping on caches. - housekeepCaches() + housekeepCaches(logger) // Perform housekeeping on staging roots. - housekeepStaging() + housekeepStaging(logger) } // housekeepAgents performs housekeeping of agent binaries. -func housekeepAgents() { +func housekeepAgents(logger *logging.Logger) { // Compute the path to the agents directory. If we fail, just abort. We // don't attempt to create the directory, because if it doesn't exist, then // we don't need to do anything and we'll just bail when we fail to list the @@ -75,13 +78,17 @@ func housekeepAgents() { if stat, err := extstat.NewFromFileName(filepath.Join(agentsDirectoryPath, agentVersion, agentName)); err != nil { continue } else if now.Sub(stat.AccessTime) > maximumAgentIdlePeriod { - os.RemoveAll(filepath.Join(agentsDirectoryPath, agentVersion)) + fullPath := filepath.Join(agentsDirectoryPath, agentVersion) + must.Succeed(os.RemoveAll(fullPath), + fmt.Sprintf("remove all files from %s", fullPath), + logger, + ) } } } // housekeepCaches performs housekeeping of caches. -func housekeepCaches() { +func housekeepCaches(logger *logging.Logger) { // Compute the path to the caches directory. If we fail, just abort. We // don't attempt to create the directory, because if it doesn't exist, then // we don't need to do anything and we'll just bail when we fail to list the @@ -110,13 +117,13 @@ func housekeepCaches() { if stat, err := os.Stat(fullPath); err != nil { continue } else if now.Sub(stat.ModTime()) > maximumCacheAge { - os.Remove(fullPath) + must.OSRemove(fullPath, logger) } } } // housekeepStaging performs housekeeping of staging roots. -func housekeepStaging() { +func housekeepStaging(logger *logging.Logger) { // Compute the path to the staging directory (the top-level directory // containing all staging roots). If we fail, just abort. We don't attempt // to create the directory, because if it doesn't exist, then we don't need @@ -156,7 +163,10 @@ func housekeepStaging() { if stat, err := os.Stat(fullPath); err != nil { continue } else if now.Sub(stat.ModTime()) > maximumStagingRootAge { - os.RemoveAll(fullPath) + must.Succeed(os.RemoveAll(fullPath), + fmt.Sprintf("remove all files from %s", fullPath), + logger, + ) } } } diff --git a/pkg/housekeeping/housekeep_test.go b/pkg/housekeeping/housekeep_test.go index 4037bd793..acbd4f7f0 100644 --- a/pkg/housekeeping/housekeep_test.go +++ b/pkg/housekeeping/housekeep_test.go @@ -1,25 +1,32 @@ package housekeeping import ( + "bytes" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" ) // TestHousekeep tests that Housekeep succeeds without panicking. func TestHousekeep(_ *testing.T) { - Housekeep() + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + Housekeep(logger) } // TestHousekeepAgents tests that housekeepAgents succeeds without panicking. func TestHousekeepAgents(_ *testing.T) { - housekeepAgents() + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + housekeepAgents(logger) } // TestHousekeepCaches tests that housekeepCaches succeeds without panicking. func TestHousekeepCaches(_ *testing.T) { - housekeepCaches() + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + housekeepCaches(logger) } // TestHousekeepStaging tests that housekeepStaging succeeds without panicking. func TestHousekeepStaging(_ *testing.T) { - housekeepStaging() + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + housekeepStaging(logger) } diff --git a/pkg/integration/integration_test.go b/pkg/integration/integration_test.go index 9e8d67b32..3c9bfeb06 100644 --- a/pkg/integration/integration_test.go +++ b/pkg/integration/integration_test.go @@ -1,8 +1,8 @@ package integration import ( + "bytes" "fmt" - "os" "testing" "google.golang.org/grpc" @@ -13,6 +13,8 @@ import ( "github.com/mutagen-io/mutagen/pkg/forwarding" "github.com/mutagen-io/mutagen/pkg/grpcutil" "github.com/mutagen-io/mutagen/pkg/ipc" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" daemonsvc "github.com/mutagen-io/mutagen/pkg/service/daemon" forwardingsvc "github.com/mutagen-io/mutagen/pkg/service/forwarding" promptingsvc "github.com/mutagen-io/mutagen/pkg/service/prompting" @@ -45,15 +47,17 @@ var synchronizationManager *synchronization.Manager // complete daemon instance for testing, and tear down all of the aforementioned // infrastructure after running tests. func TestMain(m *testing.M) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Override the expected agent bundle location. agent.ExpectedBundleLocation = agent.BundleLocationBuildDirectory // Acquire the daemon lock and defer its release. - lock, err := daemon.AcquireLock() + lock, err := daemon.AcquireLock(logger) if err != nil { cmd.Fatal(fmt.Errorf("unable to acquire daemon lock: %w", err)) } - defer lock.Release() + defer must.Release(lock, logger) // Create a forwarding session manager and defer its shutdown. forwardingManager, err = forwarding.NewManager(nil) @@ -78,7 +82,7 @@ func TestMain(m *testing.M) { defer server.Stop() // Create and register the daemon service and defer its shutdown. - daemonServer := daemonsvc.NewServer() + daemonServer := daemonsvc.NewServer(logger) daemonsvc.RegisterDaemonServer(server, daemonServer) defer daemonServer.Shutdown() @@ -102,17 +106,17 @@ func TestMain(m *testing.M) { // Create the daemon listener and defer its closure. Since we hold the // daemon lock, we preemptively remove any existing socket since it (should) // be stale. - os.Remove(endpoint) - listener, err := ipc.NewListener(endpoint) + must.OSRemove(endpoint, logger) + listener, err := ipc.NewListener(endpoint, logger) if err != nil { cmd.Fatal(fmt.Errorf("unable to create daemon listener: %w", err)) } - defer listener.Close() + defer must.Close(listener, logger) // Serve incoming connections in a separate Goroutine. We don't monitor for // errors since there's nothing that we can do about them and because // they'll likely show up in the test output anyway. - go server.Serve(listener) + go must.Serve(server, listener, logger) // Run tests. m.Run() diff --git a/pkg/integration/internal_api_test.go b/pkg/integration/internal_api_test.go index cd6f4499e..b9695380a 100644 --- a/pkg/integration/internal_api_test.go +++ b/pkg/integration/internal_api_test.go @@ -1,6 +1,7 @@ package integration import ( + "bytes" "context" "errors" "fmt" @@ -12,6 +13,8 @@ import ( "testing" "github.com/google/uuid" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/forwarding" "github.com/mutagen-io/mutagen/pkg/forwarding/endpoint/local" @@ -422,6 +425,8 @@ func init() { } func TestForwardingToHTTPDemo(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // If Docker test support isn't available, then skip this test. if os.Getenv("MUTAGEN_TEST_DOCKER") != "true" { t.Skip() @@ -480,7 +485,7 @@ func TestForwardingToHTTPDemo(t *testing.T) { if err != nil { return fmt.Errorf("unable to perform HTTP GET: %w", err) } - defer response.Body.Close() + defer must.Close(response.Body, logger) // Read the full body. message, err := io.ReadAll(response.Body) diff --git a/pkg/ipc/ipc_posix.go b/pkg/ipc/ipc_posix.go index 59255ac6f..cf992b2e0 100644 --- a/pkg/ipc/ipc_posix.go +++ b/pkg/ipc/ipc_posix.go @@ -7,6 +7,9 @@ import ( "fmt" "net" "os" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // DialContext attempts to establish an IPC connection, timing out if the @@ -21,7 +24,7 @@ func DialContext(ctx context.Context, path string) (net.Conn, error) { } // NewListener creates a new IPC listener. -func NewListener(path string) (net.Listener, error) { +func NewListener(path string, logger *logging.Logger) (net.Listener, error) { // Create the listener. listener, err := net.Listen("unix", path) if err != nil { @@ -31,7 +34,7 @@ func NewListener(path string) (net.Listener, error) { // Explicitly set socket permissions. Unfortunately we can't do this // atomically on socket creation, but we can do it quickly. if err := os.Chmod(path, 0600); err != nil { - listener.Close() + must.Close(listener, logger) return nil, fmt.Errorf("unable to set socket permissions: %w", err) } diff --git a/pkg/ipc/ipc_test.go b/pkg/ipc/ipc_test.go index 1c884485e..cbb4609c4 100644 --- a/pkg/ipc/ipc_test.go +++ b/pkg/ipc/ipc_test.go @@ -1,21 +1,27 @@ package ipc import ( + "bytes" "context" "encoding/gob" "path/filepath" "testing" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // TestDialContextNoEndpoint tests that DialContext fails if there is no // endpoint at the specified path. func TestDialTimeoutNoEndpoint(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Compute the IPC endpoint path. endpoint := filepath.Join(t.TempDir(), "test.sock") // Attempt to dial the listener and ensure that doing so fails. if c, err := DialContext(context.Background(), endpoint); err == nil { - c.Close() + must.Close(c, logger) t.Error("IPC connection succeeded unexpectedly") } } @@ -34,6 +40,8 @@ type testIPCMessage struct { // TestIPC tests that an IPC connection can be established between a listener // and a dialer. func TestIPC(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create a test message. expected := testIPCMessage{"George", 67} @@ -41,11 +49,11 @@ func TestIPC(t *testing.T) { endpoint := filepath.Join(t.TempDir(), "test.sock") // Create a listener and defer its closure. - listener, err := NewListener(endpoint) + listener, err := NewListener(endpoint, logger) if err != nil { t.Fatal("unable to create listener:", err) } - defer listener.Close() + defer must.Close(listener, logger) // Perform dialing and message sending in a separate Goroutine. go func() { @@ -54,13 +62,13 @@ func TestIPC(t *testing.T) { if err != nil { return } - defer connection.Close() + defer must.Close(connection, logger) // Create an encoder. encoder := gob.NewEncoder(connection) // Send a test message. - encoder.Encode(expected) + must.Encode(encoder, expected, logger) }() // Accept a connection and defer its closure. @@ -68,7 +76,7 @@ func TestIPC(t *testing.T) { if err != nil { t.Fatal("unable to accept connection:", err) } - defer connection.Close() + defer must.Close(connection, logger) // Create a decoder. decoder := gob.NewDecoder(connection) diff --git a/pkg/ipc/ipc_windows.go b/pkg/ipc/ipc_windows.go index e94318ef7..cdfae0549 100644 --- a/pkg/ipc/ipc_windows.go +++ b/pkg/ipc/ipc_windows.go @@ -8,8 +8,11 @@ import ( "os/user" "github.com/google/uuid" + "github.com/mutagen-io/mutagen/pkg/logging" "github.com/Microsoft/go-winio" + + "github.com/mutagen-io/mutagen/pkg/must" ) // DialContext attempts to establish an IPC connection, timing out if the @@ -33,13 +36,15 @@ type listener struct { net.Listener // path is the path to the file where the named pipe name is stored. path string + + logger *logging.Logger } // Close closes the listener and removes the pipe name record. func (l *listener) Close() error { // Remove the pipe name record. if err := os.Remove(l.path); err != nil { - l.Listener.Close() + must.Close(l.Listener, l.logger) return fmt.Errorf("unable to remove pipe name record: %w", err) } @@ -48,7 +53,7 @@ func (l *listener) Close() error { } // NewListener creates a new IPC listener. -func NewListener(path string) (net.Listener, error) { +func NewListener(path string, logger *logging.Logger) (net.Listener, error) { // Create a unique pipe name. randomUUID, err := uuid.NewRandom() if err != nil { @@ -96,9 +101,9 @@ func NewListener(path string) (net.Listener, error) { // the event of failure. var successful bool defer func() { - file.Close() + must.Close(file, logger) if !successful { - os.Remove(path) + must.OSRemove(path, logger) } }() @@ -119,5 +124,9 @@ func NewListener(path string) (net.Listener, error) { successful = true // Success. - return &listener{rawListener, path}, nil + return &listener{ + Listener: rawListener, + path: path, + logger: logger, + }, nil } diff --git a/pkg/logging/logger.go b/pkg/logging/logger.go index fd580c94e..845ce6108 100644 --- a/pkg/logging/logger.go +++ b/pkg/logging/logger.go @@ -5,6 +5,7 @@ import ( "io" "regexp" "strings" + "sync/atomic" "time" "github.com/mutagen-io/mutagen/pkg/platform/terminal" @@ -122,7 +123,7 @@ func (l *Logger) write(timestamp time.Time, level Level, message string) { // that logic and to avoid having to add a lock outside the writer. In any // case, Go's standard log package also discards analogous errors, so we'll // do the same for the time being. - l.writer.Write([]byte(line)) + mustWrite(l.writer, []byte(line), l) } // log provides logging with formatting semantics equivalent to fmt.Sprintln. @@ -210,7 +211,7 @@ var linePrefixMatcher = regexp.MustCompile(`^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} // and gated against this logger's level, its scope will be merged with that of // this logger, and the combined line will be written. Otherwise, if an incoming // line is not determined to be from another logger, than it will be written as -// a message with the specificed level. +// a message with the specified level. // // Note that unlike the Logger itself, the writer returned from this method is // not safe for concurrent use by multiple Goroutines. An external locking @@ -257,7 +258,28 @@ func (l *Logger) Writer(level Level) io.Writer { line = terminal.NeutralizeControlCharacters(line) // Write the line to the underlying writer. - l.writer.Write([]byte(line)) + mustWrite(l.writer, []byte(line), l) }, } } + +// recurse is a flag to ensure Write() does not recurse infinitely. +var recurse atomic.Int32 + +// Write takes a writer and a buffer and logs an error if writing returns an +// error or the number of bytes written don't match. +func mustWrite(w io.Writer, p []byte, logger *Logger) { + if recurse.Load() > 1 { + // We cannot have an infinite loop on an error + return + } + recurse.Add(1) + n, err := w.Write(p) + if err != nil { + logger.Warnf("Unable to write: %s", err.Error()) + } + if n < len(p) { + logger.Warnf("Unable to write all %d bytes", n) + } + recurse.Add(-1) +} diff --git a/pkg/multiplexing/multiplexer.go b/pkg/multiplexing/multiplexer.go index 038009c18..a03819b44 100644 --- a/pkg/multiplexing/multiplexer.go +++ b/pkg/multiplexing/multiplexer.go @@ -11,7 +11,9 @@ import ( "sync" "time" + "github.com/mutagen-io/mutagen/pkg/logging" "github.com/mutagen-io/mutagen/pkg/multiplexing/ring" + "github.com/mutagen-io/mutagen/pkg/must" ) var ( @@ -40,6 +42,9 @@ type Multiplexer struct { // configuration is the multiplexer configuration. configuration *Configuration + // logger is the multiplexer configuration. + logger *logging.Logger + // closeOnce guards closure of closer and closed. closeOnce sync.Once // closer closes the underlying carrier. @@ -109,7 +114,7 @@ type Multiplexer struct { // stream identifiers. // // If configuration is nil, the default configuration will be used. -func Multiplex(carrier Carrier, even bool, configuration *Configuration) *Multiplexer { +func Multiplex(carrier Carrier, even bool, configuration *Configuration, logger *logging.Logger) *Multiplexer { // If no configuration was provided, then use default values, otherwise // normalize any out-of-range values provided by the caller. if configuration == nil { @@ -122,6 +127,7 @@ func Multiplex(carrier Carrier, even bool, configuration *Configuration) *Multip multiplexer := &Multiplexer{ even: even, configuration: configuration, + logger: logger, closer: carrier, closed: make(chan struct{}), streams: make(map[uint64]*Stream), @@ -212,7 +218,7 @@ func (m *Multiplexer) read(reader Carrier, heartbeats chan<- struct{}) error { // Track the range of stream identifiers used by the remote. var largestOpenedInboundStreamIdentifier uint64 - // Loop until failure or multiplexure closure. + // Loop until failure or multiplexer closure. for { // Read the next message type. var kind messageKind @@ -607,7 +613,10 @@ func (m *Multiplexer) OpenStream(ctx context.Context) (*Stream, error) { var sentOpenMessage, established bool defer func() { if !established { - stream.close(sentOpenMessage) + err := stream.close(sentOpenMessage) + if err != nil { + m.logger.Warnf("Unable to close stream: %s", err) + } } }() @@ -664,7 +673,7 @@ func (m *Multiplexer) acceptOneStream(ctx context.Context) (*Stream, error) { // check this because we're responsible for closing it. defer func() { if !isClosed(stream.established) { - stream.Close() + must.Close(stream, m.logger) } }() @@ -694,7 +703,7 @@ func (m *Multiplexer) acceptOneStream(ctx context.Context) (*Stream, error) { return stream, nil } -// AcceptContext accepts an incoming stream. +// AcceptStream accepts an incoming stream. func (m *Multiplexer) AcceptStream(ctx context.Context) (*Stream, error) { // Loop until we find a pending stream that's not stale or encounter some // other error. @@ -731,9 +740,9 @@ func (m *Multiplexer) InternalError() error { // closeWithError is the internal close method that allows for optional error // reporting when closing. -func (m *Multiplexer) closeWithError(internalError error) (err error) { +func (m *Multiplexer) closeWithError(internalError error) { m.closeOnce.Do(func() { - err = m.closer.Close() + must.Close(m.closer, m.logger) if internalError != nil { m.internalErrorLock.Lock() m.internalError = internalError @@ -747,5 +756,6 @@ func (m *Multiplexer) closeWithError(internalError error) (err error) { // Close implements net.Listener.Close. Only the first call to Close will have // any effect. Subsequent calls will behave as no-ops and return nil errors. func (m *Multiplexer) Close() error { - return m.closeWithError(nil) + m.closeWithError(nil) + return nil } diff --git a/pkg/multiplexing/multiplexer_test.go b/pkg/multiplexing/multiplexer_test.go index 2a61e962d..47c285e30 100644 --- a/pkg/multiplexing/multiplexer_test.go +++ b/pkg/multiplexing/multiplexer_test.go @@ -1,18 +1,21 @@ package multiplexing import ( + "bytes" "context" "fmt" "net" "sync" "testing" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "golang.org/x/net/nettest" ) // makeNetTestMakePipe constructs a nettest.MakePipe with a pair of multiplexers // operating in opener and acceptor roles. -func makeNetTestMakePipe(opener, acceptor *Multiplexer) nettest.MakePipe { +func makeNetTestMakePipe(opener, acceptor *Multiplexer, logger *logging.Logger) nettest.MakePipe { return func() (c1, c2 net.Conn, stop func(), err error) { var wait sync.WaitGroup wait.Add(2) @@ -39,10 +42,10 @@ func makeNetTestMakePipe(opener, acceptor *Multiplexer) nettest.MakePipe { wait.Wait() if openErr != nil || acceptErr != nil { if opened != nil { - opened.Close() + must.Close(opened, logger) } if accepted != nil { - accepted.Close() + must.Close(accepted, logger) } if openErr != nil { err = openErr @@ -54,8 +57,8 @@ func makeNetTestMakePipe(opener, acceptor *Multiplexer) nettest.MakePipe { c1 = opened c2 = accepted stop = func() { - opened.Close() - accepted.Close() + must.Close(opened, logger) + must.Close(accepted, logger) } } return @@ -71,19 +74,23 @@ func TestMultiplexer(t *testing.T) { p1Carrier := NewCarrierFromStream(p1) p2Carrier := NewCarrierFromStream(p2) + errBuf := bytes.Buffer{} + logger := logging.NewLogger(logging.LevelError, &errBuf) // Perform multiplexing. - p1Mux := Multiplex(p1Carrier, false, nil) - p2Mux := Multiplex(p2Carrier, true, nil) + p1Mux := Multiplex(p1Carrier, false, nil, logger) + p2Mux := Multiplex(p2Carrier, true, nil, logger) // Defer multiplexer shutdown. defer func() { - p1Mux.Close() - p2Mux.Close() + must.Close(p1Mux, logger) + must.Close(p2Mux, logger) }() // Run tests from p1 to p2. - nettest.TestConn(t, makeNetTestMakePipe(p1Mux, p2Mux)) + nettest.TestConn(t, makeNetTestMakePipe(p1Mux, p2Mux, logger)) // Run tests from p2 to p1. - nettest.TestConn(t, makeNetTestMakePipe(p2Mux, p1Mux)) + nettest.TestConn(t, makeNetTestMakePipe(p2Mux, p1Mux, logger)) + + //TODO: Inspect errBuf here } diff --git a/pkg/multiplexing/stream.go b/pkg/multiplexing/stream.go index 74319705e..155b35d3e 100644 --- a/pkg/multiplexing/stream.go +++ b/pkg/multiplexing/stream.go @@ -9,6 +9,7 @@ import ( "sync" "time" + "github.com/mutagen-io/mutagen/pkg/logging" "github.com/mutagen-io/mutagen/pkg/multiplexing/ring" ) @@ -25,6 +26,7 @@ var ( // Stream represents a single multiplexed stream. It implements net.Conn but // also provides a CloseWrite method for half-closures. type Stream struct { + logger *logging.Logger // multiplexer is the parent multiplexer. multiplexer *Multiplexer // identifier is the stream identifier. @@ -377,6 +379,14 @@ func (s *Stream) Write(data []byte) (int, error) { return count, nil } +// mustCloseWrite calls closeWrite and logs the error if there is one. +func (s *Stream) mustCloseWrite(sendCloseWriteMessage bool, logger *logging.Logger) { + err := s.closeWrite(sendCloseWriteMessage) + if err != nil { + logger.Warnf("Unable to close, write message; %s", err.Error()) + } +} + // closeWrite is the internal write closure method. It makes transmission of the // stream close write message optional. func (s *Stream) closeWrite(sendCloseWriteMessage bool) (err error) { @@ -416,7 +426,7 @@ func (s *Stream) CloseWrite() error { func (s *Stream) close(sendCloseMessage bool) (err error) { // Terminate writing if it hasn't been terminated already, but don't queue // a close write message because we're about to send a full close message. - s.closeWrite(false) + s.mustCloseWrite(false, s.logger) // Perform full closure idempotently. s.closeOnce.Do(func() { diff --git a/pkg/must/must.go b/pkg/must/must.go new file mode 100644 index 000000000..f2e7fb4e7 --- /dev/null +++ b/pkg/must/must.go @@ -0,0 +1,176 @@ +package must + +import ( + "fmt" + "io" + "net" + "os" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/spf13/cobra" + "google.golang.org/protobuf/proto" +) + +func Fprint(w io.Writer, logger *logging.Logger, a ...any) { + s := fmt.Sprint(a...) + n, err := fmt.Fprint(w, s) + if err != nil { + logger.Warnf("Unable to Fprint '%s'; %s", s, err.Error()) + } + if n < len(s) { + logger.Warnf("Unable to Fprint all of '%s'; printed only %d of %d bytes", s, n, len(s)) + } +} + +func Close(c io.Closer, logger *logging.Logger) { + err := c.Close() + if err != nil { + logger.Warnf("Unable to close: %s", err.Error()) + } +} + +func Serve(ws interface{ Serve(net.Listener) error }, nl net.Listener, logger *logging.Logger) { + err := ws.Serve(nl) + if err != nil { + logger.Warnf("Unable to serve '%s': %s", nl.Addr(), err.Error()) + } +} + +func WriteString(ws interface{ WriteString(string) (int, error) }, s string, logger *logging.Logger) { + n, err := ws.WriteString(s) + if err != nil { + logger.Warnf("Unable to write string '%s': %s", s, err.Error()) + } + if n < len(s) { + logger.Warnf("Unable to write all of string '%s'; only wrote %d of %d bytes", s, n, len(s)) + } +} + +func CloseWrite(cw interface{ CloseWrite() error }, logger *logging.Logger) { + err := cw.CloseWrite() + if err != nil { + logger.Warnf("Unable to CloseWrite: %s", err.Error()) + } +} + +func Signal(s interface{ Signal(os.Signal) error }, sig os.Signal, logger *logging.Logger) { + err := s.Signal(sig) + if err != nil { + logger.Warnf("Unable to signal '%s': %s", sig, err.Error()) + } +} + +func Terminate(s interface{ Terminate() error }, logger *logging.Logger) { + err := s.Terminate() + if err != nil { + logger.Warnf("Unable to terminate: %s", err.Error()) + } +} + +func Finalize(s interface{ Finalize() error }, logger *logging.Logger) { + err := s.Finalize() + if err != nil { + logger.Warnf("Unable to finalize: %s", err.Error()) + } +} + +func Remove(r interface{ Remove(string) error }, path string, logger *logging.Logger) { + err := r.Remove(path) + if err != nil { + logger.Warnf("Unable to remove '%s': %s", path, err.Error()) + } +} + +func Unlock(locker interface{ Unlock() error }, logger *logging.Logger) { + err := locker.Unlock() + if err != nil { + logger.Warnf("Unable to unlock locker: %s", err.Error()) + } +} + +func OSRemove(name string, logger *logging.Logger) { + err := os.Remove(name) + if err != nil { + logger.Warnf("Unable to remove '%s': %s", name, err.Error()) + } +} + +func Truncate(t interface{ Truncate(int64) error }, size int64, logger *logging.Logger) { + err := t.Truncate(size) + if err != nil { + logger.Warnf("Unable to truncate to size %d: %s", size, err.Error()) + } +} + +func Kill(s interface{ Kill() error }, logger *logging.Logger) { + err := s.Kill() + if err != nil { + logger.Warnf("Unable to Kill: %s", err.Error()) + } +} + +func IOCopy(dst io.Writer, src io.Reader, logger *logging.Logger) { + _, err := io.Copy(dst, src) + if err != nil { + logger.Warnf("Unable to copy from source to destination: %s", err.Error()) + } +} + +func CommandHelp(c *cobra.Command, logger *logging.Logger) { + err := c.Help() + if err != nil { + logger.Warnf("Unable to help: %s", err.Error()) + } +} + +func RemoveFile(rf interface{ RemoveFile(string) error }, name string, logger *logging.Logger) { + err := rf.RemoveFile(name) + if err != nil { + logger.Warnf("Unable to remove file '%s': %s", name, err.Error()) + } +} + +func Shutdown(sd interface{ Shutdown() error }, logger *logging.Logger) { + err := sd.Shutdown() + if err != nil { + logger.Warnf("Unable to shutdown: %s", err.Error()) + } +} + +func Flush(sd interface{ Flush() error }, logger *logging.Logger) { + err := sd.Flush() + if err != nil { + logger.Warnf("Unable to flush: %s", err.Error()) + } +} + +func Release(r interface{ Release() error }, logger *logging.Logger) { + err := r.Release() + if err != nil { + logger.Warnf("Unable to release: %s", err.Error()) + } +} + +func ProtoEncode(e interface { + Encode(message proto.Message) error +}, message proto.Message, logger *logging.Logger) { + err := e.Encode(message) + if err != nil { + logger.Warnf("Unable to proto encode %s: %s", message, err.Error()) + } +} + +func Encode(e interface { + Encode(e any) error +}, value any, logger *logging.Logger) { + err := e.Encode(value) + if err != nil { + logger.Warnf("Unable to encode %v: %s", value, err.Error()) + } +} + +func Succeed(err error, task string, logger *logging.Logger) { + if err != nil { + logger.Warnf("Unable to succeed at %s; %s", task, err.Error()) + } +} diff --git a/pkg/must/must_windows.go b/pkg/must/must_windows.go new file mode 100644 index 000000000..87b7ee198 --- /dev/null +++ b/pkg/must/must_windows.go @@ -0,0 +1,14 @@ +//go:build:windows +package must + +import ( + "github.com/mutagen-io/mutagen/pkg/logging" + "golang.org/x/sys/windows" +) + +func CloseWindowsHandle(wh windows.Handle, logger *logging.Logger) { + err := windows.CloseHandle(wh) + if err != nil { + logger.Warnf("Unable to close handle %d: %s", wh, err.Error()) + } +} diff --git a/pkg/prompting/registry.go b/pkg/prompting/registry.go index e94782248..2eee42e7b 100644 --- a/pkg/prompting/registry.go +++ b/pkg/prompting/registry.go @@ -6,6 +6,7 @@ import ( "sync" "github.com/mutagen-io/mutagen/pkg/identifier" + "github.com/mutagen-io/mutagen/pkg/logging" ) // registryLock is the lock on the global prompter registry. @@ -121,6 +122,14 @@ func Message(identifier, message string) error { return nil } +// MustMessage invokes Message and logs any error received +func MustMessage(identifier, prompt string, logger *logging.Logger) { + err := Message(identifier, prompt) + if err != nil { + logger.Warnf("Unable to prompt '%s' with message '%s'; %s", identifier, prompt, err.Error()) + } +} + // Prompt invokes the Prompt method on a prompter in the global registry. func Prompt(identifier, prompt string) (string, error) { // Grab the holder for the specified prompter. We only need a read lock on diff --git a/pkg/service/daemon/server.go b/pkg/service/daemon/server.go index d9acb14c0..3233dbe19 100644 --- a/pkg/service/daemon/server.go +++ b/pkg/service/daemon/server.go @@ -5,6 +5,7 @@ import ( "time" "github.com/mutagen-io/mutagen/pkg/housekeeping" + "github.com/mutagen-io/mutagen/pkg/logging" "github.com/mutagen-io/mutagen/pkg/mutagen" ) @@ -20,7 +21,7 @@ type Server struct { UnimplementedDaemonServer // Termination is populated with requests from clients invoking the shutdown // method over RPC. It can be ignored by daemon host processes wishing to - // ignore temination requests originating from clients. The channel is + // ignore termination requests originating from clients. The channel is // buffered and non-blocking, so it doesn't need to be serviced by the // daemon host-process at all - additional incoming shutdown requests will // just bounce off once the channel is populated. We do this, instead of @@ -31,10 +32,12 @@ type Server struct { // shutdown is the context cancellation function for the server's internal // operation context. shutdown context.CancelFunc + + logger *logging.Logger } // NewServer creates a new daemon server. -func NewServer() *Server { +func NewServer(logger *logging.Logger) *Server { // Create a cancellable context for daemon background operations. workerCtx, shutdown := context.WithCancel(context.Background()) @@ -43,6 +46,7 @@ func NewServer() *Server { Termination: make(chan struct{}, 1), workerCtx: workerCtx, shutdown: shutdown, + logger: logger, } // Start the housekeeping Goroutine. @@ -56,7 +60,7 @@ func NewServer() *Server { func (s *Server) housekeep() { // Perform an initial housekeeping operation since the ticker won't fire // straight away. - housekeeping.Housekeep() + housekeeping.Housekeep(s.logger) // Create a ticker to regulate housekeeping and defer its shutdown. ticker := time.NewTicker(housekeepingInterval) @@ -68,7 +72,7 @@ func (s *Server) housekeep() { case <-s.workerCtx.Done(): return case <-ticker.C: - housekeeping.Housekeep() + housekeeping.Housekeep(s.logger) } } } diff --git a/pkg/sidecar/permissions.go b/pkg/sidecar/permissions.go index ecf4c0039..a0dfacd3a 100644 --- a/pkg/sidecar/permissions.go +++ b/pkg/sidecar/permissions.go @@ -4,24 +4,26 @@ import ( "fmt" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // SetVolumeOwnershipAndPermissionsIfEmpty will set the ownership and // permissions on a sidecar volume if (and only if) the volume is empty. -func SetVolumeOwnershipAndPermissionsIfEmpty(name string, ownership *filesystem.OwnershipSpecification, mode filesystem.Mode) error { +func SetVolumeOwnershipAndPermissionsIfEmpty(name string, ownership *filesystem.OwnershipSpecification, mode filesystem.Mode, logger *logging.Logger) error { // Open the volumes directory and defer its closure. - volumes, _, err := filesystem.OpenDirectory(volumeMountParent, false) + volumes, _, err := filesystem.OpenDirectory(volumeMountParent, false, logger) if err != nil { return fmt.Errorf("unable to open volumes directory: %w", err) } - defer volumes.Close() + defer must.Close(volumes, logger) // Open the volume mount point and defer its closure. - volume, err := volumes.OpenDirectory(name) + volume, err := volumes.OpenDirectory(name, logger) if err != nil { return fmt.Errorf("unable to open volume root: %w", err) } - defer volume.Close() + defer must.Close(volume, logger) // Check if the volume is empty. If not, then we're done. if contentNames, err := volume.ReadContentNames(); err != nil { diff --git a/pkg/synchronization/controller.go b/pkg/synchronization/controller.go index d0986c559..00a1a7aa3 100644 --- a/pkg/synchronization/controller.go +++ b/pkg/synchronization/controller.go @@ -8,6 +8,7 @@ import ( "sync" "time" + "github.com/mutagen-io/mutagen/pkg/must" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" @@ -107,7 +108,7 @@ func newSession( prompter string, ) (*controller, error) { // Update status. - prompting.Message(prompter, "Creating session...") + prompting.MustMessage(prompter, "Creating session...", logger) // Set the session version. version := DefaultVersion @@ -129,11 +130,11 @@ func newSession( var err error defer func() { if alphaEndpoint != nil { - alphaEndpoint.Shutdown() + must.Shutdown(alphaEndpoint, logger) alphaEndpoint = nil } if betaEndpoint != nil { - betaEndpoint.Shutdown() + must.Shutdown(betaEndpoint, logger) betaEndpoint = nil } }() @@ -200,11 +201,11 @@ func newSession( } // Save components to disk. - if err := encoding.MarshalAndSaveProtobuf(sessionPath, session); err != nil { + if err := encoding.MarshalAndSaveProtobuf(sessionPath, session, logger); err != nil { return nil, fmt.Errorf("unable to save session: %w", err) } - if err := encoding.MarshalAndSaveProtobuf(archivePath, archive); err != nil { - os.Remove(sessionPath) + if err := encoding.MarshalAndSaveProtobuf(archivePath, archive, logger); err != nil { + must.OSRemove(sessionPath, logger) return nil, fmt.Errorf("unable to save archive: %w", err) } @@ -326,7 +327,7 @@ func (c *controller) currentState() *State { // this wait early. func (c *controller) flush(ctx context.Context, prompter string, skipWait bool) error { // Update status. - prompting.Message(prompter, fmt.Sprintf("Forcing synchronization cycle for session %s...", c.session.Identifier)) + prompting.MustMessage(prompter, fmt.Sprintf("Forcing synchronization cycle for session %s...", c.session.Identifier), c.logger) // Lock the controller's lifecycle. c.lifecycleLock.Lock() @@ -417,7 +418,7 @@ func (c *controller) flush(ctx context.Context, prompter string, skipWait bool) // acquire it. func (c *controller) resume(ctx context.Context, prompter string, lifecycleLockHeld bool) error { // Update status. - prompting.Message(prompter, fmt.Sprintf("Resuming session %s...", c.session.Identifier)) + prompting.MustMessage(prompter, fmt.Sprintf("Resuming session %s...", c.session.Identifier), c.logger) // If not already held, acquire the lifecycle lock and defer its release. if !lifecycleLockHeld { @@ -471,7 +472,7 @@ func (c *controller) resume(ctx context.Context, prompter string, lifecycleLockH // Mark the session as unpaused and save it to disk. c.stateLock.Lock() c.session.Paused = false - saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session) + saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session, c.logger) c.stateLock.Unlock() // Attempt to connect to alpha. @@ -567,7 +568,7 @@ func (m controllerHaltMode) description() string { // will not attempt to acquire it. func (c *controller) halt(_ context.Context, mode controllerHaltMode, prompter string, lifecycleLockHeld bool) error { // Update status. - prompting.Message(prompter, fmt.Sprintf("%s session %s...", mode.description(), c.session.Identifier)) + prompting.MustMessage(prompter, fmt.Sprintf("%s session %s...", mode.description(), c.session.Identifier), c.logger) // If not already held, acquire the lifecycle lock and defer its release. if !lifecycleLockHeld { @@ -602,7 +603,7 @@ func (c *controller) halt(_ context.Context, mode controllerHaltMode, prompter s // Mark the session as paused and save it. c.stateLock.Lock() c.session.Paused = true - saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session) + saveErr := encoding.MarshalAndSaveProtobuf(c.sessionPath, c.session, c.logger) c.stateLock.Unlock() if saveErr != nil { return fmt.Errorf("unable to save session: %w", saveErr) @@ -651,7 +652,7 @@ func (c *controller) reset(ctx context.Context, prompter string) error { // Reset the session archive on disk. c.logger.Infof("Resetting ancestor") archive := &core.Archive{} - if err := encoding.MarshalAndSaveProtobuf(c.archivePath, archive); err != nil { + if err := encoding.MarshalAndSaveProtobuf(c.archivePath, archive, c.logger); err != nil { return fmt.Errorf("unable to clear session history: %w", err) } @@ -683,10 +684,10 @@ func (c *controller) run(ctx context.Context, alpha, beta Endpoint) { // Shutdown any endpoints. These might be non-nil if the run loop was // cancelled while partially connected rather than after sync failure. if alpha != nil { - alpha.Shutdown() + must.Shutdown(alpha, c.logger) } if beta != nil { - beta.Shutdown() + must.Shutdown(beta, c.logger) } // Reset the state. @@ -801,9 +802,9 @@ func (c *controller) run(ctx context.Context, alpha, beta Endpoint) { c.stateLock.UnlockWithoutNotify() // Shutdown the endpoints. - alpha.Shutdown() + must.Shutdown(alpha, c.logger) alpha = nil - beta.Shutdown() + must.Shutdown(beta, c.logger) beta = nil // If synchronization failed due a halting error, then wait for the @@ -1276,7 +1277,7 @@ func (c *controller) synchronize(ctx context.Context, alpha, beta Endpoint) erro c.stateLock.Unlock() return nil } - receiver = rsync.NewMonitoringReceiver(receiver, filteredPaths, signatures, monitor) + receiver = rsync.NewMonitoringReceiver(receiver, filteredPaths, signatures, monitor, c.logger) receiver = rsync.NewPreemptableReceiver(ctx, receiver) if err = beta.Supply(filteredPaths, signatures, receiver); err != nil { return fmt.Errorf("unable to stage files on alpha: %w", err) @@ -1314,7 +1315,7 @@ func (c *controller) synchronize(ctx context.Context, alpha, beta Endpoint) erro c.stateLock.Unlock() return nil } - receiver = rsync.NewMonitoringReceiver(receiver, filteredPaths, signatures, monitor) + receiver = rsync.NewMonitoringReceiver(receiver, filteredPaths, signatures, monitor, c.logger) receiver = rsync.NewPreemptableReceiver(ctx, receiver) if err = alpha.Supply(filteredPaths, signatures, receiver); err != nil { return fmt.Errorf("unable to stage files on beta: %w", err) @@ -1402,7 +1403,7 @@ func (c *controller) synchronize(ctx context.Context, alpha, beta Endpoint) erro // Save the ancestor. c.logger.Debug("Saving ancestor") archive.Content = ancestor - if err := encoding.MarshalAndSaveProtobuf(c.archivePath, archive); err != nil { + if err := encoding.MarshalAndSaveProtobuf(c.archivePath, archive, c.logger); err != nil { return fmt.Errorf("unable to save ancestor: %w", err) } } diff --git a/pkg/synchronization/core/scan.go b/pkg/synchronization/core/scan.go index a93b4a14f..57215225a 100644 --- a/pkg/synchronization/core/scan.go +++ b/pkg/synchronization/core/scan.go @@ -19,6 +19,8 @@ import ( "github.com/mutagen-io/mutagen/pkg/filesystem" "github.com/mutagen-io/mutagen/pkg/filesystem/behavior" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/stream" "github.com/mutagen-io/mutagen/pkg/synchronization/core/fastpath" "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore" @@ -136,6 +138,8 @@ type scanner struct { symbolicLinks uint64 // totalFileSize is the total size of all synchronizable files encountered. totalFileSize uint64 + + logger *logging.Logger } // file performs processing of a file entry. Exactly one of parent or file will @@ -197,7 +201,7 @@ func (s *scanner) file( Problem: fmt.Errorf("unable to open file: %w", err).Error(), }, nil } - defer file.Close() + defer must.Close(file, s.logger) } // Reset the hash state. @@ -339,7 +343,7 @@ func (s *scanner) directory( // If the directory is not yet opened, then open it and defer its closure. if directory == nil { - if d, err := parent.OpenDirectory(metadata.Name); err != nil { + if d, err := parent.OpenDirectory(metadata.Name, s.logger); err != nil { if os.IsNotExist(err) { return nil, err } @@ -349,7 +353,7 @@ func (s *scanner) directory( }, nil } else { directory = d - defer directory.Close() + defer must.Close(directory, s.logger) } } @@ -657,6 +661,7 @@ func Scan( probeMode behavior.ProbeMode, symbolicLinkMode SymbolicLinkMode, permissionsMode PermissionsMode, + logger *logging.Logger, ) (*Snapshot, *Cache, ignore.IgnoreCache, error) { // Verify that the symbolic link mode is valid for this platform. if symbolicLinkMode == SymbolicLinkMode_SymbolicLinkModePOSIXRaw && runtime.GOOS == "windows" { @@ -665,7 +670,7 @@ func Scan( // Open the root and defer its closure. We explicitly disallow symbolic // links at the root path, though intermediate symbolic links are fine. - rootObject, metadata, err := filesystem.Open(root, false) + rootObject, metadata, err := filesystem.Open(root, false, logger) if err != nil { if os.IsNotExist(err) { return &Snapshot{}, &Cache{}, nil, nil @@ -673,7 +678,7 @@ func Scan( return nil, nil, nil, fmt.Errorf("unable to open synchronization root: %w", err) } } - defer rootObject.Close() + defer must.Close(rootObject, logger) // Determine the root kind and extract the underlying object. var rootKind EntryKind @@ -713,7 +718,7 @@ func Scan( // Check executability preservation behavior. if cachedPreservesOk { preservesExecutability = cachedPreserves - } else if preserves, usedFiles, err := behavior.PreservesExecutability(directoryRoot, probeMode); err != nil { + } else if preserves, usedFiles, err := behavior.PreservesExecutability(directoryRoot, probeMode, logger); err != nil { return nil, nil, nil, fmt.Errorf("unable to probe root executability preservation behavior: %w", err) } else { preservesExecutability = preserves @@ -723,7 +728,7 @@ func Scan( // Check Unicode decomposition behavior. if cachedDecomposesOk { decomposesUnicode = cachedDecomposes - } else if decomposes, usedFiles, err := behavior.DecomposesUnicode(directoryRoot, probeMode); err != nil { + } else if decomposes, usedFiles, err := behavior.DecomposesUnicode(directoryRoot, probeMode, logger); err != nil { return nil, nil, nil, fmt.Errorf("unable to probe root Unicode decomposition behavior: %w", err) } else { decomposesUnicode = decomposes @@ -759,7 +764,7 @@ func Scan( // Check executability preservation behavior for the parent directory. if cachedPreservesOk { preservesExecutability = cachedPreserves - } else if preserves, usedFiles, err := behavior.PreservesExecutabilityByPath(parent, probeMode); err != nil { + } else if preserves, usedFiles, err := behavior.PreservesExecutabilityByPath(parent, probeMode, logger); err != nil { return nil, nil, nil, fmt.Errorf("unable to probe parent executability preservation behavior: %w", err) } else { preservesExecutability = preserves @@ -769,7 +774,7 @@ func Scan( // Check Unicode decomposition behavior for the parent directory. if cachedDecomposesOk { decomposesUnicode = cachedDecomposes - } else if decomposes, usedFiles, err := behavior.DecomposesUnicodeByPath(parent, probeMode); err != nil { + } else if decomposes, usedFiles, err := behavior.DecomposesUnicodeByPath(parent, probeMode, logger); err != nil { return nil, nil, nil, fmt.Errorf("unable to probe parent Unicode decomposition behavior: %w", err) } else { decomposesUnicode = decomposes @@ -870,6 +875,7 @@ func Scan( deviceID: metadata.DeviceID, recomposeUnicode: decomposesUnicode, preservesExecutability: preservesExecutability, + logger: logger, } // Handle the scan based on the root type. diff --git a/pkg/synchronization/core/scan_test.go b/pkg/synchronization/core/scan_test.go index 1f89477b8..eebc83562 100644 --- a/pkg/synchronization/core/scan_test.go +++ b/pkg/synchronization/core/scan_test.go @@ -1,6 +1,7 @@ package core import ( + "bytes" "context" "errors" "fmt" @@ -13,6 +14,8 @@ import ( "github.com/mutagen-io/mutagen/pkg/filesystem" "github.com/mutagen-io/mutagen/pkg/filesystem/behavior" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore" dockerignore "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore/docker" mutagenignore "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore/mutagen" @@ -73,6 +76,8 @@ func testingSnapshotStatistics(entry *Entry, cache *Cache) (directoryCount, file // TestScan tests Scan. func TestScan(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create contexts to use for tests. backgroundCtx := context.Background() cancelledCtx, cancel := context.WithCancel(backgroundCtx) @@ -83,7 +88,7 @@ func TestScan(t *testing.T) { // but not earlier. Linux doesn't seem to have ever supported them and gives // a strange error code when trying to create them. More information can be // found here: https://lwn.net/Articles/551224/. The best option is just to - // test for support. Manpages don't seem to accurately describe support. + // test for support. Man pages don't seem to accurately describe support. emptySymbolicLinksSupported := os.Symlink("", filepath.Join(t.TempDir(), "l")) == nil // Define test cases. @@ -264,11 +269,11 @@ func TestScan(t *testing.T) { } unixListener, ok := listener.(*net.UnixListener) if !ok { - listener.Close() + must.Close(listener, logger) return errors.New("listener was not a Unix listener") } unixListener.SetUnlinkOnClose(false) - unixListener.Close() + must.Close(unixListener, logger) return nil }, nil, @@ -524,6 +529,7 @@ func TestScan(t *testing.T) { baselineContentMap: test.baselineContentMap, tweak: test.tweak, untweak: test.untweak, + logger: logger, } root, err := generator.generate() if err != nil { @@ -577,6 +583,7 @@ func TestScan(t *testing.T) { behavior.ProbeMode_ProbeModeProbe, test.symbolicLinkMode, test.permissionsMode, + logger, ) if test.expectFailure { if err == nil { @@ -659,6 +666,7 @@ func TestScan(t *testing.T) { behavior.ProbeMode_ProbeModeProbe, test.symbolicLinkMode, test.permissionsMode, + logger, ) // Handle scan failure (which isn't expected at this point). @@ -731,6 +739,7 @@ func TestScan(t *testing.T) { behavior.ProbeMode_ProbeModeProbe, test.symbolicLinkMode, test.permissionsMode, + logger, ) // Handle scan failure (which isn't expected at this point). @@ -830,6 +839,7 @@ func TestScan(t *testing.T) { behavior.ProbeMode_ProbeModeProbe, test.symbolicLinkMode, test.permissionsMode, + logger, ) // Handle scan failure (which isn't expected at this point). @@ -922,6 +932,8 @@ func TestScan(t *testing.T) { // generate (and mount/unmount) disk images in Go, meaning that this test is // hard to do as a tweak/untweak operation in a standard scan test. func TestScanCrossFilesystemBoundary(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // If we don't have a filesystem mounted within another filesystem, then // skip this test. crossing := os.Getenv("MUTAGEN_TEST_SUBFS_ROOT") @@ -949,6 +961,7 @@ func TestScanCrossFilesystemBoundary(t *testing.T) { behavior.ProbeMode_ProbeModeProbe, SymbolicLinkMode_SymbolicLinkModePortable, PermissionsMode_PermissionsModePortable, + logger, ) if err != nil { t.Fatalf("unable to perform scan: %v", err) diff --git a/pkg/synchronization/core/testing_content_test.go b/pkg/synchronization/core/testing_content_test.go index e9ad41d71..6389983a1 100644 --- a/pkg/synchronization/core/testing_content_test.go +++ b/pkg/synchronization/core/testing_content_test.go @@ -6,6 +6,9 @@ import ( "fmt" "os" "path/filepath" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // testingContentManager generates and removes test content on disk using @@ -39,6 +42,8 @@ type testingContentManager struct { // generated content root and can attempt to remove content or at least make // it suitable for removal by os.RemoveAll. untweak func(string) error + + logger *logging.Logger } // generate generates test content on disk. It returns the path to the generated @@ -63,7 +68,10 @@ func (g *testingContentManager) generate() (string, error) { // parent directory when we return. defer func() { if !successful { - os.RemoveAll(g.parent) + must.Succeed(os.RemoveAll(g.parent), + fmt.Sprintf("remove all files from '%s'", g.parent), + g.logger, + ) g.parent = "" } }() @@ -86,6 +94,7 @@ func (g *testingContentManager) generate() (string, error) { storage: g.parent, contentMap: g.baselineContentMap, hasher: newTestingHasher(), + logger: g.logger, } results, problems, missingFiles := Transition( context.Background(), @@ -98,6 +107,7 @@ func (g *testingContentManager) generate() (string, error) { nil, false, provider, + g.logger, ) if missingFiles { return "", errors.New("content map missing file definitions") @@ -139,7 +149,10 @@ func (g *testingContentManager) remove() error { // try a removal operation to attempt cleanup. if g.untweak != nil { if err := g.untweak(root); err != nil { - os.RemoveAll(parent) + must.Succeed(os.RemoveAll(parent), + fmt.Sprintf("remove all files from '%s'", parent), + g.logger, + ) return fmt.Errorf("unable to untweak content root: %w", err) } } diff --git a/pkg/synchronization/core/testing_provider_test.go b/pkg/synchronization/core/testing_provider_test.go index f83258669..9d4a04d64 100644 --- a/pkg/synchronization/core/testing_provider_test.go +++ b/pkg/synchronization/core/testing_provider_test.go @@ -9,6 +9,9 @@ import ( "os" "path/filepath" "sync" + + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // testingContentMap is an in-memory content map type used by testProvider. @@ -25,6 +28,8 @@ type testingProvider struct { hasherLock sync.Mutex // hasher is the hasher to use when verifying content. hasher hash.Hash + + logger *logging.Logger } // Provide implements the Provider interface for testProvider. @@ -56,9 +61,9 @@ func (p *testingProvider) Provide(path string, digest []byte) (string, error) { // Write content. _, err = file.Write(content) - file.Close() + must.Close(file, p.logger) if err != nil { - os.Remove(file.Name()) + must.OSRemove(file.Name(), p.logger) return "", fmt.Errorf("unable to write file contents: %w", err) } diff --git a/pkg/synchronization/core/transition.go b/pkg/synchronization/core/transition.go index bb52039c4..148ca5a03 100644 --- a/pkg/synchronization/core/transition.go +++ b/pkg/synchronization/core/transition.go @@ -15,6 +15,8 @@ import ( "golang.org/x/text/unicode/norm" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/stream" "github.com/mutagen-io/mutagen/pkg/synchronization/core/fastpath" ) @@ -87,6 +89,8 @@ type transitioner struct { // providerMissingFiles indicates that the staged file provider returned an // os.IsNotExist error for at least one file that was expected to be staged. providerMissingFiles bool + + logger *logging.Logger } // recordProblem records a new problem. @@ -169,7 +173,7 @@ func (t *transitioner) walkToParentAndComputeLeafName( // Open the parent. We do allow the parent path to be a symbolic link // since we allow symbolic link resolution for parent components of the // synchronization root (just not at the synchronization root itself). - if rootParent, _, err := filesystem.OpenDirectory(rootParentPath, true); err != nil { + if rootParent, _, err := filesystem.OpenDirectory(rootParentPath, true, t.logger); err != nil { return nil, "", fmt.Errorf("unable to open synchronization root parent directory: %w", err) } else { return rootParent, rootName, nil @@ -183,7 +187,7 @@ func (t *transitioner) walkToParentAndComputeLeafName( // Open the root path. If it's not a directory, then this operation isn't // valid. - parent, _, err := filesystem.OpenDirectory(t.root, false) + parent, _, err := filesystem.OpenDirectory(t.root, false, t.logger) if err != nil { return nil, "", fmt.Errorf("unable to open synchronization root: %w", err) } @@ -193,10 +197,10 @@ func (t *transitioner) walkToParentAndComputeLeafName( for _, component := range parentComponents { // Verify that the next component exists with the proper casing. if found, err := t.nameExistsInDirectoryWithProperCase(component, parent); err != nil { - parent.Close() + must.Close(parent, t.logger) return nil, "", fmt.Errorf("unable to verify parent path casing: %w", err) } else if !found { - parent.Close() + must.Close(parent, t.logger) return nil, "", errors.New("parent path does not exist or has incorrect casing") } @@ -213,11 +217,11 @@ func (t *transitioner) walkToParentAndComputeLeafName( // correct casing on the next synchronization cycle. // Open the next directory. - if p, err := parent.OpenDirectory(component); err != nil { - parent.Close() + if p, err := parent.OpenDirectory(component, t.logger); err != nil { + must.Close(parent, t.logger) return nil, "", fmt.Errorf("unable to open parent component: %w", err) } else { - parent.Close() + must.Close(parent, t.logger) parent = p } } @@ -226,10 +230,10 @@ func (t *transitioner) walkToParentAndComputeLeafName( // requested. if validateLeafCasing { if found, err := t.nameExistsInDirectoryWithProperCase(leafName, parent); err != nil { - parent.Close() + must.Close(parent, t.logger) return nil, "", fmt.Errorf("unable to verify path leaf name casing: %w", err) } else if !found { - parent.Close() + must.Close(parent, t.logger) return nil, "", errors.New("leaf name does not exist or has incorrect casing") } } @@ -366,7 +370,7 @@ func (t *transitioner) removeSymbolicLink(parent *filesystem.Directory, name, pa func (t *transitioner) removeDirectory(parent *filesystem.Directory, name, path string, expected *Entry) bool { // Open the directory itself. We don't defer its closure because we'll need // to explicitly close it before being able to remove it. - directory, err := parent.OpenDirectory(name) + directory, err := parent.OpenDirectory(name, t.logger) if err != nil { t.recordProblem(path, fmt.Errorf("unable to open directory: %w", err)) return false @@ -375,7 +379,7 @@ func (t *transitioner) removeDirectory(parent *filesystem.Directory, name, path // List the contents for this directory. contents, err := directory.ReadContents() if err != nil { - directory.Close() + must.Close(directory, t.logger) t.recordProblem(path, fmt.Errorf("unable to read directory contents: %w", err)) return false } @@ -470,7 +474,7 @@ ContentLoop: } // Close the directory. - directory.Close() + must.Close(directory, t.logger) // RACE: There is a race condition here that is platform-dependent. On POSIX // platforms, the directory handle we had open (and whose content we @@ -519,7 +523,7 @@ func (t *transitioner) remove(path string, entry *Entry) *Entry { t.recordProblem(path, fmt.Errorf("unable to walk to transition root: %w", err)) return entry } - defer parent.Close() + defer must.Close(parent, t.logger) // Handle removal based on type. if entry.Kind == EntryKind_Directory { @@ -641,7 +645,7 @@ func (t *transitioner) findAndMoveStagedFileIntoPlace( // file handle is open. temporaryName, temporary, err := parent.CreateTemporaryFile(crossDeviceRenameTemporaryNamePrefix) if err != nil { - stagedFile.Close() + must.Close(stagedFile, t.logger) return fmt.Errorf("unable to create temporary file for cross-device rename: %w", err) } @@ -656,12 +660,12 @@ func (t *transitioner) findAndMoveStagedFileIntoPlace( _, copyErr := io.CopyBuffer(preemptableTemporary, stagedFile, t.copyBuffer) // Close out files. - stagedFile.Close() - temporary.Close() + must.Close(stagedFile, t.logger) + must.Close(temporary, t.logger) // If there was a copy error, then remove the temporary and abort. if copyErr != nil { - parent.RemoveFile(temporaryName) + must.RemoveFile(parent, temporaryName, t.logger) if copyErr == stream.ErrWritePreempted { return errTransitionCancelled } @@ -670,19 +674,19 @@ func (t *transitioner) findAndMoveStagedFileIntoPlace( // Set permissions on the temporary file. if err := parent.SetPermissions(temporaryName, t.defaultOwnership, mode); err != nil { - parent.RemoveFile(temporaryName) + must.RemoveFile(parent, temporaryName, t.logger) return fmt.Errorf("unable to set intermediate file permissions: %w", err) } // Rename the file. if err := filesystem.Rename(parent, temporaryName, parent, name, replace); err != nil { - parent.RemoveFile(temporaryName) + must.RemoveFile(parent, temporaryName, t.logger) return fmt.Errorf("unable to relocate intermediate file: %w", err) } // Remove the staged file. We don't bother checking for errors because // there's not much we can or need to do about them at this point. - os.Remove(stagedPath) + must.OSRemove(stagedPath, t.logger) // Success. return nil @@ -697,7 +701,7 @@ func (t *transitioner) swapFile(path string, oldEntry, newEntry *Entry) error { if err != nil { return fmt.Errorf("unable to walk to transition root: %w", err) } - defer parent.Close() + defer must.Close(parent, t.logger) // Ensure that the existing entry hasn't been modified from what we're // expecting. @@ -832,12 +836,12 @@ func (t *transitioner) createDirectory(parent *filesystem.Directory, name, path created.Contents = make(map[string]*Entry, len(target.Contents)) // Open the directory. - if d, err := parent.OpenDirectory(name); err != nil { + if d, err := parent.OpenDirectory(name, t.logger); err != nil { t.recordProblem(path, fmt.Errorf("unable to open new directory: %w", err)) return created } else { directory = d - defer directory.Close() + defer must.Close(directory, t.logger) } } @@ -904,7 +908,7 @@ func (t *transitioner) create(path string, target *Entry) *Entry { t.recordProblem(path, fmt.Errorf("unable to walk to transition root parent: %w", err)) return nil } - defer parent.Close() + defer must.Close(parent, t.logger) // Handle creation based on type. if target.Kind == EntryKind_Directory { @@ -946,6 +950,7 @@ func Transition( defaultOwnership *filesystem.OwnershipSpecification, recomposeUnicode bool, provider Provider, + logger *logging.Logger, ) ([]*Entry, []*Problem, bool) { // Extract the cancellation channel. cancelled := ctx.Done() @@ -962,6 +967,7 @@ func Transition( copyBuffer: make([]byte, transitionCopyBufferSize), recomposeUnicode: recomposeUnicode, provider: provider, + logger: logger, } // Set up results. diff --git a/pkg/synchronization/core/transition_test.go b/pkg/synchronization/core/transition_test.go index 518e80848..3fc9a5d41 100644 --- a/pkg/synchronization/core/transition_test.go +++ b/pkg/synchronization/core/transition_test.go @@ -1,6 +1,7 @@ package core import ( + "bytes" "context" "os" "path/filepath" @@ -9,6 +10,7 @@ import ( "time" "github.com/mutagen-io/mutagen/pkg/filesystem/behavior" + "github.com/mutagen-io/mutagen/pkg/logging" mutagenignore "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore/mutagen" ) @@ -51,6 +53,8 @@ func testingNWildcardEntries(n uint) []*Entry { // TestTransition tests Transition. func TestTransition(t *testing.T) { + logger := logging.NewLogger(logging.LevelError, &bytes.Buffer{}) + // Create contexts to use for tests. backgroundCtx := context.Background() cancelledCtx, cancel := context.WithCancel(backgroundCtx) @@ -705,6 +709,7 @@ func TestTransition(t *testing.T) { baselineContentMap: test.baselineContentMap, tweak: test.tweak, untweak: test.untweak, + logger: logger, } root, err := generator.generate() if err != nil { @@ -741,6 +746,7 @@ func TestTransition(t *testing.T) { behavior.ProbeMode_ProbeModeProbe, test.symbolicLinkMode, PermissionsMode_PermissionsModePortable, + logger, ) if err != nil { t.Errorf("%s: unable to perform scan of baseline on %s filesystem: %v", @@ -776,6 +782,7 @@ func TestTransition(t *testing.T) { nil, snapshot.DecomposesUnicode, provider, + logger, ) // Check results. diff --git a/pkg/synchronization/endpoint/local/endpoint.go b/pkg/synchronization/endpoint/local/endpoint.go index 1bf31506b..a7fe199a7 100644 --- a/pkg/synchronization/endpoint/local/endpoint.go +++ b/pkg/synchronization/endpoint/local/endpoint.go @@ -15,6 +15,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/filesystem/behavior" "github.com/mutagen-io/mutagen/pkg/filesystem/watching" "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/sidecar" "github.com/mutagen-io/mutagen/pkg/state" "github.com/mutagen-io/mutagen/pkg/synchronization" @@ -439,6 +440,7 @@ func NewEndpoint( filepath.Base(sidecarVolumeMountPoint), defaultOwnership, defaultDirectoryMode, + logger, ); err != nil { return nil, fmt.Errorf("unable to set ownership and permissions for sidecar volume: %w", err) } @@ -488,6 +490,7 @@ func NewEndpoint( hideStagingRoot, maximumStagingFileSize, hasherFactory, + logger, ), } @@ -575,7 +578,7 @@ func (e *endpoint) saveCache(ctx context.Context, cachePath string, signal <-cha // Save the cache. e.logger.Debug("Saving cache to disk") - if err := encoding.MarshalAndSaveProtobuf(cachePath, e.cache); err != nil { + if err := encoding.MarshalAndSaveProtobuf(cachePath, e.cache, e.logger); err != nil { e.logger.Error("Cache save failed:", err) e.cacheWriteError = err e.unlockScanLock() @@ -628,7 +631,7 @@ func (e *endpoint) watchPoll(ctx context.Context, pollingInterval uint32, nonRec watchErrors = watcher.Errors() defer func() { if watcher != nil { - watcher.Terminate() + must.Terminate(watcher, logger) } }() } @@ -697,7 +700,7 @@ func (e *endpoint) watchPoll(ctx context.Context, pollingInterval uint32, nonRec // case we don't want to trigger this code again on a nil // watcher). We'll allow event channels to continue since they // may contain residual events. - watcher.Terminate() + must.Terminate(watcher, logger) watcher = nil watchErrors = nil @@ -816,7 +819,7 @@ func (e *endpoint) watchRecursive(ctx context.Context, pollingInterval uint32) { var watcher watching.RecursiveWatcher defer func() { if watcher != nil { - watcher.Terminate() + must.Terminate(watcher, e.logger) } }() @@ -930,7 +933,7 @@ WatchEstablishment: e.pollSignal.Strobe() // Terminate the watcher. - watcher.Terminate() + must.Terminate(watcher, logger) watcher = nil // If the watcher failed due to an internal event overflow, then @@ -1010,6 +1013,7 @@ func (e *endpoint) scan(ctx context.Context, baseline *core.Snapshot, recheckPat e.probeMode, e.symbolicLinkMode, e.permissionsMode, + e.logger, ) if err != nil { return err @@ -1130,7 +1134,7 @@ func (e *endpoint) stageFromRoot( if err != nil { return false } - defer source.Close() + defer must.Close(source, e.logger) // Create a staging sink. We explicitly manage its closure below. sink, err := e.stager.Sink(path) @@ -1140,7 +1144,7 @@ func (e *endpoint) stageFromRoot( // Copy data to the sink and close it, then check for copy errors. _, err = io.Copy(sink, source) - sink.Close() + must.Close(sink, e.logger) if err != nil { return false } @@ -1205,8 +1209,8 @@ func (e *endpoint) Stage(paths []string, digests [][]byte) ([]string, []*rsync.S // Create an opener that we can use file opening and defer its closure. We // can't cache this across synchronization cycles since its path references // may become invalidated or may prevent modifications. - opener := filesystem.NewOpener(e.root) - defer opener.Close() + opener := filesystem.NewOpener(e.root, e.logger) + defer must.Close(opener, e.logger) // Filter the path list by looking for files that we can source locally. // @@ -1253,16 +1257,16 @@ func (e *endpoint) Stage(paths []string, digests [][]byte) ([]string, []*rsync.S } else if base, _, err := opener.OpenFile(path); err != nil { signatures[p] = emptySignature } else if signature, err := engine.Signature(base, 0); err != nil { - base.Close() + must.Close(base, e.logger) signatures[p] = emptySignature } else { - base.Close() + must.Close(base, e.logger) signatures[p] = signature } } // Create a receiver. - receiver, err := rsync.NewReceiver(e.root, filteredPaths, signatures, e.stager) + receiver, err := rsync.NewReceiver(e.root, filteredPaths, signatures, e.stager, e.logger) if err != nil { return nil, nil, nil, fmt.Errorf("unable to create rsync receiver: %w", err) } @@ -1273,7 +1277,7 @@ func (e *endpoint) Stage(paths []string, digests [][]byte) ([]string, []*rsync.S // Supply implements the supply method for local endpoints. func (e *endpoint) Supply(paths []string, signatures []*rsync.Signature, receiver rsync.Receiver) error { - return rsync.Transmit(e.root, paths, signatures, receiver) + return rsync.Transmit(e.root, paths, signatures, receiver, e.logger) } // Transition implements the Transition method for local endpoints. @@ -1344,6 +1348,7 @@ func (e *endpoint) Transition(ctx context.Context, transitions []*core.Change) ( e.defaultOwnership, e.lastReturnedScanSnapshotDecomposesUnicode, e.stager, + e.logger, ) e.lockScanLock(context.Background()) @@ -1383,7 +1388,7 @@ func (e *endpoint) Transition(ctx context.Context, transitions []*core.Change) ( // will be automatically re-enabled on the next polling operation. If // filesystem watching is disabled, then so is acceleration, and thus // there's no way that a stale scan could be returned. Note that, in the - // recurisve watching case, we only need to include transition roots because + // recursive watching case, we only need to include transition roots because // transition operations will always change the root type and thus scanning // will see any potential content changes. Also, we don't need to worry // about delayed watcher failure reporting due to external (non-transition) @@ -1432,7 +1437,7 @@ func (e *endpoint) Transition(ctx context.Context, transitions []*core.Change) ( // staging directory? It could be due to an easily correctable error, at // which point you wouldn't want to restage if you're talking about lots of // files. - e.stager.Finalize() + must.Finalize(e.stager, e.logger) // Done. return results, problems, stagerMissingFiles, nil diff --git a/pkg/synchronization/endpoint/local/staging/stager.go b/pkg/synchronization/endpoint/local/staging/stager.go index 0cfcecd12..8b7c21b5d 100644 --- a/pkg/synchronization/endpoint/local/staging/stager.go +++ b/pkg/synchronization/endpoint/local/staging/stager.go @@ -4,6 +4,7 @@ import ( "hash" "io" + "github.com/mutagen-io/mutagen/pkg/logging" "github.com/mutagen-io/mutagen/pkg/synchronization/endpoint/local/staging/store" ) @@ -11,12 +12,16 @@ import ( // store to stage files. type Stager struct { // store is the stager's underlying store. - store *store.Store + store *store.Store + logger *logging.Logger } // NewStager creates a new stager. -func NewStager(root string, hideRoot bool, maximumFileSize uint64, hasherFactory func() hash.Hash) *Stager { - return &Stager{store.NewStore(root, hideRoot, maximumFileSize, hasherFactory)} +func NewStager(root string, hideRoot bool, maximumFileSize uint64, hasherFactory func() hash.Hash, logger *logging.Logger) *Stager { + return &Stager{ + store: store.NewStore(root, hideRoot, maximumFileSize, hasherFactory, logger), + logger: logger, + } } // Initialize implements local.stager.Initialize. @@ -31,7 +36,7 @@ func (s *Stager) Contains(path string, digest []byte) (bool, error) { // Sink implements rsync.Sinker.Sink. func (s *Stager) Sink(path string) (io.WriteCloser, error) { - storage, err := s.store.Allocate() + storage, err := s.store.Allocate(s.logger) if err != nil { return nil, err } diff --git a/pkg/synchronization/endpoint/local/staging/store/store.go b/pkg/synchronization/endpoint/local/staging/store/store.go index 16dcd0107..b871baeed 100644 --- a/pkg/synchronization/endpoint/local/staging/store/store.go +++ b/pkg/synchronization/endpoint/local/staging/store/store.go @@ -15,6 +15,8 @@ import ( "github.com/zeebo/xxh3" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/stream" ) @@ -68,10 +70,12 @@ type Store struct { // prefixExists tracks whether or not individual prefix directories exist. // It is indexed on the byte value corresponding to the prefix directory. prefixExists [256]bool + + logger *logging.Logger } // NewStore creates a new store instance with the specified parameters. -func NewStore(root string, hidden bool, maximumFileSize uint64, contentHasherFactory func() hash.Hash) *Store { +func NewStore(root string, hidden bool, maximumFileSize uint64, contentHasherFactory func() hash.Hash, logger *logging.Logger) *Store { return &Store{ root: root, hidden: hidden, @@ -91,6 +95,7 @@ func NewStore(root string, hidden bool, maximumFileSize uint64, contentHasherFac return xxh3.New() }, }, + logger: logger, } } @@ -182,7 +187,7 @@ func (s *Store) Initialize() error { } // Allocate allocates temporary storage for receiving data. -func (s *Store) Allocate() (*Storage, error) { +func (s *Store) Allocate(logger *logging.Logger) (*Storage, error) { // Verify that the store is initialized. if !s.initialized { return nil, errStoreUninitialized @@ -212,6 +217,7 @@ func (s *Store) Allocate() (*Storage, error) { hasher: hasher, writer: writer, buffer: buffer, + logger: logger, }, nil } @@ -230,7 +236,7 @@ func (s *Store) target(path string, digest []byte) (string, string) { pathHasher.Reset() // Compute the path digest. - pathHasher.WriteString(path) + must.WriteString(pathHasher, path, s.logger) pathDigest := pathHasher.Sum128() // Return the path hasher to the pool. @@ -340,6 +346,8 @@ type Storage struct { buffer *bufio.Writer // currentSize is the number of bytes that have been written to the file. currentSize uint64 + + logger *logging.Logger } // Write implements io.Writer.Write for the storage. @@ -383,7 +391,7 @@ func (s *Storage) Commit(path string) error { // Verify that the content digest has sufficient length. if len(digest) == 0 { - os.Remove(s.storage.Name()) + must.OSRemove(s.storage.Name(), s.logger) return errDigestEmpty } @@ -396,7 +404,7 @@ func (s *Storage) Commit(path string) error { if !s.store.prefixExists[prefixByte] { if err := os.Mkdir(filepath.Join(s.store.root, prefix), 0700); err != nil { s.store.prefixLock.Unlock() - os.Remove(s.storage.Name()) + must.OSRemove(s.storage.Name(), s.logger) return fmt.Errorf("unable to create prefix directory (%s): %w", prefix, err) } s.store.prefixExists[prefixByte] = true @@ -405,7 +413,7 @@ func (s *Storage) Commit(path string) error { // Relocate the temporary file to its target destination. if err := filesystem.Rename(nil, s.storage.Name(), nil, target, true); err != nil { - os.Remove(s.storage.Name()) + must.OSRemove(s.storage.Name(), s.logger) return fmt.Errorf("unable to relocate storage: %w", err) } diff --git a/pkg/synchronization/endpoint/remote/client.go b/pkg/synchronization/endpoint/remote/client.go index e588d8cb8..1cd76af27 100644 --- a/pkg/synchronization/endpoint/remote/client.go +++ b/pkg/synchronization/endpoint/remote/client.go @@ -7,6 +7,7 @@ import ( "fmt" "io" + "github.com/mutagen-io/mutagen/pkg/must" "google.golang.org/protobuf/proto" "github.com/mutagen-io/mutagen/pkg/encoding" @@ -59,7 +60,7 @@ func NewEndpoint( // Perform the compression handshake. if err := compression.ClientHandshake(stream, compressionAlgorithm); err != nil { - stream.Close() + must.Close(stream, logger) return nil, fmt.Errorf("compression handshake failed: %w", err) } @@ -93,7 +94,7 @@ func NewEndpoint( var successful bool defer func() { if !successful { - closer.Close() + must.Close(closer, logger) } }() @@ -417,7 +418,7 @@ func (c *endpointClient) Stage(paths []string, digests [][]byte) ([]string, []*r // Create an encoding receiver that can transmit rsync operations to the // remote. encoder := &protobufRsyncEncoder{encoder: c.encoder, flusher: c.flusher} - receiver := rsync.NewEncodingReceiver(encoder) + receiver := rsync.NewEncodingReceiver(encoder, c.logger) // Success. return requiredPaths, response.Signatures, receiver, nil @@ -451,7 +452,7 @@ func (c *endpointClient) Supply(paths []string, signatures []*rsync.Signature, r // and forward them to the receiver. If this operation completes // successfully, supplying is complete and successful. decoder := &protobufRsyncDecoder{decoder: c.decoder} - if err := rsync.DecodeToReceiver(decoder, uint64(len(paths)), receiver); err != nil { + if err := rsync.DecodeToReceiver(decoder, uint64(len(paths)), receiver, c.logger); err != nil { return fmt.Errorf("unable to decode and forward rsync operations: %w", err) } diff --git a/pkg/synchronization/endpoint/remote/server.go b/pkg/synchronization/endpoint/remote/server.go index a8f92a14b..4e353a2c0 100644 --- a/pkg/synchronization/endpoint/remote/server.go +++ b/pkg/synchronization/endpoint/remote/server.go @@ -7,6 +7,7 @@ import ( "fmt" "io" + "github.com/mutagen-io/mutagen/pkg/must" "google.golang.org/protobuf/proto" "github.com/mutagen-io/mutagen/pkg/encoding" @@ -31,6 +32,8 @@ type endpointServer struct { encoder *encoding.ProtobufEncoder // decoder is the control stream decoder. decoder *encoding.ProtobufDecoder + + logger *logging.Logger } // ServeEndpoint creates and serves a endpoint server on the specified stream. @@ -41,7 +44,7 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { // Perform the compression handshake. compressionAlgorithm, err := compression.ServerHandshake(stream) if err != nil { - stream.Close() + must.Close(stream, logger) return fmt.Errorf("compression handshake failed: %w", err) } @@ -70,7 +73,7 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { stream, decompressor, ) - defer closer.Close() + defer must.Close(closer, logger) // Create an encoder and a decoder for Protocol Buffers messages. encoder := encoding.NewProtobufEncoder(outbound) @@ -81,24 +84,24 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { request := &InitializeSynchronizationRequest{} if err := decoder.Decode(request); err != nil { err = fmt.Errorf("unable to receive initialize request: %w", err) - encoder.Encode(&InitializeSynchronizationResponse{Error: err.Error()}) - flusher.Flush() + must.ProtoEncode(encoder, &InitializeSynchronizationResponse{Error: err.Error()}, logger) + must.Flush(flusher, logger) return err } // Ensure that the initialization request is valid. if err := request.ensureValid(); err != nil { err = fmt.Errorf("invalid initialize request: %w", err) - encoder.Encode(&InitializeSynchronizationResponse{Error: err.Error()}) - flusher.Flush() + must.ProtoEncode(encoder, &InitializeSynchronizationResponse{Error: err.Error()}, logger) + must.Flush(flusher, logger) return err } // Expand and normalize the root path. if r, err := filesystem.Normalize(request.Root); err != nil { err = fmt.Errorf("unable to normalize synchronization root: %w", err) - encoder.Encode(&InitializeSynchronizationResponse{Error: err.Error()}) - flusher.Flush() + must.ProtoEncode(encoder, &InitializeSynchronizationResponse{Error: err.Error()}, logger) + must.Flush(flusher, logger) return err } else { request.Root = r @@ -116,11 +119,11 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { ) if err != nil { err = fmt.Errorf("unable to create underlying endpoint: %w", err) - encoder.Encode(&InitializeSynchronizationResponse{Error: err.Error()}) - flusher.Flush() + must.ProtoEncode(encoder, &InitializeSynchronizationResponse{Error: err.Error()}, logger) + must.Flush(flusher, logger) return err } - defer endpoint.Shutdown() + defer must.Shutdown(endpoint, logger) // Send a successful initialize response. if err = encoder.Encode(&InitializeSynchronizationResponse{}); err != nil { @@ -135,6 +138,7 @@ func ServeEndpoint(logger *logging.Logger, stream io.ReadWriteCloser) error { flusher: flusher, encoder: encoder, decoder: decoder, + logger: logger, } // Server until an error occurs. @@ -369,7 +373,9 @@ func (s *endpointServer) serveStage(request *StageRequest) error { // Begin staging. paths, signatures, receiver, err := s.endpoint.Stage(request.Paths, request.Digests) if err != nil { - s.encodeAndFlush(&StageResponse{Error: err.Error()}) + // TODO: Log the error if it occurs, but need + // a logger and no easy way to get one. + _ = s.encodeAndFlush(&StageResponse{Error: err.Error()}) return fmt.Errorf("unable to begin staging: %w", err) } @@ -399,7 +405,7 @@ func (s *endpointServer) serveStage(request *StageRequest) error { // we need to decode and forward them to the receiver. If this operation // completes successfully, staging is complete and successful. decoder := &protobufRsyncDecoder{decoder: s.decoder} - if err = rsync.DecodeToReceiver(decoder, uint64(len(paths)), receiver); err != nil { + if err = rsync.DecodeToReceiver(decoder, uint64(len(paths)), receiver, s.logger); err != nil { return fmt.Errorf("unable to decode and forward rsync operations: %w", err) } @@ -416,7 +422,7 @@ func (s *endpointServer) serveSupply(request *SupplyRequest) error { // Create an encoding receiver to transmit rsync operations to the remote. encoder := &protobufRsyncEncoder{encoder: s.encoder, flusher: s.flusher} - receiver := rsync.NewEncodingReceiver(encoder) + receiver := rsync.NewEncodingReceiver(encoder, s.logger) // Perform supplying. if err := s.endpoint.Supply(request.Paths, request.Signatures, receiver); err != nil { diff --git a/pkg/synchronization/protocols/docker/protocol.go b/pkg/synchronization/protocols/docker/protocol.go index fa2e4ae6b..412a8ba58 100644 --- a/pkg/synchronization/protocols/docker/protocol.go +++ b/pkg/synchronization/protocols/docker/protocol.go @@ -8,6 +8,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/agent" "github.com/mutagen-io/mutagen/pkg/agent/transport/docker" "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/synchronization" "github.com/mutagen-io/mutagen/pkg/synchronization/endpoint/remote" urlpkg "github.com/mutagen-io/mutagen/pkg/url" @@ -64,7 +65,7 @@ func (h *protocolHandler) Connect( case results <- dialResult{stream, err}: case <-ctx.Done(): if stream != nil { - stream.Close() + must.Close(stream, logger) } } }() diff --git a/pkg/synchronization/protocols/ssh/protocol.go b/pkg/synchronization/protocols/ssh/protocol.go index ad4229281..0fac97a6d 100644 --- a/pkg/synchronization/protocols/ssh/protocol.go +++ b/pkg/synchronization/protocols/ssh/protocol.go @@ -8,6 +8,7 @@ import ( "github.com/mutagen-io/mutagen/pkg/agent" "github.com/mutagen-io/mutagen/pkg/agent/transport/ssh" "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/synchronization" "github.com/mutagen-io/mutagen/pkg/synchronization/endpoint/remote" urlpkg "github.com/mutagen-io/mutagen/pkg/url" @@ -64,7 +65,7 @@ func (h *protocolHandler) Connect( case results <- dialResult{stream, err}: case <-ctx.Done(): if stream != nil { - stream.Close() + must.Close(stream, logger) } } }() diff --git a/pkg/synchronization/rsync/receive.go b/pkg/synchronization/rsync/receive.go index 5dd5da0e5..c86b0a406 100644 --- a/pkg/synchronization/rsync/receive.go +++ b/pkg/synchronization/rsync/receive.go @@ -8,6 +8,8 @@ import ( "io" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // EnsureValid ensures that ReceiverState's invariants are respected. @@ -97,13 +99,15 @@ type receiver struct { // target is the destination for the current file. It should be non-nil if // and only if base is non-nil. It should be nil if burning. target io.WriteCloser + + logger *logging.Logger } // NewReceiver creates a new receiver that stores files on disk. It is the // responsibility of the caller to ensure that the provided signatures are valid // by invoking their EnsureValid method. In order for the receiver to perform // efficiently, paths should be passed in depth-first traversal order. -func NewReceiver(root string, paths []string, signatures []*Signature, sinker Sinker) (Receiver, error) { +func NewReceiver(root string, paths []string, signatures []*Signature, sinker Sinker, logger *logging.Logger) (Receiver, error) { // Ensure that the receiving request is sane. if len(paths) != len(signatures) { return nil, errors.New("number of paths does not match number of signatures") @@ -114,10 +118,11 @@ func NewReceiver(root string, paths []string, signatures []*Signature, sinker Si root: root, paths: paths, signatures: signatures, - opener: filesystem.NewOpener(root), + opener: filesystem.NewOpener(root, logger), sinker: sinker, engine: NewEngine(), total: uint64(len(paths)), + logger: logger, }, nil } @@ -151,13 +156,13 @@ func (r *receiver) Receive(transmission *Transmission) error { // Since we're already at the end of the stream for this file, there's // no need to start burning operations if this fails. if r.base != nil { - r.base.Close() + must.Close(r.base, r.logger) r.base = nil - r.target.Close() + must.Close(r.target, r.logger) r.target = nil } else if !r.burning { if target, _ := r.sinker.Sink(r.paths[r.received]); target != nil { - target.Close() + must.Close(target, r.logger) } } @@ -202,7 +207,7 @@ func (r *receiver) Receive(transmission *Transmission) error { // Create a sink. If that fails, then we need to close out the base and // burn this file stream, but it's not a terminal error. if target, err := r.sinker.Sink(path); err != nil { - r.base.Close() + must.Close(r.base, r.logger) r.base = nil r.burning = true return nil @@ -214,9 +219,9 @@ func (r *receiver) Receive(transmission *Transmission) error { // Apply the operation. If that fails, then we need to close out the base, // target, and burn this file stream, but it's not a terminal error. if err := r.engine.Patch(r.target, r.base, signature, transmission.Operation); err != nil { - r.base.Close() + must.Close(r.base, r.logger) r.base = nil - r.target.Close() + must.Close(r.target, r.logger) r.target = nil r.burning = true return nil @@ -236,14 +241,14 @@ func (r *receiver) finalize() error { // Close any open internal resources. if r.base != nil { - r.base.Close() + must.Close(r.base, r.logger) r.base = nil - r.target.Close() + must.Close(r.target, r.logger) r.target = nil } // Close the file opener. - r.opener.Close() + must.Close(r.opener, r.logger) // Mark the receiver as finalized. r.finalized = true @@ -273,12 +278,13 @@ type monitoringReceiver struct { // expected to coincide with the start of a new file. startOfFile bool // state is the current receiver state. - state *ReceiverState + state *ReceiverState + logger *logging.Logger } // NewMonitoringReceiver wraps a receiver and provides monitoring information // via a callback. -func NewMonitoringReceiver(receiver Receiver, paths []string, signatures []*Signature, monitor Monitor) Receiver { +func NewMonitoringReceiver(receiver Receiver, paths []string, signatures []*Signature, monitor Monitor, logger *logging.Logger) Receiver { // Verify that the path and signature counts match. if len(paths) != len(signatures) { panic("path count does not match signature count") @@ -294,6 +300,7 @@ func NewMonitoringReceiver(receiver Receiver, paths []string, signatures []*Sign state: &ReceiverState{ ExpectedFiles: uint64(len(paths)), }, + logger: logger, } } @@ -365,7 +372,7 @@ func (r *monitoringReceiver) Receive(transmission *Transmission) error { func (r *monitoringReceiver) finalize() error { // Perform a final status update. We don't bother checking for an error // because it's inconsequential at this point. - r.monitor(nil) + must.Succeed(r.monitor(nil), "monitor callback", r.logger) // Invoke finalize on the underlying receiver. return r.receiver.finalize() @@ -430,14 +437,17 @@ type encodingReceiver struct { encoder Encoder // finalized indicates whether or not the receiver has been finalized. finalized bool + + logger *logging.Logger } // NewEncodingReceiver creates a new receiver that handles messages by encoding // them with the specified Encoder. It is designed to be used with // DecodeToReceiver. -func NewEncodingReceiver(encoder Encoder) Receiver { +func NewEncodingReceiver(encoder Encoder, logger *logging.Logger) Receiver { return &encodingReceiver{ encoder: encoder, + logger: logger, } } @@ -475,7 +485,7 @@ func (r *encodingReceiver) finalize() error { // Decoder is the interface used by DecodeToReceiver to receive transmissions, // usually across a network. type Decoder interface { - // Decoder decodes a transmission encoded by an encoder. The transmission + // Decode decodes a transmission encoded by an encoder. The transmission // should be decoded into the specified Transmission object, which will be a // non-nil zero-valued Transmission object. The decoder is *not* responsible // for validating that the transmission is valid before returning it. @@ -493,7 +503,7 @@ type Decoder interface { // received so that it knows when forwarding is complete. It is designed to be // used with an encoding receiver, such as that returned by NewEncodingReceiver. // It finalizes the provided receiver before returning. -func DecodeToReceiver(decoder Decoder, count uint64, receiver Receiver) error { +func DecodeToReceiver(decoder Decoder, count uint64, receiver Receiver, logger *logging.Logger) error { // Allocate the transmission object that we'll use to receive into. transmission := &Transmission{} @@ -504,22 +514,22 @@ func DecodeToReceiver(decoder Decoder, count uint64, receiver Receiver) error { // Receive the next message. transmission.resetToZeroMaintainingCapacity() if err := decoder.Decode(transmission); err != nil { - decoder.Finalize() - receiver.finalize() + must.Finalize(decoder, logger) + must.Succeed(receiver.finalize(), "finalize receiver", logger) return fmt.Errorf("unable to decode transmission: %w", err) } // Validate the transmission. if err := transmission.EnsureValid(); err != nil { - decoder.Finalize() - receiver.finalize() + must.Finalize(decoder, logger) + must.Succeed(receiver.finalize(), "finalize receiver", logger) return fmt.Errorf("invalid transmission received: %w", err) } // Forward the message. if err := receiver.Receive(transmission); err != nil { - decoder.Finalize() - receiver.finalize() + must.Finalize(decoder, logger) + must.Succeed(receiver.finalize(), "finalize receiver", logger) return fmt.Errorf("unable to forward message to receiver: %w", err) } @@ -536,7 +546,7 @@ func DecodeToReceiver(decoder Decoder, count uint64, receiver Receiver) error { // Ensure that the decoder is finalized. if err := decoder.Finalize(); err != nil { - receiver.finalize() + must.Succeed(receiver.finalize(), "finalize receiver", logger) return fmt.Errorf("unable to finalize decoder: %w", err) } diff --git a/pkg/synchronization/rsync/transmit.go b/pkg/synchronization/rsync/transmit.go index 1ea724787..ae3f5d566 100644 --- a/pkg/synchronization/rsync/transmit.go +++ b/pkg/synchronization/rsync/transmit.go @@ -5,6 +5,8 @@ import ( "fmt" "github.com/mutagen-io/mutagen/pkg/filesystem" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) // Transmit performs streaming transmission of files (in rsync deltified form) @@ -12,17 +14,17 @@ import ( // that the provided signatures are valid by invoking their EnsureValid method. // In order for this function to perform efficiently, paths should be passed in // depth-first traversal order. -func Transmit(root string, paths []string, signatures []*Signature, receiver Receiver) error { +func Transmit(root string, paths []string, signatures []*Signature, receiver Receiver, logger *logging.Logger) error { // Ensure that the transmission request is sane. if len(paths) != len(signatures) { - receiver.finalize() + must.Succeed(receiver.finalize(), "finalize receiver", logger) return errors.New("number of paths does not match number of signatures") } // Create a file opener that we can use to safely open files, and defer its // closure. - opener := filesystem.NewOpener(root) - defer opener.Close() + opener := filesystem.NewOpener(root, logger) + defer must.Close(opener, logger) // Create an rsync engine. engine := NewEngine() @@ -42,7 +44,7 @@ func Transmit(root string, paths []string, signatures []*Signature, receiver Rec Error: fmt.Errorf("unable to open file: %w", err).Error(), } if err = receiver.Receive(transmission); err != nil { - receiver.finalize() + must.Succeed(receiver.finalize(), "finalize receiver", logger) return fmt.Errorf("unable to send error transmission: %w", err) } continue @@ -65,11 +67,11 @@ func Transmit(root string, paths []string, signatures []*Signature, receiver Rec err = engine.Deltify(file, signatures[i], 0, transmit) // Close the file. - file.Close() + must.Close(file, logger) // Handle any transmission errors. These are terminal. if transmitError != nil { - receiver.finalize() + must.Succeed(receiver.finalize(), "finalize receiver", logger) return fmt.Errorf("unable to transmit delta: %w", transmitError) } @@ -81,7 +83,7 @@ func Transmit(root string, paths []string, signatures []*Signature, receiver Rec transmission.Error = fmt.Errorf("engine error: %w", err).Error() } if err = receiver.Receive(transmission); err != nil { - receiver.finalize() + must.Succeed(receiver.finalize(), "finalize receiver", logger) return fmt.Errorf("unable to send done message: %w", err) } } diff --git a/scripts/build.go b/scripts/build.go index 13fec6161..4a6a2d63d 100644 --- a/scripts/build.go +++ b/scripts/build.go @@ -20,6 +20,8 @@ import ( "github.com/mutagen-io/mutagen/cmd" "github.com/mutagen-io/mutagen/pkg/agent" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/mutagen" ) @@ -386,9 +388,10 @@ type ArchiveBuilder struct { compressor *gzip.Writer archiver *tar.Writer copyBuffer []byte + logger *logging.Logger } -func NewArchiveBuilder(bundlePath string) (*ArchiveBuilder, error) { +func NewArchiveBuilder(bundlePath string, logger *logging.Logger) (*ArchiveBuilder, error) { // Open the underlying file. file, err := os.Create(bundlePath) if err != nil { @@ -398,7 +401,7 @@ func NewArchiveBuilder(bundlePath string) (*ArchiveBuilder, error) { // Create the compressor. compressor, err := gzip.NewWriterLevel(file, gzip.BestCompression) if err != nil { - file.Close() + must.Close(file, logger) return nil, fmt.Errorf("unable to create compressor: %w", err) } @@ -408,17 +411,18 @@ func NewArchiveBuilder(bundlePath string) (*ArchiveBuilder, error) { compressor: compressor, archiver: tar.NewWriter(compressor), copyBuffer: make([]byte, archiveBuilderCopyBufferSize), + logger: logger, }, nil } func (b *ArchiveBuilder) Close() error { // Close in the necessary order to trigger flushes. if err := b.archiver.Close(); err != nil { - b.compressor.Close() - b.file.Close() + must.Close(b.compressor, b.logger) + must.Close(b.file, b.logger) return fmt.Errorf("unable to close archiver: %w", err) } else if err := b.compressor.Close(); err != nil { - b.file.Close() + must.Close(b.file, b.logger) return fmt.Errorf("unable to close compressor: %w", err) } else if err := b.file.Close(); err != nil { return fmt.Errorf("unable to close file: %w", err) @@ -439,7 +443,7 @@ func (b *ArchiveBuilder) Add(name, path string, mode int64) error { if err != nil { return fmt.Errorf("unable to open file: %w", err) } - defer file.Close() + defer must.Close(file, b.logger) // Compute its size. stat, err := file.Stat() @@ -470,13 +474,13 @@ func (b *ArchiveBuilder) Add(name, path string, mode int64) error { // copyFile copies the contents at sourcePath to a newly created file at // destinationPath that inherits the permissions of sourcePath. -func copyFile(sourcePath, destinationPath string) error { +func copyFile(sourcePath, destinationPath string, logger *logging.Logger) error { // Open the source file and defer its closure. source, err := os.Open(sourcePath) if err != nil { return fmt.Errorf("unable to open source file: %w", err) } - defer source.Close() + defer must.Close(source, logger) // Grab source file metadata. metadata, err := source.Stat() @@ -485,7 +489,7 @@ func copyFile(sourcePath, destinationPath string) error { } // Remove the destination. - os.Remove(destinationPath) + must.OSRemove(destinationPath, logger) // Create the destination file and defer its closure. We open with exclusive // creation flags to ensure that we're the ones creating the file so that @@ -494,7 +498,7 @@ func copyFile(sourcePath, destinationPath string) error { if err != nil { return fmt.Errorf("unable to create destination file: %w", err) } - defer destination.Close() + defer must.Close(destination, logger) // Copy contents. if count, err := io.Copy(destination, source); err != nil { @@ -530,6 +534,8 @@ this script is operated in a non-interactive mode. // build is the primary entry point. func build() error { + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // Parse command line arguments. flagSet := pflag.NewFlagSet("build", pflag.ContinueOnError) flagSet.SetOutput(io.Discard) @@ -540,7 +546,7 @@ func build() error { flagSet.BoolVar(&enableSSPLEnhancements, "sspl", false, "enable SSPL-licensed enhancements") if err := flagSet.Parse(os.Args[1:]); err != nil { if err == pflag.ErrHelp { - fmt.Fprint(os.Stdout, usage) + must.Fprint(os.Stdout, logger, usage) return nil } else { return fmt.Errorf("unable to parse command line: %w", err) @@ -673,14 +679,14 @@ func build() error { // Build the agent bundle. log.Println("Building agent bundle...") agentBundlePath := filepath.Join(buildPath, agent.BundleName) - agentBundleBuilder, err := NewArchiveBuilder(agentBundlePath) + agentBundleBuilder, err := NewArchiveBuilder(agentBundlePath, logger) if err != nil { return fmt.Errorf("unable to create agent bundle archive builder: %w", err) } for _, target := range agentTargets { agentBuildPath := filepath.Join(agentBuildSubdirectoryPath, target.Name()) if err := agentBundleBuilder.Add(target.Name(), agentBuildPath, 0755); err != nil { - agentBundleBuilder.Close() + must.Close(agentBundleBuilder, logger) return fmt.Errorf("unable to add agent to bundle: %w", err) } } @@ -703,13 +709,13 @@ func build() error { ) // Build the release bundle. - if releaseBundle, err := NewArchiveBuilder(releaseBundlePath); err != nil { + if releaseBundle, err := NewArchiveBuilder(releaseBundlePath, logger); err != nil { return fmt.Errorf("unable to create release bundle: %w", err) } else if err = releaseBundle.Add(target.ExecutableName(cliBaseName), cliBuildPath, 0755); err != nil { - releaseBundle.Close() + must.Close(releaseBundle, logger) return fmt.Errorf("unable to add CLI to release bundle: %w", err) } else if err = releaseBundle.Add("", agentBundlePath, 0644); err != nil { - releaseBundle.Close() + must.Close(releaseBundle, logger) return fmt.Errorf("unable to add agent bundle to release bundle: %w", err) } else if err = releaseBundle.Close(); err != nil { return fmt.Errorf("unable to finalize release bundle: %w", err) @@ -721,7 +727,7 @@ func build() error { log.Println("Copying binary for testing") localCLIBuildPath := filepath.Join(cliBuildSubdirectoryPath, localTarget.Name()) localCLIRelocationPath := filepath.Join(buildPath, localTarget.ExecutableName(cliBaseName)) - if err := copyFile(localCLIBuildPath, localCLIRelocationPath); err != nil { + if err := copyFile(localCLIBuildPath, localCLIRelocationPath, logger); err != nil { return fmt.Errorf("unable to copy current platform CLI: %w", err) } diff --git a/tools/scan_bench/main.go b/tools/scan_bench/main.go index 5e90fda97..681a557e5 100644 --- a/tools/scan_bench/main.go +++ b/tools/scan_bench/main.go @@ -19,6 +19,8 @@ import ( "github.com/mutagen-io/mutagen/cmd/profile" "github.com/mutagen-io/mutagen/pkg/filesystem/behavior" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" "github.com/mutagen-io/mutagen/pkg/synchronization/core" "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore" dockerignore "github.com/mutagen-io/mutagen/pkg/synchronization/core/ignore/docker" @@ -59,6 +61,7 @@ func ignoreCachesIntersectionEqual(first, second ignore.IgnoreCache) bool { } func main() { + logger := logging.NewLogger(logging.LevelError, os.Stderr) // Parse command line arguments. flagSet := pflag.NewFlagSet("scan_bench", pflag.ContinueOnError) flagSet.SetOutput(io.Discard) @@ -72,7 +75,7 @@ func main() { flagSet.StringSliceVarP(&ignores, "ignore", "i", nil, "specify ignore paths") if err := flagSet.Parse(os.Args[1:]); err != nil { if err == pflag.ErrHelp { - fmt.Fprint(os.Stdout, usage) + must.Fprint(os.Stdout, logger, usage) return } else { cmd.Fatal(fmt.Errorf("unable to parse command line: %w", err)) @@ -157,6 +160,7 @@ func main() { behavior.ProbeMode_ProbeModeProbe, core.SymbolicLinkMode_SymbolicLinkModePortable, core.PermissionsMode_PermissionsModePortable, + logger, ) if err != nil { cmd.Fatal(fmt.Errorf("unable to perform cold scan: %w", err)) @@ -192,6 +196,7 @@ func main() { behavior.ProbeMode_ProbeModeProbe, core.SymbolicLinkMode_SymbolicLinkModePortable, core.PermissionsMode_PermissionsModePortable, + logger, ) if err != nil { cmd.Fatal(fmt.Errorf("unable to perform warm scan: %w", err)) @@ -235,6 +240,7 @@ func main() { behavior.ProbeMode_ProbeModeProbe, core.SymbolicLinkMode_SymbolicLinkModePortable, core.PermissionsMode_PermissionsModePortable, + logger, ) if err != nil { cmd.Fatal(fmt.Errorf("unable to perform second warm scan: %w", err)) @@ -276,6 +282,7 @@ func main() { behavior.ProbeMode_ProbeModeProbe, core.SymbolicLinkMode_SymbolicLinkModePortable, core.PermissionsMode_PermissionsModePortable, + logger, ) if err != nil { cmd.Fatal(fmt.Errorf("unable to perform accelerated scan (with re-check paths): %w", err)) @@ -315,6 +322,7 @@ func main() { behavior.ProbeMode_ProbeModeProbe, core.SymbolicLinkMode_SymbolicLinkModePortable, core.PermissionsMode_PermissionsModePortable, + logger, ) if err != nil { cmd.Fatal(fmt.Errorf("unable to perform accelerated scan (without re-check paths): %w", err)) diff --git a/tools/watch_demo/watch_demo.go b/tools/watch_demo/watch_demo.go index ea3a29a30..f17b5ce69 100644 --- a/tools/watch_demo/watch_demo.go +++ b/tools/watch_demo/watch_demo.go @@ -9,9 +9,13 @@ import ( "github.com/mutagen-io/mutagen/cmd" "github.com/mutagen-io/mutagen/pkg/filesystem/watching" + "github.com/mutagen-io/mutagen/pkg/logging" + "github.com/mutagen-io/mutagen/pkg/must" ) func main() { + logger := logging.NewLogger(logging.LevelError, os.Stderr) + // Parse arguments. if len(os.Args) != 2 { cmd.Fatal(errors.New("invalid number of arguments")) @@ -57,7 +61,7 @@ func main() { cmd.Fatal(fmt.Errorf("watching failed: %w", err)) case <-signalTermination: fmt.Println("Received termination signal, terminating watching...") - watcher.Terminate() + must.Terminate(watcher, logger) } } }