Fix queue bugs.

RELEASE_NOTES=n/a

Signed-off-by: Dominik Schulz <[email protected]>

fish.completion

@@ -208,6 +208,7 @@ complete -c $PROG -f -n '__fish_gopass_uses_command insert' -a "(__fish_gopass_p
 complete -c $PROG -f -n '__fish_gopass_needs_command' -a link -d 'Command: Create a symlink'
 complete -c $PROG -f -n '__fish_gopass_needs_command' -a list -d 'Command: List existing secrets'
 complete -c $PROG -f -n '__fish_gopass_uses_command list' -a "(__fish_gopass_print_dir)"
+complete -c $PROG -f -n '__fish_gopass_needs_command' -a merge -d 'Command: Merge multiple secrets into one'
 complete -c $PROG -f -n '__fish_gopass_needs_command' -a mounts -d 'Command: Edit mounted stores'
 complete -c $PROG -f -n '__fish_gopass_uses_command mounts' -a add -d 'Subcommand: Mount a password store'
 complete -c $PROG -f -n '__fish_gopass_uses_command mounts add -l yes -d "Always answer yes to yes/no questions"'

internal/action/commands.go

@@ -693,6 +693,11 @@ func (s *Action) GetCommands() []*cli.Command {
 					Usage:   "Remove merged entries",
 					Value:   true,
 				},
+				&cli.BoolFlag{
+					Name:    "force",
+					Aliases: []string{"f"},
+					Usage:   "Skip editor, merge entries unattended",
+				},
 			},
 		},
 		{

internal/action/merge.go

@@ -3,15 +3,19 @@ package action
 import (
 	"bytes"
 	"fmt"
+	"time"
 
 	"github.com/gopasspw/gopass/internal/audit"
 	"github.com/gopasspw/gopass/internal/editor"
 	"github.com/gopasspw/gopass/internal/out"
+	"github.com/gopasspw/gopass/internal/queue"
 	"github.com/gopasspw/gopass/pkg/ctxutil"
+	"github.com/gopasspw/gopass/pkg/debug"
 	"github.com/gopasspw/gopass/pkg/gopass/secrets"
 	"github.com/urfave/cli/v2"
 )
 
+// Merge implements the merge subcommand that allows merging multiple entries.
 func (s *Action) Merge(c *cli.Context) error {
 	ctx := ctxutil.WithGlobalFlags(c)
 	to := c.Args().First()
@@ -36,32 +40,37 @@ func (s *Action) Merge(c *cli.Context) error {
 		}
 		sec, err := s.Store.Get(ctxutil.WithShowParsing(ctx, false), k)
 		if err != nil {
-			return ExitError(ExitDecrypt, err, "failed to decrypt: %s: %w", k, err)
+			return ExitError(ExitDecrypt, err, "failed to decrypt: %s: %s", k, err)
 		}
 		_, err = content.WriteString("\n# Secret: " + k + "\n")
 		if err != nil {
-			return ExitError(ExitUnknown, err, "failed to write: %w", err)
+			return ExitError(ExitUnknown, err, "failed to write: %s", err)
 		}
 		_, err = content.Write(sec.Bytes())
 		if err != nil {
-			return ExitError(ExitUnknown, err, "failed to write: %w", err)
+			return ExitError(ExitUnknown, err, "failed to write: %s", err)
 		}
 	}
 
-	// invoke the editor to let the user edit the content
-	newContent, err := editor.Invoke(ctx, ed, content.Bytes())
-	if err != nil {
-		return ExitError(ExitUnknown, err, "failed to invoke editor: %s", err)
-	}
-	// If content is equal, nothing changed, exiting
-	if bytes.Equal(content.Bytes(), newContent) {
-		return nil
+	newContent := content.Bytes()
+	if !c.Bool("force") {
+		var err error
+		// invoke the editor to let the user edit the content
+		newContent, err = editor.Invoke(ctx, ed, content.Bytes())
+		if err != nil {
+			return ExitError(ExitUnknown, err, "failed to invoke editor: %s", err)
+		}
+
+		// If content is equal, nothing changed, exiting
+		if bytes.Equal(content.Bytes(), newContent) {
+			return nil
+		}
 	}
 
 	nSec := secrets.ParsePlain(newContent)
 
 	// if the secret has a password, we check it's strength
-	if pw := nSec.Password(); pw != "" {
+	if pw := nSec.Password(); pw != "" && !c.Bool("force") {
 		audit.Single(ctx, pw)
 	}
 
@@ -74,9 +83,22 @@ func (s *Action) Merge(c *cli.Context) error {
 		return nil
 	}
 
+	// wait until the previous commit is done
+	// TODO: This wouldn't be necessary if we could handle merging and deleting
+	// in a single commit, but then we'd need to expose additional implementation
+	// details of the underlying VCS. Or create some kind of transaction on top
+	// of the Git wrapper.
+	if err := queue.GetQueue(ctx).Idle(time.Minute); err != nil {
+		return err
+	}
+
 	for _, old := range from {
+		if !s.Store.Exists(ctx, old) {
+			continue
+		}
+		debug.Log("deleting merged entry %s", old)
 		if err := s.Store.Delete(ctx, old); err != nil {
-			return ExitError(ExitUnknown, err, "failed to delete %s: %w", old, err)
+			return ExitError(ExitUnknown, err, "failed to delete %s: %s", old, err)
 		}
 	}
 	return nil

internal/queue/background.go

@@ -9,6 +9,8 @@ package queue
 
 import (
 	"context"
+	"fmt"
+	"time"
 
 	"github.com/gopasspw/gopass/pkg/debug"
 )
@@ -23,6 +25,7 @@ const (
 type Queuer interface {
 	Add(Task) Task
 	Wait(context.Context) error
+	Idle(time.Duration) error
 }
 
 // WithQueue adds the given queue to the context
@@ -51,6 +54,11 @@ func (n *noop) Wait(_ context.Context) error {
 	return nil
 }
 
+// Idle always returns nil
+func (n *noop) Idle(_ time.Duration) error {
+	return nil
+}
+
 // Task is a background task
 type Task func(ctx context.Context) error
 
@@ -88,7 +96,28 @@ func (q *Queue) Add(t Task) Task {
 	return func(_ context.Context) error { return nil }
 }
 
-// Wait waits for all tasks to be processed
+// Idle returns nil the next time the queue is empty
+func (q *Queue) Idle(maxWait time.Duration) error {
+	done := make(chan struct{})
+	go func() {
+		for {
+			if len(q.work) < 1 {
+				done <- struct{}{}
+			}
+			time.Sleep(20 * time.Millisecond)
+		}
+	}()
+	select {
+	case <-done:
+		return nil
+	case <-time.After(maxWait):
+		return fmt.Errorf("timed out waiting for empty queue")
+	}
+}
+
+// Wait waits for all tasks to be processed. Must only be called once on
+// shutdown.
+// TODO: This should be called Close
 func (q *Queue) Wait(ctx context.Context) error {
 	close(q.work)
 	select {

main_test.go

@@ -76,6 +76,7 @@ var commandsWithError = map[string]struct{}{
 	".init":              {},
 	".insert":            {},
 	".link":              {},
+	".merge":             {},
 	".mounts.add":        {},
 	".mounts.remove":     {},
 	".move":              {},

zsh.completion

@@ -173,6 +173,12 @@ WARNING: This will update the secret content to the latest format. This might be
 	      _describe -t commands "gopass list" subcommands
 	      _gopass_complete_folders
 
+	      ;;
+	  merge)
+	      _arguments : "--delete[Remove merged entries]" "--force[Skip editor, merge entries unattended]"
+	      _describe -t commands "gopass merge" subcommands
+
+
 	      ;;
 	  mounts)
 	      local -a subcommands
@@ -198,7 +204,7 @@ WARNING: This will update the secret content to the latest format. This might be
 
 	      ;;
 	  pwgen)
-	      _arguments : "--no-numerals[Do not include numerals in the generated passwords.]" "--no-capitalize[Do not include capital letter in the generated passwords.]" "--ambiguous[Do not include characters that could be easily confused with each other, like '1' and 'l' or '0' and 'O']" "--one-per-line[Print one password per line]" "--xkcd[Use multiple random english words combined to a password. By default, space is used as separator and all words are lowercase]" "--sep[Word separator for generated xkcd style password. If no separator is specified, the words are combined without spaces/separator and the first character of words is capitalised. This flag implies -xkcd]" "--lang[Language to generate password from, currently de (german) and en (english, default) are supported]"
+	      _arguments : "--no-numerals[Do not include numerals in the generated passwords.]" "--no-capitalize[Do not include capital letter in the generated passwords.]" "--ambiguous[Do not include characters that could be easily confused with each other, like '1' and 'l' or '0' and 'O']" "--symbols[Include at least one symbol in the password.]" "--one-per-line[Print one password per line]" "--xkcd[Use multiple random english words combined to a password. By default, space is used as separator and all words are lowercase]" "--sep[Word separator for generated xkcd style password. If no separator is specified, the words are combined without spaces/separator and the first character of words is capitalised. This flag implies -xkcd]" "--lang[Language to generate password from, currently de (german) and en (english, default) are supported]"
 	      _describe -t commands "gopass pwgen" subcommands
 
 
@@ -305,6 +311,7 @@ WARNING: This will update the secret content to the latest format. This might be
 	  "insert:Insert a new secret"
 	  "link:Create a symlink"
 	  "list:List existing secrets"
+	  "merge:Merge multiple secrets into one"
 	  "mounts:Edit mounted stores"
 	  "move:Move secrets from one location to another"
 	  "otp:Generate time- or hmac-based tokens"