diff --git a/README.md b/README.md index 9d7fcbb..b236567 100644 --- a/README.md +++ b/README.md @@ -9,6 +9,7 @@ directory. Go port of the original Perl `mgsh` (`mgsh.perl`). - [Build](#build) · [Usage](#usage) · [Commands](#commands) · [Aliases](#aliases) - [Overview](#overview) · [Credential check](#credential-check) - [Public mirror (`pushremote`)](#public-mirror-pushremote) · + [Deleting a mirror (`deleteremote`)](#deleting-a-mirror-deleteremote) · [Releases](#releases) - [Configuration](#configuration) · [Settings reference](#settings-reference) · [Per-project configuration](#per-project-configuration) @@ -62,9 +63,9 @@ stand in. Outside `base` no project is selected. `mgsh ` starts the interactive shell with that project preselected. The commands available directly from the shell are `clone`, `init`, `log`, -`push`, `pushremote`, `release`, `list`, `tag`, `archive`, `show`, -`pull`, `fetch`, `status`, `diff`, `overview`, `config`, `count`, `login` and -`cloneall`; every other command is interactive-only. +`push`, `pushremote`, `deleteremote`, `release`, `list`, `tag`, `archive`, +`show`, `pull`, `fetch`, `status`, `diff`, `overview`, `config`, `count`, +`login` and `cloneall`; every other command is interactive-only. The interactive prompt is colored (Catppuccin-flavored) and shows the active project, its git branch and a `*` dirty marker: @@ -130,6 +131,7 @@ Run `help` for the full list. Highlights: | `cd [project]` | change project (no argument: back to the base) | | `push [comment]` | commit everything and push to the server | | `pushremote [desc]` | mirror the repo to a public server (gitea/github/gitlab) | +| `deleteremote <@name\|host>` | delete the repo on that public server (asks first) | | `pull` / `fetch` | pull / fetch from the server | | `status [-a]` / `diff` | short git status (`-a`: overview of all projects) | | `overview` | inventory of all projects, local and on the server | @@ -321,6 +323,7 @@ remote.gitlab.url = https://gitlab.example.com remote.gitlab.key = remote.gitlab.type = gitlab remote.gitlab.visibility = public +remote.gitlab.active = false # only used when named (default true) # remotes = gitea, gitlab # optional: restrict and order the set # mirror = true # `push` also mirrors via pushremote @@ -333,10 +336,29 @@ name those versions used. | command | pushes to | |------------------------|-----------------------------------------------| -| `pushremote` | every configured target, in order | -| `pushremote @gitea` | only `gitea` | +| `pushremote` | every **active** target, in order | +| `pushremote @gitea` | only `gitea`, active or not | | `pushremote @gitea @gitlab` | those two | -| `pushremote a fix` | every target, description "a fix" | +| `pushremote a fix` | every active target, description "a fix" | + +#### Active and inactive targets + +`remote..active = false` takes a target out of the default set: it is +skipped by a bare `pushremote`, by `release` without a `@name`, and by the +automatic mirror of `mirror = true`. Naming it uses it exactly as before — +`pushremote @gitea`, `release @gitea v1.2`, or by host, `pushremote @git.example.com`. + +That is the difference to the alternatives: removing the block loses the token, +and leaving the target in `remotes = …` keeps it on the automatic path. An +inactive target stays configured, keeps its credentials, is listed by `config` +(a shade dimmer, marked `inactive`) and completes with Tab — it just no longer +goes along by default. Useful for a server that is behind a VPN, rate-limited, +or only occasionally interesting. + +Unset means active, so a configuration written before this flag existed keeps +mirroring where it always did. When *every* configured target is inactive, a +bare `pushremote` says so and names one instead of claiming nothing is +configured. Each target owns a git remote of the same name in the repository, so `git push gitlab` keeps working outside mgsh. A @@ -358,6 +380,52 @@ existing file is readable by others. The provider is auto-detected from the url with `remote..type`. Set `mirror = true` to have every `push` mirror automatically. +### Deleting a mirror (`deleteremote`) + +`deleteremote` is the counterpart: it deletes the active project's repository on +a public server, through the same API and with the same token. + +``` +deleteremote <@name|host> [...] +``` + +``` +< src/myproject > deleteremote git.example.com +remote gitea https://git.example.com (as mike) +this deletes https://git.example.com/mike/myproject.git with its issues, +releases and history, and cannot be undone +delete mike/myproject on gitea? y/N ? y +deleted mike/myproject on gitea +removed git remote gitea +``` + +A target can be named either way: `@gitea` (its configured name) or +`git.example.com` (the host of its url) select the same server, and the `@` is +optional here. Both spellings work for `pushremote` and `release` too. +`active = false` changes nothing for this command — it always needs the server +named anyway. + +The guard rails, because nothing on the other end can be undone from here: + +- **the server must be named.** Unlike `pushremote`, a bare `deleteremote` does + *not* mean "every configured target" — it lists them and stops. +- the repository is looked up first, so a typo is reported as "nothing to + delete" instead of becoming a request nobody meant to send. +- each deletion is confirmed on its own, naming owner/repo and server, and the + default answer is **no**. +- the **local clone is never touched**. Only the git remote of that name is + dropped, and only while it still points at the repository that was deleted. + +Only the mirror is affected — the repository on the internal ssh git server, and +everything `list`/`archive` deal with, stays where it is. + +Deletion needs more from the token than pushing does: the `delete_repo` scope on +GitHub, `write:repository` on Gitea, and the `api` scope plus the Owner role on +GitLab. When it is missing the server answers 403, and mgsh names the scope +rather than passing the bare status on. GitLab may only *schedule* the deletion +(HTTP 202): depending on the plan the project stays visible until its retention +period is over. + ### Releases `release` turns a commit into a published release on the mirror servers, in one @@ -373,7 +441,7 @@ release [@name ...] [notes] remote hub released https://github.com/mike/mgsh.git ``` -Without `@name` it releases to every configured mirror target, exactly like +Without `@name` it releases to every active mirror target, exactly like `pushremote`. Everything after the tag becomes the release notes *and* the tag's annotation. @@ -491,6 +559,7 @@ setting. | `remote..key` | project | API token for that target | | `remote..type` | project | `gitea`\|`github`\|`gitlab`; auto-detected from the url when unset | | `remote..visibility` | project | `private` (default) or `public` for repositories `pushremote` creates | +| `remote..active` | project | `false` keeps the target out of the default set; it is then only used when named (`pushremote @`). Default `true` | | `remotes` | project | comma- or space-separated list restricting and ordering the mirror targets | | `mirror` | project | truthy (`1`/`true`/`yes`/`on`) → every `push` also mirrors | | `secretscan` | project | `off` disables the credential check `push` runs before committing (on by default; only an explicit `off` disables it) | diff --git a/alias.go b/alias.go index 643ce21..febcc6e 100644 --- a/alias.go +++ b/alias.go @@ -44,7 +44,7 @@ var builtinCmds = map[string]bool{ "": true, "quit": true, "exit": true, "help": true, "rescan": true, "dist": true, "list": true, "show": true, "log": true, "status": true, "diff": true, "pull": true, "fetch": true, "push": true, "edit": true, - "pushremote": true, "overview": true, "archive": true, "init": true, + "pushremote": true, "deleteremote": true, "overview": true, "archive": true, "init": true, "login": true, "cd": true, "checkout": true, "clone": true, "cloneall": true, "count": true, "tag": true, "alias": true, "unalias": true, "config": true, "release": true, diff --git a/commands.go b/commands.go index 499d7e8..854bb44 100644 --- a/commands.go +++ b/commands.go @@ -191,7 +191,7 @@ func runCommandDepth(line string, depth int) bool { case "config": // show the effective configuration and where it came from if opt["k"] { fmt.Println(strings.Join(configKeys(), "\n")) - fmt.Println("remote..url|key|type|visibility") + fmt.Println("remote..url|key|type|visibility|active") break } showConfig() @@ -384,13 +384,16 @@ func runCommandDepth(line string, depth int) bool { break } sshOK("touch " + shq(cfg.GitPath+"/"+PRJ+".git")) - if targets, _ := cfg.mirrorTargets(); truthy(cfg.Mirror) && len(targets) > 0 { - handlePushRemote("") // auto-mirror to every configured server + if targets, _ := cfg.mirrorTargets(); truthy(cfg.Mirror) && len(activeRemotes(targets)) > 0 { + handlePushRemote("") // auto-mirror to every active server } case "pushremote": // mirror the repo to a public git server via its API handlePushRemote(strings.Join(fields[1:], " ")) + case "deleteremote": // delete the repo on a public git server via its API + handleDeleteRemote(strings.Join(fields[1:], " ")) + case "release": // tag a commit and publish it as a release on the mirrors handleRelease(strings.Join(fields[1:], " ")) @@ -773,6 +776,7 @@ var helpItems = []struct{ cmd, desc string }{ {"init", "make new repository from current directory"}, {"push [comment]", "push changes to git server"}, {"pushremote [@name] [desc]", "mirror repo to the public server(s) (gitea/github/gitlab)"}, + {"deleteremote <@name|host>", "delete the repo on that public server (asks first)"}, {"release [notes]", "tag and publish a release on the public server(s)"}, {"pull", "pull changes from git server"}, {"fetch", "fetch changes from git server"}, diff --git a/completion.go b/completion.go index db81899..74e22b7 100644 --- a/completion.go +++ b/completion.go @@ -61,6 +61,7 @@ func builtinCompleter() *readline.PrefixCompleter { readline.PcItem("list", readline.PcItem("-a")), readline.PcItem("push"), readline.PcItem("pushremote", readline.PcItemDynamic(dynRemoteNames)), + readline.PcItem("deleteremote", readline.PcItemDynamic(dynRemoteNames)), readline.PcItem("release", readline.PcItemDynamic(dynRemoteNames)), readline.PcItem("pull"), readline.PcItem("fetch"), diff --git a/config.go b/config.go index 2e8a73d..996ea5b 100644 --- a/config.go +++ b/config.go @@ -43,13 +43,23 @@ type Config struct { // `remote..` block. Name doubles as the git remote name created in // the repository, so several targets can coexist side by side. type RemoteTarget struct { - Name string - URL string - Key string - Type string // "gitea"|"github"|"gitlab" (auto-detected when empty) - Vis string // "private" (default) | "public" + Name string + URL string + Key string + Type string // "gitea"|"github"|"gitlab" (auto-detected when empty) + Vis string // "private" (default) | "public" + Active string // falsy -> taken along only when named (default: on) } +// isActive reports whether the target takes part in a command that was given no +// target of its own — `pushremote`, `release`, an auto-mirroring `push`. An +// inactive one is not disabled, it is merely off the automatic path: naming it +// (`pushremote @gitea`) uses it as it always did. +// +// Unset means active. The setting exists to take a server *out* of the default +// set, so a configuration written before it existed must keep behaving. +func (t RemoteTarget) isActive() bool { return !falsy(t.Active) } + // legacyRemoteName is the target the pre-4.1 flat remoteurl/remotekey settings // are migrated to. It matches the git remote those versions created, so a // converted configuration keeps pushing to the same place. @@ -441,7 +451,7 @@ func foldLegacyRemoteKeys(m map[string]string) map[string]string { } // remoteFieldRe matches a named mirror target setting: remote... -var remoteFieldRe = regexp.MustCompile(`^remote\.([a-z0-9_.-]+)\.(url|key|type|visibility)$`) +var remoteFieldRe = regexp.MustCompile(`^remote\.([a-z0-9_.-]+)\.(url|key|type|visibility|active)$`) // applyRemoteTargets merges `remote..` settings into c.Remotes. // An already known target is updated field by field, so a project .mgshrc can @@ -472,6 +482,8 @@ func applyRemoteTargets(c *Config, m map[string]string) { t.Type = v case "visibility": t.Vis = v + case "active": + t.Active = v } } } @@ -510,7 +522,7 @@ func applyEnv(c *Config) { } // remoteFields are the settings a mirror target is made of. -var remoteFields = []string{"url", "key", "type", "visibility"} +var remoteFields = []string{"url", "key", "type", "visibility", "active"} // applyRemoteEnv reads MGSH_REMOTE__, the environment spelling of // a remote.. setting — MGSH_REMOTE_GITLAB_KEY for @@ -552,6 +564,8 @@ func applyRemoteEnv(c *Config) { t.Type = value case "visibility": t.Vis = value + case "active": + t.Active = value } } } diff --git a/deleteremote.go b/deleteremote.go new file mode 100644 index 0000000..b00e1f4 --- /dev/null +++ b/deleteremote.go @@ -0,0 +1,184 @@ +package main + +// deleteremote.go — the `deleteremote` command: remove the active project's +// repository from a public mirror server via that server's REST API. +// +// This is the only mgsh command that destroys something on a server it does not +// own, and nothing here can undo it. Four things keep it in check: +// +// - the server has to be named. `pushremote` without one means "every +// configured target", which is convenient there and catastrophic here. +// - the repository is looked up first, so a typo comes back as "not on that +// server" instead of as a DELETE nobody meant to send. +// - every deletion is confirmed on its own, spelling out owner/repo and the +// server it lives on, and the default answer is no. +// - the local clone is never touched. Only the git remote `pushremote` +// created is dropped, and only while it still points at what was deleted. + +import ( + "fmt" + "strings" +) + +// deleteRepo removes owner/repo from the server. +// +// The providers disagree on what success looks like: Gitea and GitHub answer +// 204, GitLab 202 — there the deletion is merely scheduled, and depending on +// the plan the project stays visible until its retention period is over. +func (r *remoteAPI) deleteRepo(owner, repo string) error { + code, data, err := r.do("DELETE", r.repoPath(owner, repo), nil) + if err != nil { + return err + } + switch code { + case 200, 202, 204: + return nil + case 403: + // the common failure by far: the token can push and create, but was + // never given the separate permission a deletion needs + return fmt.Errorf("not allowed to delete %s/%s — the token needs %s (HTTP 403): %s", + owner, repo, r.deletePermission(), firstLine(data)) + default: + return fmt.Errorf("deleting repository failed (HTTP %d): %s", code, firstLine(data)) + } +} + +// deletePermission names what a provider wants from a token before it will +// delete a repository, because a bare "403" sends people to the wrong setting. +func (r *remoteAPI) deletePermission() string { + switch r.kind { + case kindGitHub: + return "the 'delete_repo' scope" + case kindGitLab: + return "the 'api' scope and the Owner role" + default: // Gitea + return "the 'write:repository' scope" + } +} + +// parseRemoteSelectors splits `deleteremote ...` into the targets +// it names. The '@' that `pushremote` requires is accepted but optional here: +// the command takes nothing but selectors, so there is no description a bare +// word could be mistaken for. +func parseRemoteSelectors(args string) []string { + var out []string + for _, f := range strings.Fields(args) { + if s := strings.TrimPrefix(f, "@"); s != "" { + out = append(out, s) + } + } + return out +} + +// handleDeleteRemote implements `deleteremote <@name|host> ...`: it deletes the +// active project's repository on each named mirror server. +func handleDeleteRemote(args string) { + if !requireProject() { + return + } + targets, incomplete := cfg.mirrorTargets() + for _, n := range incomplete { + errorln("remote " + n + ": url or key missing — skipped") + } + if len(targets) == 0 { + errorln("deleteremote needs a 'remote..url' and 'remote..key' in " + configFile()) + return + } + + // no selector is never "all of them" — that is the whole point of the + // command taking one + sels := parseRemoteSelectors(args) + if len(sels) == 0 { + errorln("deleteremote needs the server to delete from: " + remoteChoices(targets)) + return + } + + targets = pickRemotes(targets, sels) + if len(targets) == 0 { + return // pickRemotes already named the selectors it did not recognise + } + + repo := PRJ // like pushremote: the project names the repository + done := 0 + for _, t := range targets { + if deleteOnRemote(t, repo) { + done++ + } + } + if len(targets) > 1 { + fmt.Printf("%s %d/%d repositories deleted\n", col(cDark, "deleteremote:"), done, len(targets)) + } +} + +// remoteChoices lists the configured targets the way they may be selected, for +// the message a `deleteremote` without a target earns. +func remoteChoices(targets []RemoteTarget) string { + out := make([]string, 0, len(targets)) + for _, t := range targets { + if h := remoteHost(t.URL); h != "" && !strings.EqualFold(h, t.Name) { + out = append(out, "@"+t.Name+" ("+h+")") + continue + } + out = append(out, "@"+t.Name) + } + return strings.Join(out, ", ") +} + +// deleteOnRemote deletes repo on one target, after asking. It reports whether +// something was actually deleted — a repository that is not there, and a +// question answered with no, are not failures, and neither stops the remaining +// targets. +func deleteOnRemote(t RemoteTarget, repo string) bool { + api := newRemoteAPI(t.URL, t.Key, t.Type) + + owner, err := api.authUser() + if err != nil { + errorln(t.Name + ": " + err.Error()) + return false + } + fmt.Printf("%s %s %s (as %s)\n", + col(cDark, "remote"), col(cYellow, t.Name), col(cBlue, api.url), col(cGreen, owner)) + + exists, err := api.repoExists(owner, repo) + if err != nil { + errorln(t.Name + ": " + err.Error()) + return false + } + if !exists { + fmt.Println(col(cDark, "no repository "+owner+"/"+repo+" there — nothing to delete")) + return false + } + + fmt.Println(col(cOrange, "this deletes "+api.repoWebURL(owner, repo)+ + " with its issues, releases and history, and cannot be undone")) + if !yesno("delete "+owner+"/"+repo+" on "+t.Name+"?", false) { + fmt.Println(col(cDark, "kept")) + return false + } + + if err := api.deleteRepo(owner, repo); err != nil { + errorln(t.Name + ": " + err.Error()) + return false + } + fmt.Println(col(cRed, "deleted ") + col(cWhite, owner+"/"+repo) + col(cDark, " on "+t.Name)) + + dropGitRemote(t.Name, api.repoWebURL(owner, repo)) + return true +} + +// dropGitRemote removes the local git remote named name, but only while it +// still points at web — the repository just deleted. A remote the user has +// since re-aimed somewhere else is theirs, not ours, and a `git push` failing +// against a repository that no longer exists is worse than no remote at all. +func dropGitRemote(name, web string) { + if !isDir(DIR + "/.git") { + return + } + url, err := gitCapture(DIR, "remote", "get-url", name) + if err != nil || strings.TrimSpace(url) != web { + return + } + if gitOK(DIR, "remote", "remove", name) { + fmt.Println(col(cDark, "removed git remote "+name)) + } +} diff --git a/deleteremote_test.go b/deleteremote_test.go new file mode 100644 index 0000000..aee045a --- /dev/null +++ b/deleteremote_test.go @@ -0,0 +1,266 @@ +package main + +// deleteremote_test.go — the command that deletes a repository on a server mgsh +// does not own. +// +// Nothing it does can be undone from here, so the tests are less about the happy +// path than about the ways it must refuse: no target named, an unknown target, a +// question answered with no, a repository that is not there. Each of those has +// to leave the server untouched, which is asserted on the requests the fake +// provider received — the DELETE either went out or it did not. + +import ( + "strings" + "testing" +) + +// mirrorServer is a fake Gitea holding one repository owned by "mike". +func mirrorServer(t *testing.T, project string) *fakeProvider { + t.Helper() + f := newFakeProvider(t) + f.route("GET /api/v1/user", 200, `{"login":"mike"}`) + f.route("GET /api/v1/repos/mike/"+project, 200, `{}`) + f.route("DELETE /api/v1/repos/mike/"+project, 204, "") + return f +} + +// mirrorProject points the globals at a project with f as its single mirror +// target, and answers every y/n question with answer. It returns the project +// directory and the recorded questions. +func mirrorProject(t *testing.T, f *fakeProvider, name string, answer bool) (string, *[]string) { + t.Helper() + dir := useProject(t, name) + cfg.Remotes = []RemoteTarget{{Name: "gitea", URL: f.URL, Key: "tok", Type: "gitea"}} + return dir, fakeAnswers(t, answer) +} + +// deleted reports whether the DELETE for project actually went to the server. +func deleted(f *fakeProvider, project string) bool { + return f.find("DELETE /api/v1/repos/mike/"+project) != nil +} + +// TestDeleteRepoPerProvider pins the request each provider expects. GitLab +// answers 202 — the deletion is only scheduled — which still counts as success. +func TestDeleteRepoPerProvider(t *testing.T) { + for _, c := range []struct { + typ, path string + code int + }{ + {"gitea", "/api/v1/repos/mike/mgsh", 204}, + {"github", "/api/v3/repos/mike/mgsh", 204}, + {"gitlab", "/api/v4/projects/mike%2Fmgsh", 202}, + } { + f := newFakeProvider(t) + f.route("DELETE "+c.path, c.code, "") + + if err := newRemoteAPI(f.URL, "tok", c.typ).deleteRepo("mike", "mgsh"); err != nil { + t.Fatalf("%s: deleteRepo: %v (requests: %v)", c.typ, err, f.paths()) + } + if f.find("DELETE "+c.path) == nil { + t.Errorf("%s: expected DELETE %s, got %v", c.typ, c.path, f.paths()) + } + } +} + +// TestDeleteRepoNamesTheMissingPermission: deleting needs more of a token than +// pushing does, so a 403 here is the likeliest failure of all. Passing the bare +// status on would send people to the wrong setting. +func TestDeleteRepoNamesTheMissingPermission(t *testing.T) { + f := newFakeProvider(t) + f.route("DELETE /api/v3/repos/mike/mgsh", 403, `{"message":"Must have admin rights"}`) + + err := newRemoteAPI(f.URL, "tok", "github").deleteRepo("mike", "mgsh") + if err == nil || !strings.Contains(err.Error(), "delete_repo") { + t.Errorf("403 error = %v, want it to name the delete_repo scope", err) + } +} + +// TestDeleteRepoTreatsUnroutedAsFailure: anything the server did not accept must +// surface as an error, or a repository that is still there looks deleted. +func TestDeleteRepoReportsFailure(t *testing.T) { + f := newFakeProvider(t) // everything 404s + if err := newRemoteAPI(f.URL, "tok", "gitea").deleteRepo("mike", "mgsh"); err == nil { + t.Error("deleteRepo accepted a 404 as success") + } +} + +// TestDeleteRemoteNeedsATarget is the one that matters most: `pushremote` with +// no @name means "every configured server", and inheriting that here would wipe +// the project off all of them at once. +func TestDeleteRemoteNeedsATarget(t *testing.T) { + f := mirrorServer(t, "notes") + _, asked := mirrorProject(t, f, "notes", true) // even a standing "yes" + + out := captureStdout(t, func() { runCommand("deleteremote") }) + + if len(f.got) != 0 { + t.Fatalf("deleteremote talked to the server without being told where: %v", f.paths()) + } + if len(*asked) != 0 { + t.Errorf("asked %q although no target was named", *asked) + } + if !strings.Contains(out, "@gitea") { + t.Errorf("output does not say which targets exist: %q", out) + } +} + +// TestDeleteRemoteRejectsUnknownTarget: a mistyped server name must not fall +// back to some other target. +func TestDeleteRemoteRejectsUnknownTarget(t *testing.T) { + f := mirrorServer(t, "notes") + _, asked := mirrorProject(t, f, "notes", true) + + out := captureStdout(t, func() { runCommand("deleteremote @gitae") }) + + if len(f.got) != 0 { + t.Fatalf("an unknown target still reached a server: %v", f.paths()) + } + if len(*asked) != 0 { + t.Errorf("asked %q for an unknown target", *asked) + } + if !strings.Contains(out, "gitae") { + t.Errorf("unknown target not reported: %q", out) + } +} + +// TestDeleteRemoteKeepsRepoWhenDeclined: the question is the last guard, so a +// "no" has to stop the DELETE, not just the message about it. +func TestDeleteRemoteKeepsRepoWhenDeclined(t *testing.T) { + f := mirrorServer(t, "notes") + _, asked := mirrorProject(t, f, "notes", false) + + captureStdout(t, func() { runCommand("deleteremote @gitea") }) + + if len(*asked) != 1 { + t.Fatalf("questions asked = %q, want exactly one", *asked) + } + if !strings.Contains((*asked)[0], "mike/notes") { + t.Errorf("question %q does not name the repository being deleted", (*asked)[0]) + } + if deleted(f, "notes") { + t.Fatalf("deleted the repository after the user declined: %v", f.paths()) + } +} + +// TestDeleteRemoteWithoutRepositoryAsksNothing: a project that was never +// mirrored (or a typo in the project name) is a no-op, not a question. +func TestDeleteRemoteWithoutRepositoryAsksNothing(t *testing.T) { + f := newFakeProvider(t) + f.route("GET /api/v1/user", 200, `{"login":"mike"}`) // the repo lookup 404s + _, asked := mirrorProject(t, f, "notes", true) + + out := captureStdout(t, func() { runCommand("deleteremote @gitea") }) + + if len(*asked) != 0 { + t.Errorf("asked %q about a repository that is not there", *asked) + } + if deleted(f, "notes") { + t.Error("sent a DELETE for a repository the server does not have") + } + if !strings.Contains(out, "nothing to delete") { + t.Errorf("output = %q, want it to say there is nothing to delete", out) + } +} + +// TestDeleteRemoteByHostAndGitRemoteCleanup covers the whole accepted path: the +// server picked by its host rather than its configured name, the DELETE sent, +// and the now-dangling git remote dropped — while an unrelated remote stays. +func TestDeleteRemoteByHostAndGitRemoteCleanup(t *testing.T) { + f := mirrorServer(t, "notes") + dir, asked := mirrorProject(t, f, "notes", true) + + mustGit(t, dir, "init", "-q") + mustGit(t, dir, "remote", "add", "gitea", f.URL+"/mike/notes.git") + mustGit(t, dir, "remote", "add", "origin", "git@git.example:notes.git") + + captureStdout(t, func() { runCommand("deleteremote " + remoteHost(f.URL)) }) + + if len(*asked) != 1 { + t.Fatalf("questions asked = %q, want exactly one", *asked) + } + if !deleted(f, "notes") { + t.Fatalf("no DELETE sent, requests: %v", f.paths()) + } + remotes, err := gitCapture(dir, "remote") + if err != nil { + t.Fatal(err) + } + names := splitLines(strings.TrimSpace(remotes)) + for _, n := range names { + if n == "gitea" { + t.Errorf("git remote gitea survived the deletion: %q", names) + } + } + if len(names) != 1 || names[0] != "origin" { + t.Errorf("git remotes = %q, want origin left alone", names) + } +} + +// TestDeleteRemoteKeepsARepointedGitRemote: the local remote is only dropped +// while it still points at what was deleted. One the user has since aimed +// somewhere else is theirs. +func TestDeleteRemoteKeepsARepointedGitRemote(t *testing.T) { + f := mirrorServer(t, "notes") + dir, _ := mirrorProject(t, f, "notes", true) + + mustGit(t, dir, "init", "-q") + mustGit(t, dir, "remote", "add", "gitea", "https://elsewhere.example/mike/notes.git") + + captureStdout(t, func() { runCommand("deleteremote @gitea") }) + + if !deleted(f, "notes") { + t.Fatalf("no DELETE sent, requests: %v", f.paths()) + } + url, err := gitCapture(dir, "remote", "get-url", "gitea") + if err != nil { + t.Fatalf("git remote gitea was removed although it pointed elsewhere: %v", err) + } + if strings.TrimSpace(url) != "https://elsewhere.example/mike/notes.git" { + t.Errorf("git remote url = %q, want it untouched", strings.TrimSpace(url)) + } +} + +// TestRemoteMatches: a target answers to its configured name and to the host of +// its url, in any case and with or without the '@'. +func TestRemoteMatches(t *testing.T) { + tgt := RemoteTarget{Name: "gitea", URL: "https://git.example.com:3000/root"} + for _, c := range []struct { + sel string + want bool + }{ + {"gitea", true}, + {"GITEA", true}, + {"@gitea", true}, + {"git.example.com", true}, + {"https://git.example.com/mike/x", true}, + {"git@git.example.com:mike/x.git", true}, + {"example.com", false}, // a suffix is not the host + {"other", false}, + {"@", false}, + {"", false}, + } { + if got := remoteMatches(tgt, c.sel); got != c.want { + t.Errorf("remoteMatches(%q) = %v, want %v", c.sel, got, c.want) + } + } +} + +// TestParseRemoteSelectors: everything on the line selects a server, '@' or not +// — the command takes nothing else that a bare word could be confused with. +func TestParseRemoteSelectors(t *testing.T) { + for _, c := range []struct { + in string + want []string + }{ + {"", nil}, + {"@gitea", []string{"gitea"}}, + {"git.example.com", []string{"git.example.com"}}, + {"@gitea git.example.com", []string{"gitea", "git.example.com"}}, + {"@", nil}, + } { + got := parseRemoteSelectors(c.in) + if strings.Join(got, ",") != strings.Join(c.want, ",") { + t.Errorf("parseRemoteSelectors(%q) = %v, want %v", c.in, got, c.want) + } + } +} diff --git a/main.go b/main.go index d7cb76f..a8baad8 100644 --- a/main.go +++ b/main.go @@ -145,7 +145,7 @@ func parseArgs() (int, string, bool) { } cls := map[string]int{ "clone": 2, "init": 2, "log": 2, - "push": 1, "pushremote": 1, "list": 1, "tag": 1, "archive": 1, "show": 1, + "push": 1, "pushremote": 1, "deleteremote": 1, "list": 1, "tag": 1, "archive": 1, "show": 1, "pull": 1, "fetch": 1, "status": 1, "diff": 1, "overview": 1, "config": 1, "count": 1, "login": 1, "cloneall": 1, "release": 1, } diff --git a/mgsh_test.go b/mgsh_test.go index 7f4ce5a..1d7d172 100644 --- a/mgsh_test.go +++ b/mgsh_test.go @@ -499,6 +499,77 @@ remotes = hub, public } } +// TestInactiveRemoteIsOnlyUsedWhenNamed: `active = false` takes a target off the +// automatic path — not used when no name is given — while naming it, by name or +// by host, still reaches it. It is not the same as removing the target, and not +// the same as an unusable one: it stays in the configuration, listed and ready. +func TestInactiveRemoteIsOnlyUsedWhenNamed(t *testing.T) { + rc := ` +remote.gitea.url = https://git.example.com +remote.gitea.key = tok +remote.gitea.active = false +remote.hub.url = https://github.com +remote.hub.key = tok2 +` + var c Config + applyConfig(&c, parseConfig(rc)) + + all, incomplete := c.mirrorTargets() + if len(all) != 2 || len(incomplete) != 0 { + t.Fatalf("mirrorTargets = %+v (incomplete %v), want both targets", all, incomplete) + } + if got := pickRemotes(all, nil); len(got) != 1 || got[0].Name != "hub" { + t.Errorf("selection without a name = %+v, want only the active hub", got) + } + for _, sel := range []string{"gitea", "@gitea", "git.example.com"} { + if got := pickRemotes(all, []string{sel}); len(got) != 1 || got[0].Name != "gitea" { + t.Errorf("pickRemotes(%q) = %+v, want the inactive gitea", sel, got) + } + } + + // MGSH_REMOTE__ACTIVE is the environment spelling + t.Setenv("MGSH_REMOTE_HUB_ACTIVE", "false") + var c2 Config + applyConfig(&c2, parseConfig(rc)) + applyEnv(&c2) + all2, _ := c2.mirrorTargets() + if got := pickRemotes(all2, nil); len(got) != 0 { + t.Errorf("MGSH_REMOTE_HUB_ACTIVE=false left %+v in the default set", got) + } +} + +// TestRemoteActiveDefaultsToOn: the setting exists to take a server *out* of the +// default set, so anything but a deliberate "off" — unset, "true", a typo — has +// to leave it in. A configuration written before the flag existed must not +// suddenly stop mirroring. +func TestRemoteActiveDefaultsToOn(t *testing.T) { + for _, v := range []string{"", "true", "yes", "1", "on", "sometimes"} { + if !(RemoteTarget{Active: v}).isActive() { + t.Errorf("active = %q switched the target off", v) + } + } + for _, v := range []string{"false", "no", "0", "off", " FALSE "} { + if (RemoteTarget{Active: v}).isActive() { + t.Errorf("active = %q did not switch the target off", v) + } + } +} + +// TestPushRemoteWithOnlyInactiveTargets: the message has to name the reason. +// "needs a remote..url" would send the user looking for a setting that is +// sitting right there in the file. +func TestPushRemoteWithOnlyInactiveTargets(t *testing.T) { + dir := useProject(t, "notes") + mustGit(t, dir, "init", "-q") + cfg.Remotes = []RemoteTarget{{Name: "gitea", URL: "https://git.example.com", Key: "tok", Active: "false"}} + + out := captureStdout(t, func() { runCommand("pushremote") }) + + if !strings.Contains(out, "inactive") || !strings.Contains(out, "@gitea") { + t.Errorf("output = %q, want it to report the inactive target and how to name it", out) + } +} + func TestParsePushRemoteArgs(t *testing.T) { cases := []struct { in string diff --git a/mgshrc.example b/mgshrc.example index e09a6c3..e3b1fdb 100644 --- a/mgshrc.example +++ b/mgshrc.example @@ -26,8 +26,13 @@ gitpath = /home/git # and visibility. is yours to pick and becomes the git remote created in # the repository, so `git push gitlab` keeps working outside mgsh. # -# `pushremote` pushes to every configured server in the order given, -# `pushremote @gitlab` to a single one. +# `pushremote` pushes to every active server in the order given, +# `pushremote @gitlab` to a single one (active or not). +# +# `deleteremote @gitlab` (or `deleteremote gitlab.example.com`) deletes the +# project's repository there again, after asking. It always needs the server +# named, and the token needs permission to delete: delete_repo on GitHub, +# write:repository on Gitea, api + Owner on GitLab. # # remote.gitea.url = https://git.example.com # remote.gitea.key = @@ -38,6 +43,8 @@ gitpath = /home/git # remote.gitlab.key = # remote.gitlab.type = gitlab # remote.gitlab.visibility = public +# remote.gitlab.active = false # default true; false = only used when +# # named, e.g. `pushremote @gitlab` # # remotes = gitea, gitlab # optional: restrict and order the set # mirror = true # `push` also mirrors via pushremote diff --git a/release.go b/release.go index 3d7aded..d60ba3b 100644 --- a/release.go +++ b/release.go @@ -169,15 +169,13 @@ func handleRelease(args string) { return } - targets, incomplete := cfg.mirrorTargets() + configured, incomplete := cfg.mirrorTargets() for _, n := range incomplete { errorln("remote " + n + ": url or key missing — skipped") } - targets = pickRemotes(targets, names) + targets := pickRemotes(configured, names) if len(targets) == 0 { - if len(names) == 0 { // an unknown @name already reported itself - errorln("release needs a mirror target — see 'config'") - } + reportNoTargets("release", configured, names) return } diff --git a/remote.go b/remote.go index 5ace454..02cf5d4 100644 --- a/remote.go +++ b/remote.go @@ -238,18 +238,64 @@ func parsePushRemoteArgs(args string) (names []string, description string) { return names, strings.Join(fields[i:], " ") } +// remoteHost reduces a configured url — or whatever a user typed in its place — +// to its bare host: scheme, credentials, port and path removed. +func remoteHost(s string) string { + s = strings.TrimSpace(s) + if i := strings.Index(s, "://"); i >= 0 { + s = s[i+3:] + } + if i := strings.IndexByte(s, '@'); i >= 0 { // user[:pass]@host + s = s[i+1:] + } + if i := strings.IndexAny(s, "/:"); i >= 0 { + s = s[:i] + } + return s +} + +// remoteMatches reports whether sel picks target t. A target answers both to its +// configured name and to the host of its url: the name is what the config calls +// the server, the host is what the user sees in the browser, and `@gitea` should +// not be the only way to say `git.example.com`. +func remoteMatches(t RemoteTarget, sel string) bool { + sel = strings.TrimPrefix(strings.TrimSpace(sel), "@") + if sel == "" { + return false + } + if strings.EqualFold(t.Name, sel) { + return true + } + h := remoteHost(t.URL) + return h != "" && strings.EqualFold(h, remoteHost(sel)) +} + +// activeRemotes keeps the targets that take part when a command was given no +// target of its own (see RemoteTarget.isActive). +func activeRemotes(all []RemoteTarget) []RemoteTarget { + out := make([]RemoteTarget, 0, len(all)) + for _, t := range all { + if t.isActive() { + out = append(out, t) + } + } + return out +} + // pickRemotes narrows all to the explicitly requested names, complaining about -// any that are not configured. With no names given, all targets are used. +// any that are not configured. With no names given the active targets are used +// — `active = false` takes a server off that automatic path, and naming it is +// what puts it back on. func pickRemotes(all []RemoteTarget, names []string) []RemoteTarget { if len(names) == 0 { - return all + return activeRemotes(all) } var out []RemoteTarget seen := map[string]bool{} for _, n := range names { found := false for _, t := range all { - if !strings.EqualFold(t.Name, n) { + if !remoteMatches(t, n) { continue } if !seen[t.Name] { // `@hub @hub` must not push twice @@ -266,8 +312,24 @@ func pickRemotes(all []RemoteTarget, names []string) []RemoteTarget { return out } +// reportNoTargets explains an empty selection. There are three ways to end up +// with nothing to push to, and they need different answers: nothing configured +// at all, everything configured switched to `active = false`, or a name nobody +// recognised — which pickRemotes has already complained about by itself. +func reportNoTargets(cmd string, configured []RemoteTarget, names []string) { + switch { + case len(names) > 0: + return + case len(configured) > 0: + errorln("every configured remote is inactive — name one, e.g. '" + + cmd + " @" + configured[0].Name + "'") + default: + errorln(cmd + " needs a 'remote..url' and 'remote..key' in " + configFile()) + } +} + // handlePushRemote implements `pushremote [@name ...] [description]`. Without -// a @name it mirrors to every configured target; the description, if given, is +// a @name it mirrors to every active target; the description, if given, is // set on the repository when it is created. func handlePushRemote(args string) { if !requireRepo() { @@ -275,15 +337,13 @@ func handlePushRemote(args string) { } names, description := parsePushRemoteArgs(args) - targets, incomplete := cfg.mirrorTargets() + configured, incomplete := cfg.mirrorTargets() for _, n := range incomplete { errorln("remote " + n + ": url or key missing — skipped") } - targets = pickRemotes(targets, names) + targets := pickRemotes(configured, names) if len(targets) == 0 { - if len(names) == 0 { // an unknown @name already reported itself - errorln("pushremote needs a 'remote..url' and 'remote..key' in " + configFile()) - } + reportNoTargets("pushremote", configured, names) return } diff --git a/show_config.go b/show_config.go index bbcd350..6db9bb3 100644 --- a/show_config.go +++ b/show_config.go @@ -104,9 +104,15 @@ func showRemotes() { if kind == "" { kind = remoteKindName(detectRemoteKind(t.URL, "")) + " (detected)" } - fmt.Printf(" %s%s %s\n", - col(cGreen, padRight("@"+t.Name, 14)), t.URL, - col(cDark, kind+", "+vis+", key "+maskSecret(t.Key))) + // an inactive target is still a target, just not an automatic one: it + // steps back a shade instead of dropping out of the listing + name, state := col(cGreen, padRight("@"+t.Name, 14)), "" + if !t.isActive() { + name = col(cGrey, padRight("@"+t.Name, 14)) + state = ", inactive (only when named)" + } + fmt.Printf(" %s%s %s\n", name, t.URL, + col(cDark, kind+", "+vis+", key "+maskSecret(t.Key)+state)) } for _, n := range incomplete { fmt.Printf(" %s%s\n", col(cRed, padRight("@"+n, 14)), col(cRed, "incomplete: url or key missing")) diff --git a/version.txt b/version.txt index 8d9df19..26089d1 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -4.0.62 +4.0.64