Moritz Poldrack: 2 cmd: rename sync function to avoid collisions with stdlib package cmd: make syncing run in parallel 2 files changed, 86 insertions(+), 78 deletions(-)
offmap/patches/.build.yml: SUCCESS in 1m54s [cmd: rename sync function to avoid collisions with stdlib package][0] from [Moritz Poldrack][1] [0]: https://lists.sr.ht/~rockorager/offmap/patches/37529 [1]: mailto:git@moritz.sh ✓ #905181 SUCCESS offmap/patches/.build.yml https://builds.sr.ht/~rockorager/job/905181
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~rockorager/offmap/patches/37529/mbox | git am -3Learn more about email & git
Signed-off-by: Moritz Poldrack <git@moritz.sh> ---
How about "synchronize"?
cmd/sync.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index 328e841..353d94f 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -15,13 +15,13 @@ func newSyncCommand() *cobra.Command { cmd := &cobra.Command{ Use: "sync", Short: "sync synchronizes mail stores", - Run: sync, + Run: syncRun, } cmd.Flags().StringSliceP("account", "a", nil, "only sync the specified account(s)") return cmd } -func sync(cmd *cobra.Command, args []string) { +func syncRun(cmd *cobra.Command, args []string) { verbosity, _ := cmd.Flags().GetCount("verbose") log.SetLevel(verbosity) accts, err := cmd.Flags().GetStringSlice("account") -- 2.39.0
Applied. To ssh://git@git.sr.ht/~rockorager/offmap faea313..f702f57 main -> mai
Signed-off-by: Moritz Poldrack <git@moritz.sh> ---
I like it! The one comment I have is we should change all of the log.Fatalf's in the account goroutines into log.Errorf's + return. That way one account sync can fail and not pull down the entire program. What do you think?I think the os.Exit(1) in log.Fatal should be removed completely. Right now a log.Fatal is basically worse than a panic since defers are not run. So I think it should be part of another patchseries to make cancellations and abortions (is that the correct word? sounds strange in this context) less of a nuclear options and have them clean up behind them (move already downloaded messages to cur, logout for all connections, potentially other cleanup tasks…)I agree - I actually (had) one that changed the calls to sync and diff to the RunE method of cobra, and returned errors back up to the main loop, but that becomes a moot point with this series to just return and exit the goroutine. We can do that all as a separate patchset. I'm fine with keeping syncRun in (1/2) too. Reviewed-by: Tim Culverhouse <tim@timculverhouse.com>-- Tim
cmd/sync.go | 160 +++++++++++++++++++++++++++------------------------- 1 file changed, 84 insertions(+), 76 deletions(-) diff --git a/cmd/sync.go b/cmd/sync.go index 353d94f..fbaa48c 100644 --- a/cmd/sync.go +++ b/cmd/sync.go @@ -1,6 +1,7 @@ package main import ( + "sync" "sync/atomic" "time" @@ -33,96 +34,103 @@ func syncRun(cmd *cobra.Command, args []string) { if err != nil { log.Debugf("%v", err) } + var wg sync.WaitGroup for _, cfg := range cfgs { - log.Infof("offmap: syncing account %s", cfg.Name) - start := time.Now() + wg.Add(1) + go func(cfg *offmap.Config) { + defer wg.Done() + log.Infof("offmap: syncing account %s", cfg.Name) + start := time.Now() - // Bootstrap the app - state, err := offmap.NewState(cfg) - if err != nil { - log.Fatalf("%v", err) - } - err = state.Load() - if err != nil { - log.Fatalf("%v", err) - } - local, err := maildir.NewStore(cfg) - if err != nil { - log.Fatalf("could not create store: %v", err) - } - remote, err := imap.NewStore(cfg) - if err != nil { - log.Fatalf("could not create store: %v", err) - } - - // Get the diffs - localDiff, err := local.Diff(state.Mailboxes) - if err != nil { - log.Fatalf("%v", err) - } - remoteDiff, err := remote.Diff(state.Mailboxes) - if err != nil { - log.Fatalf("%v", err) - } - - var quit atomic.Bool - go func() { - // Save our state if we cancel the command - <-cmd.Context().Done() - quit.Store(true) - }() - - // Do mailboxes first - err = remote.ApplyMailboxDiff(state, localDiff) - if err != nil { - // Save any changes we've made. log.Fatalf calls os.Exit(1). - // Defers are bypassed with this call - if err := state.Save(); err != nil { - log.Errorf("offmap: could not save state: %v", err) + // Bootstrap the app + state, err := offmap.NewState(cfg) + if err != nil { + log.Fatalf("%v", err) } - log.Fatalf("imap: could not apply diff: %v", err) - } - - // Apply remote changes to local - err = local.ApplyMailboxDiff(state, remoteDiff) - if err != nil { - // Save any changes we've made. log.Fatalf calls os.Exit(1). - // Defers are bypassed with this call - if err := state.Save(); err != nil { - log.Errorf("offmap: could not save state: %v", err) + err = state.Load() + if err != nil { + log.Fatalf("%v", err) + } + local, err := maildir.NewStore(cfg) + if err != nil { + log.Fatalf("could not create store: %v", err) + } + remote, err := imap.NewStore(cfg) + if err != nil { + log.Fatalf("could not create store: %v", err) } - log.Fatalf("%v", err) - } - // Now the state has an up to date picture of mailboxes. Sync - // each mailbox, if it has changes - for _, mbox := range state.Mailboxes { - if quit.Load() { - break + // Get the diffs + localDiff, err := local.Diff(state.Mailboxes) + if err != nil { + log.Fatalf("%v", err) + } + remoteDiff, err := remote.Diff(state.Mailboxes) + if err != nil { + log.Fatalf("%v", err) } - ok1, err := remote.ApplyEmailDiff(state, mbox.RemoteName, localDiff) + + var quit atomic.Bool + go func() { + // Save our state if we cancel the command + <-cmd.Context().Done() + quit.Store(true) + }() + + // Do mailboxes first + err = remote.ApplyMailboxDiff(state, localDiff) if err != nil { - log.Errorf("%v", err) - continue + // Save any changes we've made. log.Fatalf calls os.Exit(1). + // Defers are bypassed with this call + if err := state.Save(); err != nil { + log.Errorf("offmap: could not save state: %v", err) + } + log.Fatalf("imap: could not apply diff: %v", err) } - ok2, err := local.ApplyEmailDiff(state, mbox.LocalName, remoteDiff) + + // Apply remote changes to local + err = local.ApplyMailboxDiff(state, remoteDiff) if err != nil { - log.Errorf("%v", err) - continue + // Save any changes we've made. log.Fatalf calls os.Exit(1). + // Defers are bypassed with this call + if err := state.Save(); err != nil { + log.Errorf("offmap: could not save state: %v", err) + } + log.Fatalf("%v", err) } - if ok1 || ok2 { - err = remote.UpdateMailboxStatus(state, mbox.LocalName) + + // Now the state has an up to date picture of mailboxes. Sync + // each mailbox, if it has changes + for _, mbox := range state.Mailboxes { + if quit.Load() { + break + } + ok1, err := remote.ApplyEmailDiff(state, mbox.RemoteName, localDiff) if err != nil { log.Errorf("%v", err) + continue + } + ok2, err := local.ApplyEmailDiff(state, mbox.LocalName, remoteDiff) + if err != nil { + log.Errorf("%v", err) + continue + } + if ok1 || ok2 { + err = remote.UpdateMailboxStatus(state, mbox.LocalName) + if err != nil { + log.Errorf("%v", err) + } } } - } - // Save and clean up - if err := state.Save(); err != nil { - log.Errorf("offmap: could not save state: %v", err) - } - remote.Cleanup() - log.Infof("offmap (%s): sync completed in %s", cfg.Name, time.Since(start).Round(1*time.Millisecond)) + // Save and clean up + if err := state.Save(); err != nil { + log.Errorf("offmap: could not save state: %v", err) + } + remote.Cleanup() + log.Infof("offmap (%s): sync completed in %s", cfg.Name, time.Since(start).Round(1*time.Millisecond)) + }(cfg) } + + wg.Wait() } -- 2.39.0
builds.sr.ht <builds@sr.ht>offmap/patches/.build.yml: SUCCESS in 1m54s [cmd: rename sync function to avoid collisions with stdlib package][0] from [Moritz Poldrack][1] [0]: https://lists.sr.ht/~rockorager/offmap/patches/37529 [1]: mailto:git@moritz.sh ✓ #905181 SUCCESS offmap/patches/.build.yml https://builds.sr.ht/~rockorager/job/905181