Define mirror targets one way: remote.<name>.<field>

There were two spellings for the same thing -- a flat remoteurl/remotekey/remotetype/remotevisibility set for a single server, and remote.<name>.* blocks for several. The flat one is gone; every target, including a lone one, is now a named block with the fields url, key, type and visibility. An existing ~/.mgshrc is converted on the next start. Only the key is rewritten, so values, comments, alignment, commented-out lines and the file's 0600 mode survive untouched, and mgsh prints each rename rather than doing it quietly. The target is named "public", which is what the old settings called the git remote they created, so a converted setup keeps pushing to the same place under the same remote name. A file that carries both spellings keeps what the new one says. The environment follows the same shape: MGSH_REMOTEURL and friends are replaced by MGSH_REMOTE_<NAME>_<FIELD>, so MGSH_REMOTE_GITLAB_KEY sets remote.gitlab.key. The field is read from the end of the variable name, which leaves target names free to contain underscores. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-26 18:00:26 +02:00
parent 61a7059f61
commit 2a622046f2
7 changed files with 301 additions and 99 deletions
@@ -468,7 +468,7 @@ remote.broken.url       = https://nowhere.example    # no key -> unusable
 }

 func TestMirrorTargetsLegacyAndSelection(t *testing.T) {
-	// the flat remoteurl/remotekey pair stays supported, as target "public"
+	// the pre-4.1 flat pair still loads, folded onto the target "public"
 	var c Config
 	applyConfig(&c, parseConfig("remoteurl = https://git.example.com\nremotekey = tok\n"))
 	usable, _ := c.mirrorTargets()
@@ -1049,3 +1049,101 @@ func TestParseDuSizes(t *testing.T) {
 		}
 	}
 }
+
+// TestMigrateRemoteKeys rewrites the pre-4.1 flat spelling in place. Only the
+// key changes: values, comments and everything else stay byte for byte.
+func TestMigrateRemoteKeys(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, ".mgshrc")
+	before := `# my config
+base             = /home/me/src
+remoteurl        = https://git.example.com   # the mirror
+remotekey        = s3cr3t-token
+  remotevisibility: public
+# remotetype     = gitea      (commented out, must stay put)
+alias co 'checkout $1'
+`
+	if err := os.WriteFile(path, []byte(before), 0600); err != nil {
+		t.Fatal(err)
+	}
+	out := captureStdout(t, func() { migrateRemoteKeys(path, before) })
+
+	got := readFile(t, path)
+	for _, want := range []string{
+		"remote.public.url        = https://git.example.com   # the mirror",
+		"remote.public.key        = s3cr3t-token",
+		"  remote.public.visibility: public",
+		"# remotetype     = gitea      (commented out, must stay put)",
+		"base             = /home/me/src",
+		"alias co 'checkout $1'",
+	} {
+		if !strings.Contains(got, want) {
+			t.Errorf("migrated file missing %q:\n%s", want, got)
+		}
+	}
+	if strings.Contains(got, "\nremoteurl") || strings.Contains(got, "\nremotekey") {
+		t.Errorf("old spelling left behind:\n%s", got)
+	}
+	if !strings.Contains(out, "remoteurl → remote.public.url") {
+		t.Errorf("migration was not reported: %q", out)
+	}
+	// the file keeps its private mode
+	if fi, err := os.Stat(path); err != nil {
+		t.Fatal(err)
+	} else if fi.Mode().Perm() != 0o600 {
+		t.Errorf("mode after migration = %04o, want 0600", fi.Mode().Perm())
+	}
+
+	// running again changes nothing and says nothing
+	second := captureStdout(t, func() { migrateRemoteKeys(path, readFile(t, path)) })
+	if strings.TrimSpace(second) != "" {
+		t.Errorf("a converted file was migrated again: %q", second)
+	}
+	if readFile(t, path) != got {
+		t.Error("a second migration changed the file")
+	}
+}
+
+// TestLegacyKeysDoNotOverrideExplicitOnes: a config carrying both spellings must
+// keep what the new one says.
+func TestLegacyKeysDoNotOverrideExplicitOnes(t *testing.T) {
+	var c Config
+	applyConfig(&c, parseConfig(
+		"remoteurl = https://old.example\nremote.public.url = https://new.example\n"+
+			"remote.public.key = tok\n"))
+	targets, _ := c.mirrorTargets()
+	if len(targets) != 1 || targets[0].URL != "https://new.example" {
+		t.Errorf("targets = %+v, want the remote.public.url value", targets)
+	}
+}
+
+// TestRemoteEnvOverrides: MGSH_REMOTE_<NAME>_<FIELD> is the environment
+// spelling of remote.<name>.<field>.
+func TestRemoteEnvOverrides(t *testing.T) {
+	t.Setenv("MGSH_REMOTE_GITLAB_URL", "https://gitlab.example")
+	t.Setenv("MGSH_REMOTE_GITLAB_KEY", "env-token")
+	t.Setenv("MGSH_REMOTE_GITLAB_VISIBILITY", "public")
+	t.Setenv("MGSH_REMOTE_MY_HUB_URL", "https://hub.example") // name with an underscore
+	t.Setenv("MGSH_REMOTE_MY_HUB_KEY", "hub-token")
+	t.Setenv("MGSH_REMOTES", "") // must not be mistaken for a target field
+
+	var c Config
+	applyConfig(&c, parseConfig("remote.gitlab.url = https://from-file.example\nremote.gitlab.key = file-token\n"))
+	applyEnv(&c)
+
+	targets, incomplete := c.mirrorTargets()
+	if len(incomplete) != 0 {
+		t.Fatalf("incomplete targets: %v", incomplete)
+	}
+	byName := map[string]RemoteTarget{}
+	for _, tg := range targets {
+		byName[tg.Name] = tg
+	}
+	if g := byName["gitlab"]; g.URL != "https://gitlab.example" || g.Key != "env-token" || g.Vis != "public" {
+		t.Errorf("env did not override the file: %+v", g)
+	}
+	// the field is taken from the end, so the name may contain underscores
+	if h := byName["my_hub"]; h.URL != "https://hub.example" || h.Key != "hub-token" {
+		t.Errorf("MGSH_REMOTE_MY_HUB_* = %+v, want target my_hub", h)
+	}
+}