internal/mod/modregistry: avoid round trip

There's no need to resolve the name just to fetch the
manifest in another round trip when we can fetch
the manifest in one request.

Signed-off-by: Roger Peppe <[email protected]>
Change-Id: I59d07b537d2b969759c919d9c8d11c7bc93a1554
Reviewed-on: https://review-eu.gerrithub.io/c/cue-lang/cue/+/1173533
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>

cmd/cue/cmd/testdata/script/registry_auth.txtar

@@ -9,7 +9,7 @@ cmp stdout expect-stdout
 env-fill dockerconfig/badpassword.json
 cp dockerconfig/badpassword.json dockerconfig/config.json
 ! exec cue export .
-stderr 'instance: cannot resolve dependencies: example.com/[email protected]: module example.com/[email protected]: error response: 401 Unauthorized: authentication required'
+stderr 'instance: cannot resolve dependencies: example.com/[email protected]: module example.com/[email protected]: error response: 401 Unauthorized; body: "invalid credentials'
 
 -- dockerconfig/config.json --
 {

cmd/cue/cmd/testdata/script/registry_module_not_found.txtar

@@ -5,7 +5,7 @@
 cmp stderr expect-stderr
 
 -- expect-stderr --
-instance: cannot resolve dependencies: example.com/[email protected]: module example.com/[email protected]: error response: 404 Not Found: repository name not known to registry
+instance: cannot resolve dependencies: example.com/[email protected]: module example.com/[email protected]: module not found
 -- main.cue --
 package main
 import "example.com/e"

cmd/cue/cmd/testdata/script/registry_wrong_prefix.txtar

@@ -3,7 +3,7 @@ env CUE_REGISTRY=$DEBUG_REGISTRY_HOST/something+insecure
 cmp stderr expect-stderr
 
 -- expect-stderr --
-instance: cannot resolve dependencies: example.com/[email protected]: module example.com/[email protected]: error response: 404 Not Found: repository name not known to registry
+instance: cannot resolve dependencies: example.com/[email protected]: module example.com/[email protected]: 404 Not Found: name unknown: repository name not known to registry
 -- main.cue --
 package main
 import "example.com/e"

cue/load/testdata/testfetch/depnotfound.txtar

@@ -1,5 +1,5 @@
 -- out/modfetch/error --
-instance: cannot resolve dependencies: [email protected]: module [email protected]: error response: 404 Not Found: repository name not known to registry
+instance: cannot resolve dependencies: [email protected]: module [email protected]: 404 Not Found: name unknown: repository name not known to registry
 -- cue.mod/module.cue --
 module: "main.org@v0"
 

internal/mod/modregistry/client.go

@@ -62,17 +62,13 @@ func NewClient(registry ociregistry.Interface) *Client {
 // module is not present in the store at this version.
 func (c *Client) GetModule(ctx context.Context, m module.Version) (*Module, error) {
 	repoName := c.repoName(m.Path())
-	modDesc, err := c.registry.ResolveTag(ctx, repoName, m.Version())
+	manifest, modDesc, err := fetchManifest(ctx, c.registry, repoName, m.Version())
 	if err != nil {
 		if errors.Is(err, ociregistry.ErrManifestUnknown) {
 			return nil, fmt.Errorf("module %v: %w", m, ErrNotFound)
 		}
 		return nil, fmt.Errorf("module %v: %v", m, err)
 	}
-	manifest, digest, err := fetchManifest(ctx, c.registry, repoName, modDesc)
-	if err != nil {
-		return nil, fmt.Errorf("cannot unmarshal manifest data: %v", err)
-	}
 	if !isModule(manifest) {
 		return nil, fmt.Errorf("%v does not resolve to a manifest (media type is %q)", m, modDesc.MediaType)
 	}
@@ -88,7 +84,7 @@ func (c *Client) GetModule(ctx context.Context, m module.Version) (*Module, erro
 		client:         c,
 		repo:           repoName,
 		manifest:       *manifest,
-		manifestDigest: digest,
+		manifestDigest: modDesc.Digest,
 	}, nil
 }
 
@@ -325,24 +321,25 @@ func (m *Module) ManifestDigest() ociregistry.Digest {
 	return m.manifestDigest
 }
 
-func fetchManifest(ctx context.Context, r ociregistry.Interface, repoName string, desc ocispec.Descriptor) (*ociregistry.Manifest, digest.Digest, error) {
-	if !isJSON(desc.MediaType) {
-		return nil, "", fmt.Errorf("expected JSON media type but %q does not look like JSON", desc.MediaType)
-	}
-	rd, err := r.GetManifest(ctx, repoName, desc.Digest)
+func fetchManifest(ctx context.Context, r ociregistry.Interface, repoName string, vers string) (*ociregistry.Manifest, ociregistry.Descriptor, error) {
+	rd, err := r.GetTag(ctx, repoName, vers)
 	if err != nil {
-		return nil, "", err
+		return nil, ociregistry.Descriptor{}, err
+	}
+	desc := rd.Descriptor()
+	if !isJSON(desc.MediaType) {
+		return nil, ociregistry.Descriptor{}, fmt.Errorf("expected JSON media type but %q does not look like JSON", desc.MediaType)
 	}
 	defer rd.Close()
 	data, err := io.ReadAll(rd)
 	if err != nil {
-		return nil, "", err
+		return nil, ociregistry.Descriptor{}, err
 	}
 	var m ociregistry.Manifest
 	if err := json.Unmarshal(data, &m); err != nil {
-		return nil, "", fmt.Errorf("cannot decode %s content as manifest: %v", desc.MediaType, err)
+		return nil, ociregistry.Descriptor{}, fmt.Errorf("cannot decode %s content as manifest: %v", desc.MediaType, err)
 	}
-	return &m, rd.Descriptor().Digest, nil
+	return &m, desc, nil
 }
 
 func isModule(m *ocispec.Manifest) bool {

internal/mod/modrequirements/requirements_test.go

@@ -113,7 +113,7 @@ deps: "bar.com@v0": v: "v0.0.2"	// doesn't exist
 		"foo.com/bar/[email protected]",
 	), nil)
 	_, err := rs.Graph(ctx)
-	qt.Assert(t, qt.ErrorMatches(err, `[email protected]: module [email protected]: error response: 404 Not Found: repository name not known to registry`))
+	qt.Assert(t, qt.ErrorMatches(err, `[email protected]: module [email protected]: 404 Not Found: name unknown: repository name not known to registry`))
 	qt.Assert(t, qt.ErrorAs(err, new(*mvs.BuildListError[module.Version])))
 }