Merge pull request #3742 from MichaelEischer/from-repo

copy: replace --repo2 with --from-repo

changelog/unreleased/pull-3742

@@ -0,0 +1,19 @@
+Change: Replace `--repo2` option used by init/copy with `--from-repo`
+
+The `init` and the `copy` command can read data from another repository.
+However, confusingly `--repo2` referred to the repository from which the
+`init` command copies parameters, but for the `copy` command `--repo2`
+referred to the copy destination.
+
+We have introduced a new option `--from-repo` which always refers to the
+source repository for both commands. The old parameters names have been
+deprecated but still work. To create a new repository and copy all snapshots
+to it, the commands are now as follows:
+
+```
+restic -r /srv/restic-repo-copy init --from-repo /srv/restic-repo --copy-chunker-params
+restic -r /srv/restic-repo-copy copy --from-repo /srv/restic-repo
+```
+
+https://github.com/restic/restic/pull/3742
+https://forum.restic.net/t/restic-repository2-confusion/5017

cmd/restic/cmd_copy.go

@@ -50,17 +50,21 @@ func init() {
 	cmdRoot.AddCommand(cmdCopy)
 
 	f := cmdCopy.Flags()
-	initSecondaryRepoOptions(f, &copyOptions.secondaryRepoOptions, "destination", "to copy snapshots to")
+	initSecondaryRepoOptions(f, &copyOptions.secondaryRepoOptions, "destination", "to copy snapshots from")
 	f.StringArrayVarP(&copyOptions.Hosts, "host", "H", nil, "only consider snapshots for this `host`, when no snapshot ID is given (can be specified multiple times)")
 	f.Var(&copyOptions.Tags, "tag", "only consider snapshots which include this `taglist`, when no snapshot ID is given")
 	f.StringArrayVar(&copyOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot ID is given")
 }
 
 func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error {
-	dstGopts, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "destination")
+	secondaryGopts, isFromRepo, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "destination")
 	if err != nil {
 		return err
 	}
+	if isFromRepo {
+		// swap global options, if the secondary repo was set via from-repo
+		gopts, secondaryGopts = secondaryGopts, gopts
+	}
 
 	ctx, cancel := context.WithCancel(gopts.ctx)
 	defer cancel()
@@ -70,7 +74,7 @@ func runCopy(opts CopyOptions, gopts GlobalOptions, args []string) error {
 		return err
 	}
 
-	dstRepo, err := OpenRepository(dstGopts)
+	dstRepo, err := OpenRepository(secondaryGopts)
 	if err != nil {
 		return err
 	}

cmd/restic/cmd_init.go

@@ -110,7 +110,7 @@ func runInit(opts InitOptions, gopts GlobalOptions, args []string) error {
 
 func maybeReadChunkerPolynomial(opts InitOptions, gopts GlobalOptions) (*chunker.Pol, error) {
 	if opts.CopyChunkerParameters {
-		otherGopts, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "secondary")
+		otherGopts, _, err := fillSecondaryGlobalOpts(opts.secondaryRepoOptions, gopts, "secondary")
 		if err != nil {
 			return nil, err
 		}
@@ -124,7 +124,7 @@ func maybeReadChunkerPolynomial(opts InitOptions, gopts GlobalOptions) (*chunker
 		return &pol, nil
 	}
 
-	if opts.Repo != "" {
+	if opts.Repo != "" || opts.RepositoryFile != "" || opts.LegacyRepo != "" || opts.LegacyRepositoryFile != "" {
 		return nil, errors.Fatal("Secondary repository must only be specified when copying the chunker parameters")
 	}
 	return nil, nil

cmd/restic/integration_test.go

@@ -749,14 +749,17 @@ func TestBackupTags(t *testing.T) {
 }
 
 func testRunCopy(t testing.TB, srcGopts GlobalOptions, dstGopts GlobalOptions) {
+	gopts := srcGopts
+	gopts.Repo = dstGopts.Repo
+	gopts.password = dstGopts.password
 	copyOpts := CopyOptions{
 		secondaryRepoOptions: secondaryRepoOptions{
-			Repo:     dstGopts.Repo,
+			Repo:     srcGopts.Repo,
-			password: dstGopts.password,
+			password: srcGopts.password,
 		},
 	}
 
-	rtest.OK(t, runCopy(copyOpts, srcGopts, nil))
+	rtest.OK(t, runCopy(copyOpts, gopts, nil))
 }
 
 func TestCopy(t *testing.T) {

cmd/restic/secondary_repo.go

@@ -8,49 +8,98 @@ import (
 )
 
 type secondaryRepoOptions struct {
+	password string
+	// from-repo options
 	Repo            string
 	RepositoryFile  string
-	password        string
 	PasswordFile    string
 	PasswordCommand string
 	KeyHint         string
+	// repo2 options
+	LegacyRepo            string
+	LegacyRepositoryFile  string
+	LegacyPasswordFile    string
+	LegacyPasswordCommand string
+	LegacyKeyHint         string
 }
 
 func initSecondaryRepoOptions(f *pflag.FlagSet, opts *secondaryRepoOptions, repoPrefix string, repoUsage string) {
-	f.StringVarP(&opts.Repo, "repo2", "", os.Getenv("RESTIC_REPOSITORY2"), repoPrefix+" `repository` "+repoUsage+" (default: $RESTIC_REPOSITORY2)")
+	f.StringVarP(&opts.LegacyRepo, "repo2", "", os.Getenv("RESTIC_REPOSITORY2"), repoPrefix+" `repository` "+repoUsage+" (default: $RESTIC_REPOSITORY2)")
-	f.StringVarP(&opts.RepositoryFile, "repository-file2", "", os.Getenv("RESTIC_REPOSITORY_FILE2"), "`file` from which to read the "+repoPrefix+" repository location "+repoUsage+" (default: $RESTIC_REPOSITORY_FILE2)")
+	f.StringVarP(&opts.LegacyRepositoryFile, "repository-file2", "", os.Getenv("RESTIC_REPOSITORY_FILE2"), "`file` from which to read the "+repoPrefix+" repository location "+repoUsage+" (default: $RESTIC_REPOSITORY_FILE2)")
-	f.StringVarP(&opts.PasswordFile, "password-file2", "", os.Getenv("RESTIC_PASSWORD_FILE2"), "`file` to read the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_FILE2)")
+	f.StringVarP(&opts.LegacyPasswordFile, "password-file2", "", os.Getenv("RESTIC_PASSWORD_FILE2"), "`file` to read the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_FILE2)")
-	f.StringVarP(&opts.KeyHint, "key-hint2", "", os.Getenv("RESTIC_KEY_HINT2"), "key ID of key to try decrypting the "+repoPrefix+" repository first (default: $RESTIC_KEY_HINT2)")
+	f.StringVarP(&opts.LegacyKeyHint, "key-hint2", "", os.Getenv("RESTIC_KEY_HINT2"), "key ID of key to try decrypting the "+repoPrefix+" repository first (default: $RESTIC_KEY_HINT2)")
-	f.StringVarP(&opts.PasswordCommand, "password-command2", "", os.Getenv("RESTIC_PASSWORD_COMMAND2"), "shell `command` to obtain the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_COMMAND2)")
+	f.StringVarP(&opts.LegacyPasswordCommand, "password-command2", "", os.Getenv("RESTIC_PASSWORD_COMMAND2"), "shell `command` to obtain the "+repoPrefix+" repository password from (default: $RESTIC_PASSWORD_COMMAND2)")
+
+	// hide repo2 options
+	_ = f.MarkDeprecated("repo2", "use --repo or --from-repo instead")
+	_ = f.MarkDeprecated("repository-file2", "use --repository-file or --from-repository-file instead")
+	_ = f.MarkHidden("password-file2")
+	_ = f.MarkHidden("key-hint2")
+	_ = f.MarkHidden("password-command2")
+
+	f.StringVarP(&opts.Repo, "from-repo", "", os.Getenv("RESTIC_FROM_REPOSITORY"), "source `repository` "+repoUsage+" (default: $RESTIC_FROM_REPOSITORY)")
+	f.StringVarP(&opts.RepositoryFile, "from-repository-file", "", os.Getenv("RESTIC_FROM_REPOSITORY_FILE"), "`file` from which to read the source repository location "+repoUsage+" (default: $RESTIC_FROM_REPOSITORY_FILE)")
+	f.StringVarP(&opts.PasswordFile, "from-password-file", "", os.Getenv("RESTIC_FROM_PASSWORD_FILE2"), "`file` to read the source repository password from (default: $RESTIC_FROM_PASSWORD_FILE)")
+	f.StringVarP(&opts.KeyHint, "from-key-hint", "", os.Getenv("RESTIC_FROM_KEY_HINT"), "key ID of key to try decrypting the source repository first (default: $RESTIC_FROM_KEY_HINT)")
+	f.StringVarP(&opts.PasswordCommand, "from-password-command", "", os.Getenv("RESTIC_FROM_PASSWORD_COMMAND"), "shell `command` to obtain the source repository password from (default: $RESTIC_FROM_PASSWORD_COMMAND)")
 }
 
-func fillSecondaryGlobalOpts(opts secondaryRepoOptions, gopts GlobalOptions, repoPrefix string) (GlobalOptions, error) {
+func fillSecondaryGlobalOpts(opts secondaryRepoOptions, gopts GlobalOptions, repoPrefix string) (GlobalOptions, bool, error) {
-	if opts.Repo == "" && opts.RepositoryFile == "" {
+	if opts.Repo == "" && opts.RepositoryFile == "" && opts.LegacyRepo == "" && opts.LegacyRepositoryFile == "" {
-		return GlobalOptions{}, errors.Fatal("Please specify a " + repoPrefix + " repository location (--repo2 or --repository-file2)")
+		return GlobalOptions{}, false, errors.Fatal("Please specify a source repository location (--from-repo or --from-repository-file)")
 	}
 
-	if opts.Repo != "" && opts.RepositoryFile != "" {
+	hasFromRepo := opts.Repo != "" || opts.RepositoryFile != "" || opts.PasswordFile != "" ||
-		return GlobalOptions{}, errors.Fatal("Options --repo2 and --repository-file2 are mutually exclusive, please specify only one")
+		opts.KeyHint != "" || opts.PasswordCommand != ""
+	hasRepo2 := opts.LegacyRepo != "" || opts.LegacyRepositoryFile != "" || opts.LegacyPasswordFile != "" ||
+		opts.LegacyKeyHint != "" || opts.LegacyPasswordCommand != ""
+
+	if hasFromRepo && hasRepo2 {
+		return GlobalOptions{}, false, errors.Fatal("Option groups repo2 and from-repo are mutually exclusive, please specify only one")
 	}
 
 	var err error
 	dstGopts := gopts
-	dstGopts.Repo = opts.Repo
+	var pwdEnv string
-	dstGopts.RepositoryFile = opts.RepositoryFile
+
-	dstGopts.PasswordFile = opts.PasswordFile
+	if hasFromRepo {
-	dstGopts.PasswordCommand = opts.PasswordCommand
+		if opts.Repo != "" && opts.RepositoryFile != "" {
-	dstGopts.KeyHint = opts.KeyHint
+			return GlobalOptions{}, false, errors.Fatal("Options --from-repo and --from-repository-file are mutually exclusive, please specify only one")
+		}
+
+		dstGopts.Repo = opts.Repo
+		dstGopts.RepositoryFile = opts.RepositoryFile
+		dstGopts.PasswordFile = opts.PasswordFile
+		dstGopts.PasswordCommand = opts.PasswordCommand
+		dstGopts.KeyHint = opts.KeyHint
+
+		pwdEnv = "RESTIC_FROM_PASSWORD"
+		repoPrefix = "source"
+	} else {
+		if opts.LegacyRepo != "" && opts.LegacyRepositoryFile != "" {
+			return GlobalOptions{}, false, errors.Fatal("Options --repo2 and --repository-file2 are mutually exclusive, please specify only one")
+		}
+
+		dstGopts.Repo = opts.LegacyRepo
+		dstGopts.RepositoryFile = opts.LegacyRepositoryFile
+		dstGopts.PasswordFile = opts.LegacyPasswordFile
+		dstGopts.PasswordCommand = opts.LegacyPasswordCommand
+		dstGopts.KeyHint = opts.LegacyKeyHint
+
+		pwdEnv = "RESTIC_PASSWORD2"
+	}
+
 	if opts.password != "" {
 		dstGopts.password = opts.password
 	} else {
-		dstGopts.password, err = resolvePassword(dstGopts, "RESTIC_PASSWORD2")
+		dstGopts.password, err = resolvePassword(dstGopts, pwdEnv)
 		if err != nil {
-			return GlobalOptions{}, err
+			return GlobalOptions{}, false, err
 		}
 	}
 	dstGopts.password, err = ReadPassword(dstGopts, "enter password for "+repoPrefix+" repository: ")
 	if err != nil {
-		return GlobalOptions{}, err
+		return GlobalOptions{}, false, err
 	}
-	return dstGopts, nil
+	return dstGopts, hasFromRepo, nil
 }

cmd/restic/secondary_repo_test.go

@@ -14,6 +14,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) {
 	type secondaryRepoTestCase struct {
 		Opts     secondaryRepoOptions
 		DstGOpts GlobalOptions
+		FromRepo bool
 	}
 
 	//validSecondaryRepoTestCases is a list with test cases that must pass
@@ -28,6 +29,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) {
 				Repo:     "backupDst",
 				password: "secretDst",
 			},
+			FromRepo: true,
 		},
 		{
 			// Test if RepositoryFile and PasswordFile are parsed correctly.
@@ -40,6 +42,7 @@ func TestFillSecondaryGlobalOpts(t *testing.T) {
 				password:       "secretDst",
 				PasswordFile:   "passwordFileDst",
 			},
+			FromRepo: true,
 		},
 		{
 			// Test if RepositoryFile and PasswordCommand are parsed correctly.
@@ -52,6 +55,42 @@ func TestFillSecondaryGlobalOpts(t *testing.T) {
 				password:        "secretDst",
 				PasswordCommand: "echo secretDst",
 			},
+			FromRepo: true,
+		},
+		{
+			// Test if LegacyRepo and Password are parsed correctly.
+			Opts: secondaryRepoOptions{
+				LegacyRepo: "backupDst",
+				password:   "secretDst",
+			},
+			DstGOpts: GlobalOptions{
+				Repo:     "backupDst",
+				password: "secretDst",
+			},
+		},
+		{
+			// Test if LegacyRepositoryFile and LegacyPasswordFile are parsed correctly.
+			Opts: secondaryRepoOptions{
+				LegacyRepositoryFile: "backupDst",
+				LegacyPasswordFile:   "passwordFileDst",
+			},
+			DstGOpts: GlobalOptions{
+				RepositoryFile: "backupDst",
+				password:       "secretDst",
+				PasswordFile:   "passwordFileDst",
+			},
+		},
+		{
+			// Test if LegacyRepositoryFile and LegacyPasswordCommand are parsed correctly.
+			Opts: secondaryRepoOptions{
+				LegacyRepositoryFile:  "backupDst",
+				LegacyPasswordCommand: "echo secretDst",
+			},
+			DstGOpts: GlobalOptions{
+				RepositoryFile:  "backupDst",
+				password:        "secretDst",
+				PasswordCommand: "echo secretDst",
+			},
 		},
 	}
 
@@ -96,6 +135,20 @@ func TestFillSecondaryGlobalOpts(t *testing.T) {
 				Repo: "backupDst",
 			},
 		},
+		{
+			// Test must fail as current and legacy options are mixed
+			Opts: secondaryRepoOptions{
+				Repo:       "backupDst",
+				LegacyRepo: "backupDst",
+			},
+		},
+		{
+			// Test must fail as current and legacy options are mixed
+			Opts: secondaryRepoOptions{
+				Repo:                  "backupDst",
+				LegacyPasswordCommand: "notEmpty",
+			},
+		},
 	}
 
 	//gOpts defines the Global options used in the secondary repository tests
@@ -119,14 +172,15 @@ func TestFillSecondaryGlobalOpts(t *testing.T) {
 
 	// Test all valid cases
 	for _, testCase := range validSecondaryRepoTestCases {
-		DstGOpts, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination")
+		DstGOpts, isFromRepo, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination")
 		rtest.OK(t, err)
 		rtest.Equals(t, DstGOpts, testCase.DstGOpts)
+		rtest.Equals(t, isFromRepo, testCase.FromRepo)
 	}
 
 	// Test all invalid cases
 	for _, testCase := range invalidSecondaryRepoTestCases {
-		_, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination")
+		_, _, err := fillSecondaryGlobalOpts(testCase.Opts, gOpts, "destination")
 		rtest.Assert(t, err != nil, "Expected error, but function did not return an error")
 	}
 }

doc/045_working_with_repos.rst

@@ -90,7 +90,7 @@ example from a local to a remote repository, you can use the ``copy`` command:
 
 .. code-block:: console
 
-    $ restic -r /srv/restic-repo copy --repo2 /srv/restic-repo-copy
+    $ restic -r /srv/restic-repo-copy copy --from-repo /srv/restic-repo
     repository d6504c63 opened successfully, password is correct
     repository 3dd0878c opened successfully, password is correct
 
@@ -117,17 +117,17 @@ be skipped by later copy runs.
     both the source and destination repository, *may occupy up to twice their
     space* in the destination repository. See below for how to avoid this.
 
-The destination repository is specified with ``--repo2`` or can be read 
+The source repository is specified with ``--from-repo`` or can be read
-from a file specified via ``--repository-file2``. Both of these options
+from a file specified via ``--from-repository-file``. Both of these options
-can also set as environment variables ``$RESTIC_REPOSITORY2`` or
+can also be set as environment variables ``$RESTIC_FROM_REPOSITORY`` or
-``$RESTIC_REPOSITORY_FILE2`` respectively. For the destination repository 
+``$RESTIC_FROM_REPOSITORY_FILE``, respectively. For the destination repository
-the password can be read from a file ``--password-file2`` or from a command 
+the password can be read from a file ``--from-password-file`` or from a command
-``--password-command2``.
+``--from-password-command``.
-Alternatively the environment variables ``$RESTIC_PASSWORD_COMMAND2`` and
+Alternatively the environment variables ``$RESTIC_FROM_PASSWORD_COMMAND`` and
-``$RESTIC_PASSWORD_FILE2`` can be used. It is also possible to directly
+``$RESTIC_FROM_PASSWORD_FILE`` can be used. It is also possible to directly
-pass the password via ``$RESTIC_PASSWORD2``. The key which should be used
+pass the password via ``$RESTIC_FROM_PASSWORD``. The key which should be used
-for decryption can be selected by passing its ID via the flag ``--key-hint2``
+for decryption can be selected by passing its ID via the flag ``--from-key-hint``
-or the environment variable ``$RESTIC_KEY_HINT2``.
+or the environment variable ``$RESTIC_FROM_KEY_HINT``.
 
 .. note:: In case the source and destination repository use the same backend,
     the configuration options and environment variables used to configure the