RSDK-13288

cli/module_build.go

@@ -11,6 +11,7 @@ import (
 	"net/url"
 	"os"
 	"os/exec"
+	"path"
 	"path/filepath"
 	"runtime"
 	"slices"
@@ -98,6 +99,122 @@ 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), "/")
+}
+
+// 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. 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 {
+	// 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)
+	// 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 isPython {
+		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 +285,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,16 +322,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 {
-			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 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'")

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{})
+
+	// 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
+	}
+	notPython := func(ctx context.Context, owner, repo, ref, filePath, token string) (bool, error) {
+		return false, nil
+	}
+	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) {
+		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", 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, ""},
+	}
+	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

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