diff --git a/USAGE.md b/USAGE.md index 11c0218..b65d5af 100644 --- a/USAGE.md +++ b/USAGE.md @@ -22,7 +22,8 @@ vallic login --token vallic whoami vallic team -# Record which project this directory is, outside a clone of its repository +# Record a project for directories that are not a clone of its repository. +# One for the whole machine: a clone's own remote still answers first. vallic link acme-site # What this checkout points at: project, environment, branch, last deploy diff --git a/internal/api/models.go b/internal/api/models.go index 84600b2..c9aa9b3 100644 --- a/internal/api/models.go +++ b/internal/api/models.go @@ -107,6 +107,18 @@ type EnvironmentDetail struct { // Hostnames is what the environment answers on. Nil only against a // control plane that predates the member. Hostnames *Hostnames `json:"hostnames"` + + // Servers are the machines the environment runs on, by the IDs `server + // list` prints, limited to those the caller may see. Not nil where SSH + // is: staff outside the team get no shell target and still need the + // machine. Nil only against a control plane that predates the member. + Servers []EnvironmentServer `json:"servers"` +} + +// EnvironmentServer is one placement of an environment on a machine. +type EnvironmentServer struct { + ID int `json:"id"` + Role string `json:"role"` } // Hostnames is where an environment can be reached. @@ -372,6 +384,12 @@ type Deployment struct { // SkipSteps says it goes live without the application's deploy steps. SkipSteps bool `json:"skip_steps"` + + // Task is the work doing it, whose log is the deploy's. Not the + // deployment's own id: the two are numbered apart, and following the + // deployment's id as a task's found another task or none. Nil against a + // control plane that predates the member, and where nothing was queued. + Task *int `json:"task"` } // Describe says what happened, in the words the trigger calls for. diff --git a/internal/api/models_test.go b/internal/api/models_test.go index dc7a815..cbfad96 100644 --- a/internal/api/models_test.go +++ b/internal/api/models_test.go @@ -357,3 +357,19 @@ func TestARetryAfterSurvivesAResponseWithNoEnvelope(t *testing.T) { t.Errorf("RetryAfter = %v, want 7s", apiErr.RetryAfter) } } + +// `--format json` re-encodes these structs, so a member the struct lacks is a +// member the CLI's JSON drops. +func TestEnvironmentServersSurviveReencoding(t *testing.T) { + var detail EnvironmentDetail + if err := json.Unmarshal([]byte(`{"id":9,"ssh":null,"servers":[{"id":42,"role":"web"},{"id":43,"role":"db"}]}`), &detail); err != nil { + t.Fatal(err) + } + out, err := json.Marshal(detail) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(out), `"servers":[{"id":42,"role":"web"},{"id":43,"role":"db"}]`) { + t.Fatalf("servers lost: %s", out) + } +} diff --git a/internal/cli/build.go b/internal/cli/build.go index 736aa4d..2233745 100644 --- a/internal/cli/build.go +++ b/internal/cli/build.go @@ -79,7 +79,7 @@ set. The output says which it is going to be.`, // after. `build --deploy` skipped it until [2026-09-30], which // made the flag a way round a question the other path asks. if asksBeforeShipping(deploy, detail.Protected, env.Interactive()) { - if !env.confirm(fmt.Sprintf("Build and deploy to %s, which is protected?", detail.Name)) { + if !env.confirm(fmt.Sprintf("Build and deploy to %s, which is protected?", target.Named())) { return fmt.Errorf("cancelled") } } diff --git a/internal/cli/deploy.go b/internal/cli/deploy.go index 7c0ab88..12ad4ab 100644 --- a/internal/cli/deploy.go +++ b/internal/cli/deploy.go @@ -74,7 +74,7 @@ the whole of the check.`, // environment, not operating one — but a `vallic deploy` run // from the wrong checkout should be recoverable before it // happens rather than after. - if !env.confirm(fmt.Sprintf("Deploy to %s, which is protected?", detail.Name)) { + if !env.confirm(fmt.Sprintf("Deploy to %s, which is protected?", target.Named())) { return fmt.Errorf("cancelled") } } @@ -160,9 +160,20 @@ func (e *Env) waitForDeployment(ctx context.Context, deployment *api.Deployment, ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - e.Printer.Say("Waiting for deployment %d…", deployment.ID) + // The task the control plane says is doing it, and never the deployment's + // own id: the two are numbered apart, and following that one found + // another task's log, or none. A control plane that does not say cannot + // be followed from here, and a wait that guessed would report another + // task's ending as this deploy's. + if deployment.Task == nil { + return fmt.Errorf("deployment %d is queued, but this control plane does not say which task runs it; follow it with `vallic activity list`", deployment.ID) + } + + id := *deployment.Task + + e.Printer.Say("Waiting for deployment %d (task %d)…", deployment.ID, id) - state, err := e.follow(ctx, client, deployment.ID, e.Err) + state, err := e.follow(ctx, client, id, e.Err) if err != nil { return err } @@ -171,7 +182,7 @@ func (e *Env) waitForDeployment(ctx context.Context, deployment *api.Deployment, // log route carries the state so the loop above needs one request per // tick; it deliberately does not carry the rest, because that is the // task's contract and not the log's. - task, err := client.Task(ctx, deployment.ID) + task, err := client.Task(ctx, id) if err != nil { // The work finished and this is only the epilogue failing. Reporting // the state we already saw beats turning a successful deploy into an diff --git a/internal/cli/deploy_test.go b/internal/cli/deploy_test.go index 15e2a9e..96466fe 100644 --- a/internal/cli/deploy_test.go +++ b/internal/cli/deploy_test.go @@ -172,6 +172,67 @@ func TestAFollowRidesOutARateLimitInsteadOfAbandoningTheDeploy(t *testing.T) { } } +// A wait follows the task the control plane names, not the deployment's id. +// +// The two are numbered apart. A wait that asked for the deployment's id as a +// task's was told there was no such task — or, had one existed, would have +// reported its ending as this deploy's. +func TestAWaitFollowsTheDeploymentsTask(t *testing.T) { + var elsewhere int64 + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + + switch { + case strings.HasSuffix(r.URL.Path, "/tasks/40/log"): + _, _ = w.Write([]byte(`{"log":{"offset":0,"content":"live\n","next_offset":5,` + + `"size":5,"truncated":false,"state":"succeeded","complete":true}}`)) + case strings.HasSuffix(r.URL.Path, "/tasks/40"): + _, _ = w.Write([]byte(`{"task":{"id":40,"type":"deploy","state":"succeeded"}}`)) + default: + atomic.AddInt64(&elsewhere, 1) + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":{"code":"not_found","message":"No such task."}}`)) + } + })) + defer server.Close() + + env, _, errOut := followEnv(t) + env.client = api.New(server.URL, auth.Anonymous{}, "test") + + task := 40 + if err := env.waitForDeployment(context.Background(), &api.Deployment{ID: 12, Task: &task}, 10*time.Second); err != nil { + t.Fatalf("waitForDeployment: %v", err) + } + + if n := atomic.LoadInt64(&elsewhere); n != 0 { + t.Errorf("%d request(s) went to a task other than 40", n) + } + + if !strings.Contains(errOut.String(), "task 40") { + t.Errorf("stderr = %q, want it to say which task it follows", errOut.String()) + } +} + +// A control plane that does not name the task is not guessed at. +// +// Following the deployment's id instead is the defect: it reports another +// task's ending, or none, as this deploy's. +func TestAWaitWithoutATaskIsRefused(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + t.Error("a wait with no task asked the control plane for one anyway") + })) + defer server.Close() + + env, _, _ := followEnv(t) + env.client = api.New(server.URL, auth.Anonymous{}, "test") + + err := env.waitForDeployment(context.Background(), &api.Deployment{ID: 12}, 10*time.Second) + if err == nil || !strings.Contains(err.Error(), "vallic activity list") { + t.Fatalf("err = %v, want a refusal that says where to look", err) + } +} + // followEnv is an Env that writes where a test can read it. func followEnv(t *testing.T) (*Env, *bytes.Buffer, *bytes.Buffer) { t.Helper() diff --git a/internal/cli/init.go b/internal/cli/init.go index dc79e2f..28567f4 100644 --- a/internal/cli/init.go +++ b/internal/cli/init.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "path/filepath" + "regexp" "strings" ) @@ -25,6 +26,8 @@ const ( typePHP = "php" typeNode = "nodejs" typeGo = "go" + typePython = "python" + typeDjango = "django" ) // initCommand writes a starting vallic.yaml. @@ -49,7 +52,7 @@ func initCommand() *Command { return &Command{ Name: "init", Summary: "write a starting vallic.yaml for this checkout", - Usage: "init [--type drupal|laravel|wordpress|php|nodejs|go] [--force]", + Usage: "init [--type drupal|laravel|wordpress|php|nodejs|go|python|django] [--force]", Long: `Writes vallic.yaml in the working directory, and nothing else. It needs no credential and reaches nothing, so it works in a checkout of a project that does not exist on the platform yet. @@ -57,8 +60,10 @@ project that does not exist on the platform yet. What kind of project this is comes from the files here: a composer.json requiring drupal/core is Drupal, laravel/framework is Laravel, a WordPress core package or a wp-config.php is WordPress, a composer.json on its own is -PHP, go.mod is Go, package.json is Node. The command says which file decided -it, and --type overrides the answer. +PHP, go.mod is Go, manage.py or a dependency on Django is Django, a uv.lock, +pyproject.toml or requirements.txt is otherwise Python, and package.json is +Node. The command says which file decided it, and --type overrides the +answer. The kind is worth getting right rather than settling for the generic one: it decides the cron command the platform runs, which directories survive a @@ -235,26 +240,64 @@ func detectProject(dir string) (kind, because string, err error) { return typeGo, "go.mod", nil } + // Before package.json, for the reason composer.json is first: a Django + // site builds its front end with npm and is not a Node project. + if kind, because := detectPython(dir); kind != "" { + return kind, because, nil + } + if _, readErr := os.Stat(filepath.Join(dir, "package.json")); readErr == nil { return typeNode, "package.json", nil } // A usage error rather than a failure: the fix is an argument, and the - // help printed with it lists the four that work. + // help printed with it lists the kinds that work. return "", "", Usagef(nil, - "nothing here says what this project is: no composer.json, wp-config.php, go.mod or package.json\n"+ - " name it: vallic init --type drupal|laravel|wordpress|php|nodejs|go", + "nothing here says what this project is: no composer.json, wp-config.php, go.mod, pyproject.toml, requirements.txt or package.json\n"+ + " name it: vallic init --type drupal|laravel|wordpress|php|nodejs|go|python|django", ) } +// The files a Python project declares its dependencies in, the most exact +// first: a lock names every package, a requirements file may only include +// another. +var pythonFiles = []string{"uv.lock", "pyproject.toml", "requirements.txt"} + +// detectPython recognises a Python project, and Django among them. +// +// Django by manage.py, which `django-admin startproject` writes at the root, +// or by a dependency on it where the project keeps manage.py elsewhere. The +// difference is worth the look: Django is the kind whose uploads survive a +// deploy and whose sessions are cleared, and a Django site written as plain +// Python keeps neither. +func detectPython(dir string) (kind, because string) { + if _, err := os.Stat(filepath.Join(dir, "manage.py")); err == nil { + return typeDjango, "manage.py" + } + + for _, file := range pythonFiles { + if raw, err := os.ReadFile(filepath.Join(dir, file)); err == nil && pythonRequires(raw, "django") { + return typeDjango, file + ", which requires Django" + } + } + + for _, file := range pythonFiles { + if _, err := os.Stat(filepath.Join(dir, file)); err == nil { + return typePython, file + } + } + + return "", "" +} + // knownProjectType refuses a --type this writes no file for. func knownProjectType(kind string) error { switch kind { - case typeDrupal, typeLaravel, typeWordPress, typePHP, typeNode, typeGo: + case typeDrupal, typeLaravel, typeWordPress, typePHP, typeNode, typeGo, typePython, typeDjango: return nil } - return Usagef(nil, "--type takes drupal, laravel, wordpress, php, nodejs or go, not %q", kind) + return Usagef(nil, "--type takes drupal, laravel, wordpress, php, nodejs, go, python or django, not %q", kind) } // starterFor builds the starting file for one kind of project. @@ -264,6 +307,8 @@ func starterFor(kind, dir string) *starter { return nodeStarter(dir) case typeGo: return goStarter(dir) + case typePython, typeDjango: + return pythonStarter(kind, dir) default: return phpStarter(kind, dir) } @@ -396,6 +441,214 @@ func goStarter(dir string) *starter { return manifest } +// pythonStarter is a Python application, Django or not, which serves its own +// HTTP through Gunicorn. +// +// Everything is run as `python -m …` or `python manage.py …`. The virtual +// environment is built in the checkout and moved into the release, so +// `python` is always the release's own, while a script such as +// .venv/bin/gunicorn still names the directory it was built in. +func pythonStarter(kind, dir string) *starter { + manifest := &starter{ + Type: kind, + VersionKey: "python", + Version: pythonVersion(dir), + Services: []string{"postgres: '18'"}, + } + + exists := func(name string) bool { + _, err := os.Stat(filepath.Join(dir, name)) + + return err == nil + } + + // Whatever the repository locks with. uv with its lock, so a build + // installs exactly what was tested; pip where requirements.txt is what + // the project keeps; uv without a lock, resolving afresh, for a + // pyproject.toml alone. + switch { + case exists("uv.lock"): + manifest.Build = []string{"uv sync --locked --no-dev --no-editable"} + case exists("requirements.txt"): + manifest.Build = []string{"python -m venv .venv", ".venv/bin/pip install -r requirements.txt"} + case exists("pyproject.toml"): + manifest.Build = []string{"uv sync --no-dev --no-editable"} + manifest.Notes = append(manifest.Notes, + "no uv.lock here, so every build resolves the dependencies again: run uv lock, commit it, and add --locked") + default: + manifest.Notes = append(manifest.Notes, + "no uv.lock, pyproject.toml or requirements.txt here, so no build step is written") + } + + if manifest.Version == "" { + manifest.Notes = append(manifest.Notes, + "nothing here states a Python version — .python-version, or requires-python in pyproject.toml — so none is pinned and the platform's default runs") + } + + if kind != typeDjango { + // Gunicorn reads gunicorn.conf.py where it starts, and the image + // starts it with nothing else, so a project with one needs no line. + // Otherwise the application's module and variable are its own, and a + // guess would name one that is not there. + if !exists("gunicorn.conf.py") { + manifest.Notes = append(manifest.Notes, + "add a start: line naming what serves the application, such as python -m gunicorn app:app") + } + + return manifest + } + + // Collected in the build, which can write, and served by the application + // from the release. The step needs STATIC_ROOT, which only the settings + // can say. + manifest.Build = append(manifest.Build, "python manage.py collectstatic --noinput") + manifest.Deploy = []string{"'python manage.py migrate --noinput'"} + + project, settings := djangoProject(dir) + if project == "" { + manifest.Notes = append(manifest.Notes, + "no project package with a wsgi.py found, so add a start: line naming the WSGI application") + } else { + manifest.Start = "python -m gunicorn " + project + ".wsgi" + } + + if raw, err := os.ReadFile(settings); err == nil && !strings.Contains(string(raw), "STATIC_ROOT") { + name, _ := filepath.Rel(dir, settings) + manifest.Notes = append(manifest.Notes, + "the build runs collectstatic, which needs STATIC_ROOT in "+filepath.ToSlash(name)) + } + + return manifest +} + +// The settings module manage.py names, as startproject writes it. +var djangoSettingsModule = regexp.MustCompile(`DJANGO_SETTINGS_MODULE['"]\s*,\s*['"]([A-Za-z_][\w.]*)['"]`) + +// djangoProject finds the package holding the site's wsgi.py and settings. +// +// From manage.py where it says, which is the one answer the project itself +// gives. Otherwise the one directory here holding both files; with none, or +// with several, the answer is the project's to write rather than this +// command's to pick. +func djangoProject(dir string) (project, settings string) { + if raw, err := os.ReadFile(filepath.Join(dir, "manage.py")); err == nil { + if match := djangoSettingsModule.FindSubmatch(raw); match != nil { + module := string(match[1]) + project, _, _ = strings.Cut(module, ".") + settings = filepath.Join(dir, filepath.FromSlash(strings.ReplaceAll(module, ".", "/"))+".py") + + if _, err := os.Stat(filepath.Join(dir, project, "wsgi.py")); err == nil { + return project, settings + } + + return "", settings + } + } + + entries, err := os.ReadDir(dir) + if err != nil { + return "", "" + } + + for _, entry := range entries { + if !entry.IsDir() { + continue + } + + wsgi := filepath.Join(dir, entry.Name(), "wsgi.py") + candidate := filepath.Join(dir, entry.Name(), "settings.py") + + if _, err := os.Stat(wsgi); err != nil { + continue + } + + if _, err := os.Stat(candidate); err != nil { + continue + } + + if project != "" { + return "", "" + } + + project, settings = entry.Name(), candidate + } + + return project, settings +} + +// pythonVersion is the Python the repository says it runs on. +// +// .python-version first, which uv and pyenv write and which names the +// interpreter actually in use; then requires-python, which is a floor rather +// than a choice but is the version the project has promised to work on. Two +// components, because the images are tagged `3.14.8-slim-trixie-1` and the +// catalogue matches a manifest's `3.14` on the version line. +func pythonVersion(dir string) string { + if raw, err := os.ReadFile(filepath.Join(dir, ".python-version")); err == nil { + line, _, _ := strings.Cut(strings.TrimSpace(string(raw)), "\n") + + if version := firstVersion(line, 2); strings.Contains(version, ".") { + return version + } + } + + raw, err := os.ReadFile(filepath.Join(dir, "pyproject.toml")) + if err != nil { + return "" + } + + for _, line := range strings.Split(string(raw), "\n") { + key, value, found := strings.Cut(line, "=") + + if found && strings.TrimSpace(key) == "requires-python" { + return firstVersion(value, 2) + } + } + + return "" +} + +// pythonRequires reports whether a dependency file asks for a package. +// +// Read the way all three spell a requirement: a requirements.txt line, a +// quoted entry in pyproject.toml's dependencies, uv.lock's `name = "…"`. Each +// starts with the package's name, which Python compares ignoring case and +// treating `-`, `_` and `.` as one character. +func pythonRequires(raw []byte, name string) bool { + for _, line := range strings.Split(string(raw), "\n") { + line, _, _ = strings.Cut(line, "#") + candidates := []string{strings.TrimSpace(line)} + + for _, quote := range []string{`"`, "'"} { + parts := strings.Split(line, quote) + + for i := 1; i < len(parts); i += 2 { + candidates = append(candidates, parts[i]) + } + } + + for _, candidate := range candidates { + if pythonPackageName(candidate) == name { + return true + } + } + } + + return false +} + +// pythonPackageName is the normalised name a requirement starts with. +func pythonPackageName(requirement string) string { + end := strings.IndexFunc(requirement, func(r rune) bool { + return !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9' || r == '-' || r == '_' || r == '.') + }) + if end < 0 { + end = len(requirement) + } + + return strings.ToLower(strings.NewReplacer("_", "-", ".", "-").Replace(requirement[:end])) +} + // noteUnpinned records that no version was pinned, and which file would have // said one. // diff --git a/internal/cli/init_test.go b/internal/cli/init_test.go index 931aa32..2c41197 100644 --- a/internal/cli/init_test.go +++ b/internal/cli/init_test.go @@ -15,7 +15,13 @@ func checkoutWith(t *testing.T, files map[string]string) string { dir := t.TempDir() for name, contents := range files { - if err := os.WriteFile(filepath.Join(dir, name), []byte(contents), 0o644); err != nil { + path := filepath.Join(dir, filepath.FromSlash(name)) + + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(path, []byte(contents), 0o644); err != nil { t.Fatal(err) } } @@ -70,6 +76,42 @@ func TestDetectProjectReadsTheFilesInTheCheckout(t *testing.T) { files: map[string]string{"package.json": `{}`}, want: typeNode, }, + { + name: "a django site with a front end to build", + files: map[string]string{ + "manage.py": "import os\n", + "package.json": `{"scripts":{"build":"vite build"}}`, + }, + want: typeDjango, + }, + { + name: "django by its requirement, with manage.py elsewhere", + files: map[string]string{"requirements.txt": "Django[argon2]>=5.2,<6\npsycopg[binary]\n"}, + want: typeDjango, + }, + { + name: "django by pyproject.toml's dependencies", + files: map[string]string{"pyproject.toml": "[project]\nname = \"shop\"\ndependencies = [\n \"django>=5.2\",\n]\n"}, + want: typeDjango, + }, + { + name: "python, whose packages only sound like django", + files: map[string]string{"pyproject.toml": "[project]\ndependencies = [\"flask>=3\", \"django-environ\"]\n# not django\n"}, + want: typePython, + }, + { + name: "python by requirements.txt", + files: map[string]string{"requirements.txt": "fastapi\nuvicorn\n"}, + want: typePython, + }, + { + name: "a go service with its documentation built in python", + files: map[string]string{ + "go.mod": "module example.com/api\n\ngo 1.27\n", + "requirements.txt": "mkdocs\n", + }, + want: typeGo, + }, } for _, c := range cases { @@ -146,6 +188,31 @@ func TestStarterPinsOnlyWhatTheRepositoryStates(t *testing.T) { key: "go", want: "1.27", }, + { + name: "python, from .python-version before requires-python", + kind: typePython, + files: map[string]string{ + ".python-version": "3.13.2\n", + "pyproject.toml": "[project]\nrequires-python = \">=3.12\"\n", + }, + key: "python", + want: "3.13", + }, + { + name: "python, from the floor requires-python sets", + kind: typeDjango, + files: map[string]string{"pyproject.toml": "[project]\nrequires-python = \">=3.12,<4\"\n"}, + key: "python", + want: "3.12", + }, + { + name: "python with nothing to say, which is said out loud", + kind: typePython, + files: map[string]string{"requirements.txt": "flask\n"}, + key: "python", + want: "", + wantsNote: true, + }, { name: "nothing at all, which is said out loud", kind: typePHP, @@ -214,12 +281,17 @@ var manifestKeys = map[string]bool{ } func TestStarterUsesOnlyKeysTheControlPlaneUnderstands(t *testing.T) { - for _, kind := range []string{typeDrupal, typePHP, typeNode, typeGo} { + for _, kind := range []string{typeDrupal, typePHP, typeNode, typeGo, typePython, typeDjango} { t.Run(kind, func(t *testing.T) { rendered := renderStarter(starterFor(kind, checkoutWith(t, map[string]string{ - "composer.json": `{"require":{"php":"^8.4","drupal/core":"^11"}}`, - "package.json": `{"engines":{"node":"24"},"scripts":{"build":"next build","start":"next start"}}`, - "go.mod": "module example.com/api\n\ngo 1.27\n", + "composer.json": `{"require":{"php":"^8.4","drupal/core":"^11"}}`, + "package.json": `{"engines":{"node":"24"},"scripts":{"build":"next build","start":"next start"}}`, + "go.mod": "module example.com/api\n\ngo 1.27\n", + "pyproject.toml": "[project]\nrequires-python = \">=3.14\"\n", + "uv.lock": "version = 1\n", + "manage.py": `os.environ.setdefault("DJANGO_SETTINGS_MODULE", "mysite.settings")`, + "mysite/wsgi.py": "", + "mysite/settings.py": "STATIC_ROOT = BASE_DIR / 'staticfiles'\n", }))) for _, line := range strings.Split(rendered, "\n") { @@ -449,3 +521,107 @@ func TestDeployStepsMatchWhatEachFrameworkNeeds(t *testing.T) { t.Errorf("laravel Deploy = %v, want --force: a prompt nobody can answer hangs the deploy", laravel) } } + +// The build is whatever the repository locks with. uv with its lock installs +// exactly what was tested; requirements.txt is pip's file, and uv given it +// would want a pyproject.toml the project does not have; a pyproject.toml +// with no lock still builds, but resolves afresh every time, which is worth +// saying. +func TestPythonBuildsWithWhatTheRepositoryLocks(t *testing.T) { + cases := []struct { + name string + files map[string]string + want []string + wantsNote string + }{ + { + name: "uv, held to its lock", + files: map[string]string{"pyproject.toml": "[project]\n", "uv.lock": "version = 1\n", "requirements.txt": "flask\n"}, + want: []string{"uv sync --locked --no-dev --no-editable"}, + }, + { + name: "pip, into the release's own environment", + files: map[string]string{"requirements.txt": "flask\n"}, + want: []string{"python -m venv .venv", ".venv/bin/pip install -r requirements.txt"}, + }, + { + name: "uv with nothing locked", + files: map[string]string{"pyproject.toml": "[project]\n"}, + want: []string{"uv sync --no-dev --no-editable"}, + wantsNote: "uv lock", + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := starterFor(typePython, checkoutWith(t, c.files)) + + if strings.Join(got.Build, " | ") != strings.Join(c.want, " | ") { + t.Errorf("Build = %q, want %q", got.Build, c.want) + } + + if c.wantsNote != "" && !strings.Contains(strings.Join(got.Notes, "\n"), c.wantsNote) { + t.Errorf("Notes = %q, want one mentioning %q", got.Notes, c.wantsNote) + } + + // No start line for a project that is not Django: the module and + // the variable are the application's own, and a guess names one + // that is not there. Said instead. + if got.Start != "" { + t.Errorf("Start = %q, want none written for a plain Python project", got.Start) + } + + if !strings.Contains(strings.Join(got.Notes, "\n"), "start:") { + t.Errorf("Notes = %q, want one asking for a start: line", got.Notes) + } + }) + } +} + +// Django's starter does what the framework needs and nothing it cannot know: +// static files collected in the build, migrate on every deploy — the first +// one too, which is what creates the tables — and Gunicorn on the WSGI module +// manage.py names. +func TestDjangoStarterServesTheProjectManagePyNames(t *testing.T) { + dir := checkoutWith(t, map[string]string{ + "manage.py": "os.environ.setdefault('DJANGO_SETTINGS_MODULE', 'config.settings.production')\n", + "config/wsgi.py": "", + "config/settings/production.py": "DEBUG = False\n", + "requirements.txt": "Django>=5.2\n", + }) + + got := starterFor(typeDjango, dir) + + if got.Start != "python -m gunicorn config.wsgi" { + t.Errorf("Start = %q, want Gunicorn on config.wsgi", got.Start) + } + + if last := got.Build[len(got.Build)-1]; last != "python manage.py collectstatic --noinput" { + t.Errorf("last build step = %q, want collectstatic after the dependencies", last) + } + + if len(got.Deploy) != 1 || !strings.Contains(got.Deploy[0], "manage.py migrate --noinput") { + t.Errorf("Deploy = %v, want migrate, with no prompt to hang on", got.Deploy) + } + + // The settings have no STATIC_ROOT, so the step the file writes would + // fail the first build. Said, with the file that needs it. + if !strings.Contains(strings.Join(got.Notes, "\n"), "STATIC_ROOT in config/settings/production.py") { + t.Errorf("Notes = %q, want one naming the settings file STATIC_ROOT is missing from", got.Notes) + } + + // With no manage.py, the one directory holding wsgi.py and settings.py + // is the project; with two, the answer is the project's to give. + found, _ := djangoProject(checkoutWith(t, map[string]string{"shop/wsgi.py": "", "shop/settings.py": ""})) + if found != "shop" { + t.Errorf("djangoProject() = %q, want the one package holding both files", found) + } + + ambiguous, _ := djangoProject(checkoutWith(t, map[string]string{ + "shop/wsgi.py": "", "shop/settings.py": "", + "blog/wsgi.py": "", "blog/settings.py": "", + })) + if ambiguous != "" { + t.Errorf("djangoProject() = %q from two candidates, want none picked", ambiguous) + } +} diff --git a/internal/cli/link.go b/internal/cli/link.go index 0079079..e8167d2 100644 --- a/internal/cli/link.go +++ b/internal/cli/link.go @@ -21,10 +21,10 @@ import ( // // What it records is not a property of the checkout, and that is the whole // caveat. It goes in the configuration this machine shares between every -// directory, and the resolver reads it *before* the git remote, so a -// recorded project answers for the checkouts that were answering correctly -// too. Every message below says so rather than leaving it to be discovered -// when a deploy names the wrong site. +// directory, so the resolver reads it only *after* the git remote: a +// checkout that names its own project goes on resolving to it. Read first, +// a project linked once answered for every checkout on the machine, and a +// command run in one project's checkout acted on another project. func linkCommand() *Command { var forget bool @@ -43,9 +43,9 @@ added under another name, or a directory that is not a checkout at all. With no argument it says what is recorded now, or what commands resolve to here without it. -What it records is machine-wide, not per checkout: it is read before the git -remote, so every other checkout on this machine resolves to it as well. ---clear puts that back. +What it records is machine-wide, not per checkout, so it answers only where +a checkout cannot: a checkout whose git remote names a project you can see +goes on resolving to that project. --clear forgets it. It records no environment, and that is deliberate. The environment comes from the branch, which changes while you work. An environment recorded once would @@ -110,7 +110,7 @@ VALLIC_ENVIRONMENT.`, env.Printer.Good("Commands act on %s where a checkout cannot say.", chosen.MachineName) if dir, dirErr := config.Dir(); dirErr == nil { - env.Printer.Say(" recorded in %s, for every checkout on this machine", dir) + env.Printer.Say(" recorded in %s, for the checkouts on this machine that cannot say", dir) } warnOverridden(ctx, env, projects, chosen) @@ -142,40 +142,46 @@ func reportLink(ctx context.Context, env *Env, cfg *config.Config, projects []ap }) } - if recorded == "" { - if inferred == nil { - if len(projects) == 0 { - return fmt.Errorf("this credential can see no projects, so there is nothing to link to") - } + // VALLIC_PROJECT, which every command reads before the checkout and the + // file. Worth separating, because --clear removes the file's and not it. + if fromEnv := os.Getenv("VALLIC_PROJECT"); fromEnv != "" { + env.Printer.Line("%s", fromEnv) + env.Printer.Say(" from VALLIC_PROJECT, which every command reads first") - // The state this command exists for. Named as usage, because the - // fix is an argument and the candidates are the useful half of - // the answer. - return Usagef(nil, - "nothing is recorded, and this checkout does not say which project it is\n"+ - " record one: vallic link \n it can be: %s", - strings.Join(projectNames(projects), ", "), - ) - } + return nil + } + // The checkout, which answers ahead of anything recorded. + if inferred != nil { env.Printer.Line("%s", inferred.MachineName) - env.Printer.Say(" worked out here; nothing is recorded, and nothing needs to be") + env.Printer.Say(" worked out from this checkout") + + if recorded != "" && recorded != inferred.MachineName { + env.Printer.Say(" %s is recorded too, and answers only where a checkout cannot say", recorded) + } return nil } - env.Printer.Line("%s", recorded) + if recorded == "" { + if len(projects) == 0 { + return fmt.Errorf("this credential can see no projects, so there is nothing to link to") + } - if fromEnv := os.Getenv("VALLIC_PROJECT"); fromEnv == recorded { - // Not the file, whatever the file says. Worth separating, because - // --clear removes one of these and not the other. - env.Printer.Say(" from VALLIC_PROJECT, which is read before the configuration file") - } else if dir, err := config.Dir(); err == nil { - env.Printer.Say(" recorded in %s, for every checkout on this machine", dir) + // The state this command exists for. Named as usage, because the + // fix is an argument and the candidates are the useful half of the + // answer. + return Usagef(nil, + "nothing is recorded, and this checkout does not say which project it is\n"+ + " record one: vallic link \n it can be: %s", + strings.Join(projectNames(projects), ", "), + ) } - if inferred != nil && inferred.MachineName != recorded { - env.Printer.Warn("this checkout resolves to %s on its own, and the recorded project is read first", inferred.MachineName) + env.Printer.Line("%s", recorded) + + if dir, err := config.Dir(); err == nil { + env.Printer.Say(" recorded in %s, for the checkouts on this machine that cannot say", dir) } return nil @@ -210,31 +216,31 @@ func clearLink(env *Env, cfg *config.Config) error { // command reads the same variable again. env.Printer.Good("The configuration file records no project now.") default: - env.Printer.Good("Forgot %s. Checkouts answer for themselves again.", was) + env.Printer.Good("Forgot %s. A checkout that cannot say which project it is will be asked.", was) } if fromEnv != "" { - env.Printer.Warn("VALLIC_PROJECT is set to %s, and it is read before the file", fromEnv) + env.Printer.Warn("VALLIC_PROJECT is set to %s, and every command reads it first", fromEnv) } return nil } -// warnOverridden says what the link has taken over from. +// warnOverridden says where the link will not answer. // -// A link is silent by nature: it changes what every later command resolves to -// and prints nothing at the time. Somebody who has just recorded a project in -// a checkout that was resolving correctly has made that checkout point -// somewhere else, and this is the only moment they can be told. +// A link is silent by nature: it changes what later commands resolve to and +// prints nothing at the time. Somebody who has just linked a project here +// while this checkout names another expects the link to answer for it, and +// it does not: the checkout comes first. func warnOverridden(ctx context.Context, env *Env, projects []api.Project, chosen *api.Project) { if fromEnv := os.Getenv("VALLIC_PROJECT"); fromEnv != "" && fromEnv != chosen.MachineName { - env.Printer.Warn("VALLIC_PROJECT is set to %s, is read before the file, and has not changed", fromEnv) + env.Printer.Warn("VALLIC_PROJECT is set to %s, every command reads it first, and it has not changed", fromEnv) } inferred := inferProject(ctx, projects) if inferred != nil && inferred.ID != chosen.ID { - env.Printer.Warn("this checkout resolved to %s before this, and a recorded project is read first", inferred.MachineName) + env.Printer.Warn("this checkout goes on resolving to %s, which its git remote names; the link answers only where a checkout cannot say", inferred.MachineName) } } diff --git a/internal/cli/release.go b/internal/cli/release.go index a61550a..8773503 100644 --- a/internal/cli/release.go +++ b/internal/cli/release.go @@ -234,9 +234,9 @@ func (e *Env) putBack( } if detail.Protected && !yes && e.Interactive() { - question := fmt.Sprintf("Roll %s back, which is protected?", detail.Name) + question := fmt.Sprintf("Roll %s back, which is protected?", target.Named()) if kind == "redeploy" { - question = fmt.Sprintf("Redeploy %s, which is protected?", detail.Name) + question = fmt.Sprintf("Redeploy %s, which is protected?", target.Named()) } if !e.confirm(question) { diff --git a/internal/cli/target.go b/internal/cli/target.go index ae7532b..12d7175 100644 --- a/internal/cli/target.go +++ b/internal/cli/target.go @@ -2,6 +2,7 @@ package cli import ( "context" + "fmt" "os" "github.com/vallic/vallic-cli/internal/api" @@ -14,12 +15,31 @@ type Target struct { Environment *api.Environment } +// Named is the target as a question about it should name it: the +// environment's slug and the project's label, `acme-production (Acme)`. +// +// Not the environment's name. Every project has a "production", so a +// question that said only that could not show somebody that the checkout +// they ran it in resolved to another project's. +func (t *Target) Named() string { + if t == nil || t.Environment == nil { + return "this environment" + } + + if t.Project == nil || t.Project.Label == "" { + return t.Environment.Slug + } + + return fmt.Sprintf("%s (%s)", t.Environment.Slug, t.Project.Label) +} + // Resolver builds a resolver from the configuration and the flags. // // The precedence is settled here, once, for both nouns: a flag beats the -// environment, which beats the config file, and a checkout answers for -// itself where none of them said anything. See internal/resolve for the two -// rules that hold below that. +// environment variable, and both beat the checkout. The project `vallic +// link` recorded comes after the checkout, not before it — it is one answer +// for every directory on the machine, and a checkout that names its own +// project is the better one. See internal/resolve for the rules below that. func (e *Env) Resolver(client resolve.Client) (*resolve.Resolver, error) { cfg, err := e.Config() if err != nil { @@ -37,7 +57,8 @@ func (e *Env) Resolver(client resolve.Client) (*resolve.Resolver, error) { return &resolve.Resolver{ Client: client, Dir: dir, - ProjectHint: firstNonEmpty(e.flagProject, cfg.Project), + ProjectHint: firstNonEmpty(e.flagProject, os.Getenv("VALLIC_PROJECT")), + ProjectRecorded: cfg.Project, EnvironmentHint: firstNonEmpty(e.flagEnvironment, cfg.Environment), }, nil } diff --git a/internal/cli/target_test.go b/internal/cli/target_test.go new file mode 100644 index 0000000..7eeee17 --- /dev/null +++ b/internal/cli/target_test.go @@ -0,0 +1,32 @@ +package cli + +import ( + "testing" + + "github.com/vallic/vallic-cli/internal/api" +) + +// A question about a protected environment names the project too. +// +// Every project has a "production". Asked "Deploy to production, which is +// protected?", somebody whose checkout had resolved to another project's +// would answer yes, and deploy that one. +func TestTheTargetIsNamedWithItsProject(t *testing.T) { + target := &Target{ + Project: &api.Project{MachineName: "acme", Label: "Acme"}, + Environment: &api.Environment{Slug: "acme-production", Name: "production"}, + } + + if got := target.Named(); got != "acme-production (Acme)" { + t.Errorf("Named() = %q", got) + } + + target.Project.Label = "" + if got := target.Named(); got != "acme-production" { + t.Errorf("Named() without a label = %q", got) + } + + if got := (&Target{}).Named(); got != "this environment" { + t.Errorf("Named() with nothing = %q", got) + } +} diff --git a/internal/resolve/resolve.go b/internal/resolve/resolve.go index bffc03b..9f6cbc7 100644 --- a/internal/resolve/resolve.go +++ b/internal/resolve/resolve.go @@ -119,12 +119,18 @@ type Resolver struct { // Dir is the working directory whose git remote and branch are read. Dir string - // ProjectHint and EnvironmentHint are the flag, then the environment - // variable, then the config file — settled by the caller before it gets - // here, because the precedence is the same for both and belongs in one - // place. + // ProjectHint and EnvironmentHint are what this command was told: the + // flag, then the environment variable — and for the environment, the + // config file — settled by the caller before it gets here. ProjectHint string EnvironmentHint string + + // ProjectRecorded is what `vallic link` wrote in the configuration. It + // answers for every directory on the machine, so it is asked only where + // the checkout cannot answer: read before the git remote, a project + // linked for one checkout sent `vallic drush` and `vallic deploy` run in + // another to the first one's production. + ProjectRecorded string } // Project picks the project. @@ -149,6 +155,12 @@ func (r *Resolver) Project(ctx context.Context) (*api.Project, error) { } } + // What was linked, for a checkout whose remote names no project here: a + // fork, a mirror, a directory with no git at all. + if r.ProjectRecorded != "" { + return matchProject(projects, r.ProjectRecorded) + } + if len(projects) == 1 { return &projects[0], nil } diff --git a/internal/resolve/resolve_test.go b/internal/resolve/resolve_test.go index 9cc0089..19ef398 100644 --- a/internal/resolve/resolve_test.go +++ b/internal/resolve/resolve_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os/exec" "strings" "testing" @@ -205,6 +206,92 @@ func TestProjectIsMatchedByRepository(t *testing.T) { } } +// A checkout that names its project answers ahead of anything linked. +// +// The link is one answer for every directory on the machine. Read first, a +// project linked once answered in a checkout of another, and a command run +// there reached the linked project's environments instead of its own. +func TestACheckoutBeatsTheRecordedProject(t *testing.T) { + client := &fakeClient{ + projects: []api.Project{ + {ID: 1, MachineName: "blog", Repository: repo("acme/blog")}, + {ID: 2, MachineName: "webshop", Repository: repo("acme/shop")}, + }, + } + + r := &Resolver{Client: client, Dir: checkout(t, "git@github.com:acme/shop.git"), ProjectRecorded: "blog"} + + got, err := r.Project(context.Background()) + if err != nil { + t.Fatal(err) + } + if got.MachineName != "webshop" { + t.Errorf("resolved %s, want the checkout's own webshop", got.MachineName) + } +} + +// The linked project answers where the checkout cannot: no git, or a remote +// naming no project this credential can see. +func TestTheRecordedProjectAnswersWhereTheCheckoutCannot(t *testing.T) { + client := &fakeClient{ + projects: []api.Project{ + {ID: 1, MachineName: "blog", Repository: repo("acme/blog")}, + {ID: 2, MachineName: "webshop", Repository: repo("acme/shop")}, + }, + } + + for name, dir := range map[string]string{ + "no git": t.TempDir(), + "a fork's remote": checkout(t, "git@github.com:someone/shop-fork.git"), + } { + got, err := (&Resolver{Client: client, Dir: dir, ProjectRecorded: "blog"}).Project(context.Background()) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if got.MachineName != "blog" { + t.Errorf("%s: resolved %s, want the linked blog", name, got.MachineName) + } + } +} + +// What this command was told — a flag, VALLIC_PROJECT — beats the checkout. +func TestTheHintBeatsTheCheckout(t *testing.T) { + client := &fakeClient{ + projects: []api.Project{ + {ID: 1, MachineName: "blog", Repository: repo("acme/blog")}, + {ID: 2, MachineName: "webshop", Repository: repo("acme/shop")}, + }, + } + + r := &Resolver{Client: client, Dir: checkout(t, "git@github.com:acme/shop.git"), ProjectHint: "blog"} + + got, err := r.Project(context.Background()) + if err != nil { + t.Fatal(err) + } + if got.MachineName != "blog" { + t.Errorf("resolved %s, want the blog the command was told", got.MachineName) + } +} + +// checkout makes a git repository whose origin is remote. +func checkout(t *testing.T, remote string) string { + t.Helper() + + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git is not installed") + } + + dir := t.TempDir() + for _, args := range [][]string{{"init", "-q"}, {"remote", "add", "origin", remote}} { + if out, err := exec.Command("git", append([]string{"-C", dir}, args...)...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + + return dir +} + // Two projects and nothing to choose between them is a question, not a guess. func TestAmbiguousProjectIsRefused(t *testing.T) { client := &fakeClient{