From f9f81b1f68e17b0f4c5c3b40e72aaa0572ff3f17 Mon Sep 17 00:00:00 2001 From: Allison Chiang <83091926+allisonschiang@users.noreply.github.com> Date: Thu, 11 Jun 2026 17:37:25 -0400 Subject: [PATCH 1/3] init --- cli/module_build.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/module_build.go b/cli/module_build.go index c36226fce55..e094152618b 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -207,8 +207,10 @@ func (c *viamClient) moduleBuildStartAction(ctx context.Context, cmd *cli.Comman manifestDir := filepath.Dir(args.Module) mainPyPath := filepath.Join(manifestDir, "src", "main.py") if _, err := os.Stat(mainPyPath); err == nil { - return "", errors.New("cloud build is not currently supported for Windows Python modules.\n" + - "Build locally with 'viam module build local' and upload with 'viam module upload'") + if len(manifest.Models) == 0 { + return "", errors.New("your models must be populated before running cloud build,\n" + + "run 'viam module update-models' and try again") + } } } From 3f757000d755efb177d2a77b96c4622f119148e0 Mon Sep 17 00:00:00 2001 From: Allison Chiang <83091926+allisonschiang@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:00:12 -0400 Subject: [PATCH 2/3] check remote --- cli/module_build.go | 127 +++++++++++++++++++++++++++++++++++---- cli/module_build_test.go | 70 +++++++++++++++++++++ cli/module_registry.go | 7 +++ 3 files changed, 192 insertions(+), 12 deletions(-) diff --git a/cli/module_build.go b/cli/module_build.go index e094152618b..e0d4569efa7 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -11,6 +11,7 @@ import ( "net/url" "os" "os/exec" + "path" "path/filepath" "runtime" "slices" @@ -98,6 +99,117 @@ func parseGitHubRepo(repoURL string) (owner, repo string, ok bool, err error) { return parts[0], strings.TrimSuffix(parts[1], ".git"), true, nil } +// githubFetchManifest fetches and parses a module manifest from a repo at a given +// ref using GitHub's contents API. manifestPath is the manifest's path within the +// repo (e.g. "meta.json" or "subdir/meta.json"). It is a package var so tests can +// stub it. +var githubFetchManifest = func(ctx context.Context, owner, repo, ref, manifestPath, token string) (ModuleManifest, error) { + apiURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/contents/%s?ref=%s", + owner, repo, manifestPath, url.QueryEscape(ref)) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return ModuleManifest{}, err + } + // the raw media type returns the file body directly rather than base64-wrapped JSON + req.Header.Set("Accept", "application/vnd.github.raw+json") + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + return ModuleManifest{}, err + } + defer resp.Body.Close() //nolint:errcheck + if resp.StatusCode != http.StatusOK { + return ModuleManifest{}, fmt.Errorf("unexpected status %d from github api", resp.StatusCode) + } + body, err := io.ReadAll(resp.Body) + if err != nil { + return ModuleManifest{}, err + } + return parseManifest(body) +} + +// githubPathExists reports whether a file exists in a repo at a given ref using +// GitHub's contents API. It is a package var so tests can stub it. +var githubPathExists = func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + apiURL := fmt.Sprintf("https://api.github.com/repos/%s/%s/contents/%s?ref=%s", + owner, repo, filePath, url.QueryEscape(ref)) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiURL, nil) + if err != nil { + return false, err + } + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + return false, err + } + defer resp.Body.Close() //nolint:errcheck + switch resp.StatusCode { + case http.StatusOK: + return true, nil + case http.StatusNotFound: + return false, nil + default: + return false, fmt.Errorf("unexpected status %d from github api", resp.StatusCode) + } +} + +// joinRepoPath builds a repo-relative path (forward slashes, no leading slash) for +// GitHub's contents API from a build workdir and filename. +func joinRepoPath(workdir, name string) string { + return strings.TrimPrefix(path.Join(workdir, name), "/") +} + +// validateModelsPopulated stops an early cloud build that targets Windows for a Go +// module whose manifest in the remote repo has no models populated (which a Windows Go +// cloud build requires). The cloud builder clones and builds the repo at ref, so this +// checks the files the build will actually use -- those in the repo at ref -- rather than +// local copies. The check is scoped to Go modules (detected by a go.mod at the build +// workdir) because Windows Python builds don't work regardless of models. Like +// validateRefExists, it only hard-fails when it can prove models is empty, and otherwise +// (non-github host, unreachable files) proceeds with the build. +func (c *viamClient) validateModelsPopulated( + ctx context.Context, cmd *cli.Command, repoURL, ref, workdir, token string, platforms []string, +) error { + // only Windows cloud builds are affected + if !slices.ContainsFunc(platforms, func(p string) bool { return strings.HasPrefix(p, osWindows+"/") }) { + return nil + } + owner, repo, ok, err := parseGitHubRepo(repoURL) + if err != nil { + return err + } + if !ok { + return nil + } + gArgs := parseStructFromCtx[globalArgs](cmd) + // a go.mod at the build workdir marks this as a Go module; the empty-models failure is + // specific to Windows Go builds, so skip anything else (e.g. Python). + isGo, err := githubPathExists(ctx, owner, repo, ref, joinRepoPath(workdir, "go.mod"), token) + if err != nil { + debugf(cmd.Root().ErrWriter, gArgs.Debug, + "could not determine module language on %s@%s: %v — proceeding anyway", repoURL, ref, err) + return nil + } + if !isGo { + return nil + } + remote, err := githubFetchManifest(ctx, owner, repo, ref, joinRepoPath(workdir, defaultManifestFilename), token) + if err != nil { + debugf(cmd.Root().ErrWriter, gArgs.Debug, + "could not fetch %s from %s@%s to validate models: %v — proceeding anyway", defaultManifestFilename, repoURL, ref, err) + return nil + } + if len(remote.Models) == 0 { + return errors.New("your models must be populated before running a Windows Go cloud build,\n" + + "run 'viam module update-models' and try again") + } + return nil +} + // validateRefExists checks that ref exists on the remote at repoURL before a // cloud build is started, and only stops the build if the ref can be proven to not exist, or // else it will go through to with the build attempt (like with non-github links) @@ -168,6 +280,9 @@ func (c *viamClient) moduleBuildStartForRepo( gitRef := args.Ref token := args.Token workdir := args.Workdir + if err := c.validateModelsPopulated(ctx, cmd, repo, gitRef, workdir, token, platforms); err != nil { + return "", err + } req := buildpb.StartBuildRequest{ Repo: repo, Ref: &gitRef, @@ -202,18 +317,6 @@ func (c *viamClient) moduleBuildStartAction(ctx context.Context, cmd *cli.Comman return "", err } - // Check if this is a Windows Python module by looking for src/main.py - if runtime.GOOS == osWindows && manifest.Build != nil { - manifestDir := filepath.Dir(args.Module) - mainPyPath := filepath.Join(manifestDir, "src", "main.py") - if _, err := os.Stat(mainPyPath); err == nil { - if len(manifest.Models) == 0 { - return "", errors.New("your models must be populated before running cloud build,\n" + - "run 'viam module update-models' and try again") - } - } - } - if manifest.URL == "" { return "", errors.New("meta.json must have a url field set in order to start a cloud build. " + "Ex: 'https://github.com/your-username/your-repo'") diff --git a/cli/module_build_test.go b/cli/module_build_test.go index 62299f95be3..69791e57396 100644 --- a/cli/module_build_test.go +++ b/cli/module_build_test.go @@ -117,6 +117,76 @@ func TestValidateRefExists(t *testing.T) { } } +func TestValidateModelsPopulated(t *testing.T) { + origExists, origFetch := githubPathExists, githubFetchManifest + t.Cleanup(func() { githubPathExists, githubFetchManifest = origExists, origFetch }) + + viamClient := &viamClient{} + cmd := newTestContext(t, map[string]any{}) + + // go.mod existence stubs (Go-module detection) + goMod := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + return true, nil + } + notGoMod := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + return false, nil + } + goModErr := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + return false, errors.New("network down") + } + existsUncalled := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + t.Fatal("githubPathExists should not have been called") + return false, nil + } + + // remote manifest stubs + withModels := func(ctx context.Context, owner, repo, ref, manifestPath, token string) (ModuleManifest, error) { + return ModuleManifest{Models: make([]ModuleComponent, 1)}, nil + } + noModels := func(ctx context.Context, owner, repo, ref, manifestPath, token string) (ModuleManifest, error) { + return ModuleManifest{Models: nil}, nil + } + fetchFailed := func(ctx context.Context, owner, repo, ref, manifestPath, token string) (ModuleManifest, error) { + return ModuleManifest{}, errors.New("network down") + } + fetchUncalled := func(ctx context.Context, owner, repo, ref, manifestPath, token string) (ModuleManifest, error) { + t.Fatal("githubFetchManifest should not have been called") + return ModuleManifest{}, nil + } + + win := []string{"windows/amd64"} + repo := "https://github.com/test-org/test-repo" + cases := []struct { + name string + exists func(context.Context, string, string, string, string, string) (bool, error) + fetch func(context.Context, string, string, string, string, string) (ModuleManifest, error) + url string + platforms []string + wantErrSubstr string + }{ + {"windows go module with models", goMod, withModels, repo, win, ""}, + {"windows go module, empty models", goMod, noModels, repo, win, "models must be populated"}, + {"windows go module, manifest fetch fails -> proceed", goMod, fetchFailed, repo, win, ""}, + {"windows non-go module skips check", notGoMod, fetchUncalled, repo, win, ""}, + {"windows, go.mod check fails -> proceed", goModErr, fetchUncalled, repo, win, ""}, + {"non-windows build skips check", existsUncalled, fetchUncalled, repo, []string{"linux/amd64"}, ""}, + {"non-github host skips check", existsUncalled, fetchUncalled, "https://gitlab.com/test-org/test-repo", win, ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + githubPathExists = tc.exists + githubFetchManifest = tc.fetch + err := viamClient.validateModelsPopulated(context.Background(), cmd, tc.url, "main", "", "", tc.platforms) + if tc.wantErrSubstr == "" { + test.That(t, err, test.ShouldBeNil) + } else { + test.That(t, err, test.ShouldNotBeNil) + test.That(t, err.Error(), test.ShouldContainSubstring, tc.wantErrSubstr) + } + }) + } +} + func TestStartBuild(t *testing.T) { // stub the ref validator so the test doesn't hit the network against // the placeholder url in the test manifest diff --git a/cli/module_registry.go b/cli/module_registry.go index da6e3ba9225..596983b0b4f 100644 --- a/cli/module_registry.go +++ b/cli/module_registry.go @@ -855,6 +855,13 @@ func loadManifest(manifestPath string) (ModuleManifest, error) { } return ModuleManifest{}, err } + return parseManifest(manifestBytes) +} + +// parseManifest unmarshals raw meta.json bytes into a ModuleManifest. It is +// shared by loadManifest (local file) and the remote-manifest fetch used to +// validate a cloud build against the repo it will actually build. +func parseManifest(manifestBytes []byte) (ModuleManifest, error) { var manifest ModuleManifest if err := json.Unmarshal(manifestBytes, &manifest); err != nil { return ModuleManifest{}, err From 253ae647da209f7e742e1e1f109fdda249fb8e7a Mon Sep 17 00:00:00 2001 From: Allison Chiang <83091926+allisonschiang@users.noreply.github.com> Date: Thu, 11 Jun 2026 18:06:58 -0400 Subject: [PATCH 3/3] update go check --- cli/module_build.go | 21 +++++++++++++-------- cli/module_build_test.go | 18 +++++++++--------- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/cli/module_build.go b/cli/module_build.go index e0d4569efa7..2e110a7ed33 100644 --- a/cli/module_build.go +++ b/cli/module_build.go @@ -163,14 +163,18 @@ func joinRepoPath(workdir, name string) string { return strings.TrimPrefix(path.Join(workdir, name), "/") } +// pythonMainPath is the entrypoint a generated Python module has (src/main.py); its +// presence in the repo marks the module as Python. +const pythonMainPath = "src/main.py" + // validateModelsPopulated stops an early cloud build that targets Windows for a Go // module whose manifest in the remote repo has no models populated (which a Windows Go // cloud build requires). The cloud builder clones and builds the repo at ref, so this // checks the files the build will actually use -- those in the repo at ref -- rather than -// local copies. The check is scoped to Go modules (detected by a go.mod at the build -// workdir) because Windows Python builds don't work regardless of models. Like -// validateRefExists, it only hard-fails when it can prove models is empty, and otherwise -// (non-github host, unreachable files) proceeds with the build. +// local copies. Windows Python builds don't work regardless of models, so the check first +// rules out Python (a src/main.py at the build workdir) and only then treats the module as +// Go. Like validateRefExists, it only hard-fails when it can prove models is empty, and +// otherwise (non-github host, unreachable files) proceeds with the build. func (c *viamClient) validateModelsPopulated( ctx context.Context, cmd *cli.Command, repoURL, ref, workdir, token string, platforms []string, ) error { @@ -186,15 +190,16 @@ func (c *viamClient) validateModelsPopulated( return nil } gArgs := parseStructFromCtx[globalArgs](cmd) - // a go.mod at the build workdir marks this as a Go module; the empty-models failure is - // specific to Windows Go builds, so skip anything else (e.g. Python). - isGo, err := githubPathExists(ctx, owner, repo, ref, joinRepoPath(workdir, "go.mod"), token) + // rule out Python first: a src/main.py at the build workdir marks the module as Python, + // whose Windows builds don't work regardless of models -- so skip it. Anything else is + // treated as Go. + isPython, err := githubPathExists(ctx, owner, repo, ref, joinRepoPath(workdir, pythonMainPath), token) if err != nil { debugf(cmd.Root().ErrWriter, gArgs.Debug, "could not determine module language on %s@%s: %v — proceeding anyway", repoURL, ref, err) return nil } - if !isGo { + if isPython { return nil } remote, err := githubFetchManifest(ctx, owner, repo, ref, joinRepoPath(workdir, defaultManifestFilename), token) diff --git a/cli/module_build_test.go b/cli/module_build_test.go index 69791e57396..238c8373e79 100644 --- a/cli/module_build_test.go +++ b/cli/module_build_test.go @@ -124,14 +124,14 @@ func TestValidateModelsPopulated(t *testing.T) { viamClient := &viamClient{} cmd := newTestContext(t, map[string]any{}) - // go.mod existence stubs (Go-module detection) - goMod := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + // language-detection stubs: githubPathExists checks for src/main.py (Python marker) + isPython := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { return true, nil } - notGoMod := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + notPython := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { return false, nil } - goModErr := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { + pythonCheckErr := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { return false, errors.New("network down") } existsUncalled := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) { @@ -164,11 +164,11 @@ func TestValidateModelsPopulated(t *testing.T) { platforms []string wantErrSubstr string }{ - {"windows go module with models", goMod, withModels, repo, win, ""}, - {"windows go module, empty models", goMod, noModels, repo, win, "models must be populated"}, - {"windows go module, manifest fetch fails -> proceed", goMod, fetchFailed, repo, win, ""}, - {"windows non-go module skips check", notGoMod, fetchUncalled, repo, win, ""}, - {"windows, go.mod check fails -> proceed", goModErr, fetchUncalled, repo, win, ""}, + {"windows go module with models", notPython, withModels, repo, win, ""}, + {"windows go module, empty models", notPython, noModels, repo, win, "models must be populated"}, + {"windows go module, manifest fetch fails -> proceed", notPython, fetchFailed, repo, win, ""}, + {"windows python module skips check", isPython, fetchUncalled, repo, win, ""}, + {"windows, language check fails -> proceed", pythonCheckErr, fetchUncalled, repo, win, ""}, {"non-windows build skips check", existsUncalled, fetchUncalled, repo, []string{"linux/amd64"}, ""}, {"non-github host skips check", existsUncalled, fetchUncalled, "https://gitlab.com/test-org/test-repo", win, ""}, }