Signed-off-by: Moritz Poldrack <git@moritz.sh>
---
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
Signed-off-by: Moritz Poldrack <git@moritz.sh>
---
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
On Thu Dec 15, 2022 at 1:49 PM CST, Moritz Poldrack wrote:
> On Thu Dec 15, 2022 at 6:45 PM CET, Tim Culverhouse wrote:
> > On Wed Dec 14, 2022 at 1:47 PM CST, Moritz Poldrack wrote:
> > > 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>