Skip to content

Commit 9bb88b9

Browse files
rogpepperogpeppe
committed
mod/modcache: implement FetchFromCache
This adds a method to the registry cache to enable fetching from the cache directly without falling back to registry access. This will be used as the fast path for looking up modules by their absolute version without the need to check all path prefixes in the registry. We want the `modload` package to be able to depend on this logic, so avoid the need for modcache to depend on modload by exporting the concrete type instead of the interface type. Also define an interface type for the new cache method so we can potentially have different kinds of cache implementations in the future. Also document the error return of the `Fetch` and the new `FetchFromCache` methods so that we can distinguish between "module not present" and other errors. For #3618. Signed-off-by: Roger Peppe <[email protected]> Change-Id: Idf713910ccb7f09b0c43c125281171dfb5638866 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211884 Reviewed-by: Daniel Martí <[email protected]> Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]>
1 parent 0105c3f commit 9bb88b9

File tree

5 files changed

+82
-21
lines changed

5 files changed

+82
-21
lines changed

‎internal/mod/modpkgload/pkgload.go

+12
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,21 @@ import (
2121
type Registry interface {
2222
// Fetch returns the location of the contents for the given module
2323
// version, downloading it if necessary.
24+
// It returns an error that satisfies [errors.Is]([modregistry.ErrNotFound]) if the
25+
// module is not present in the store at this version.
2426
Fetch(ctx context.Context, m module.Version) (module.SourceLoc, error)
2527
}
2628

29+
// CachedRegistry is optionally implemented by a registry that
30+
// implements a cache.
31+
type CachedRegistry interface {
32+
// FetchFromCache looks up the given module in the cache.
33+
// It returns an error that satisfies [errors.Is]([modregistry.ErrNotFound]) if the
34+
// module is not present in the cache at this version or if there
35+
// is no cache.
36+
FetchFromCache(mv module.Version) (module.SourceLoc, error)
37+
}
38+
2739
// Flags is a set of flags tracking metadata about a package.
2840
type Flags int8
2941

‎mod/modcache/cache.go

+7-7
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,13 @@ var errNotCached = fmt.Errorf("not in cache")
2424
// returning the name of the cache file and the result.
2525
// If the read fails, the caller can use
2626
// writeDiskModFile(file, data) to write a new cache entry.
27-
func (c *cache) readDiskModFile(mv module.Version) (file string, data []byte, err error) {
27+
func (c *Cache) readDiskModFile(mv module.Version) (file string, data []byte, err error) {
2828
return c.readDiskCache(mv, "mod")
2929
}
3030

3131
// writeDiskModFile writes a cue.mod/module.cue cache entry.
3232
// The file name must have been returned by a previous call to readDiskModFile.
33-
func (c *cache) writeDiskModFile(ctx context.Context, file string, text []byte) error {
33+
func (c *Cache) writeDiskModFile(ctx context.Context, file string, text []byte) error {
3434
return c.writeDiskCache(ctx, file, text)
3535
}
3636

@@ -39,7 +39,7 @@ func (c *cache) writeDiskModFile(ctx context.Context, file string, text []byte)
3939
// It returns the name of the cache file and the content of the file.
4040
// If the read fails, the caller can use
4141
// writeDiskCache(file, data) to write a new cache entry.
42-
func (c *cache) readDiskCache(mv module.Version, suffix string) (file string, data []byte, err error) {
42+
func (c *Cache) readDiskCache(mv module.Version, suffix string) (file string, data []byte, err error) {
4343
file, err = c.cachePath(mv, suffix)
4444
if err != nil {
4545
return "", nil, errNotCached
@@ -53,7 +53,7 @@ func (c *cache) readDiskCache(mv module.Version, suffix string) (file string, da
5353

5454
// writeDiskCache is the generic "write to a cache file" implementation.
5555
// The file must have been returned by a previous call to readDiskCache.
56-
func (c *cache) writeDiskCache(ctx context.Context, file string, data []byte) error {
56+
func (c *Cache) writeDiskCache(ctx context.Context, file string, data []byte) error {
5757
if file == "" {
5858
return nil
5959
}
@@ -95,7 +95,7 @@ func (c *cache) writeDiskCache(ctx context.Context, file string, data []byte) er
9595
// An error satisfying [errors.Is](err, [fs.ErrNotExist]) will be returned
9696
// along with the directory if the directory does not exist or if the directory
9797
// is not completely populated.
98-
func (c *cache) downloadDir(m module.Version) (string, error) {
98+
func (c *Cache) downloadDir(m module.Version) (string, error) {
9999
if !m.IsCanonical() {
100100
return "", fmt.Errorf("non-semver module version %q", m.Version())
101101
}
@@ -132,7 +132,7 @@ func (c *cache) downloadDir(m module.Version) (string, error) {
132132
return dir, nil
133133
}
134134

135-
func (c *cache) cachePath(m module.Version, suffix string) (string, error) {
135+
func (c *Cache) cachePath(m module.Version, suffix string) (string, error) {
136136
if !m.IsValid() || m.Version() == "" {
137137
return "", fmt.Errorf("non-semver module version %q", m)
138138
}
@@ -161,7 +161,7 @@ func (e *downloadDirPartialError) Is(err error) bool { return err == fs.ErrNotEx
161161

162162
// lockVersion locks a file within the module cache that guards the downloading
163163
// and extraction of module data for the given module version.
164-
func (c *cache) lockVersion(mod module.Version) (unlock func(), err error) {
164+
func (c *Cache) lockVersion(mod module.Version) (unlock func(), err error) {
165165
path, err := c.cachePath(mod, "lock")
166166
if err != nil {
167167
return nil, err

‎mod/modcache/fetch.go

+26-12
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616

1717
"github.com/rogpeppe/go-internal/robustio"
1818

19-
"cuelang.org/go/internal/mod/modload"
2019
"cuelang.org/go/internal/par"
2120
"cuelang.org/go/mod/modfile"
2221
"cuelang.org/go/mod/modregistry"
@@ -34,25 +33,28 @@ const logging = false // TODO hook this up to CUE_DEBUG
3433
// returned by the registry implement the `OSRootFS` interface,
3534
// allowing a caller to find the native OS filepath where modules
3635
// are stored.
37-
func New(registry *modregistry.Client, dir string) (modload.Registry, error) {
36+
//
37+
// The returned type implements [modconfig.Registry]
38+
// and [modconfig.CachedRegistry].
39+
func New(registry *modregistry.Client, dir string) (*Cache, error) {
3840
info, err := os.Stat(dir)
3941
if err == nil && !info.IsDir() {
4042
return nil, fmt.Errorf("%q is not a directory", dir)
4143
}
42-
return &cache{
44+
return &Cache{
4345
dir: filepath.Join(dir, "mod"),
4446
reg: registry,
4547
}, nil
4648
}
4749

48-
type cache struct {
50+
type Cache struct {
4951
dir string // typically ${CUE_CACHE_DIR}/mod
5052
reg *modregistry.Client
5153
downloadZipCache par.ErrCache[module.Version, string]
5254
modFileCache par.ErrCache[string, []byte]
5355
}
5456

55-
func (c *cache) Requirements(ctx context.Context, mv module.Version) ([]module.Version, error) {
57+
func (c *Cache) Requirements(ctx context.Context, mv module.Version) ([]module.Version, error) {
5658
data, err := c.downloadModFile(ctx, mv)
5759
if err != nil {
5860
return nil, err
@@ -64,9 +66,21 @@ func (c *cache) Requirements(ctx context.Context, mv module.Version) ([]module.V
6466
return mf.DepVersions(), nil
6567
}
6668

69+
// FetchFromCache implements [cuelang.org/go/mod/modconfig.CachedRegistry].
70+
func (c *Cache) FetchFromCache(mv module.Version) (module.SourceLoc, error) {
71+
dir, err := c.downloadDir(mv)
72+
if err != nil {
73+
if errors.Is(err, fs.ErrNotExist) {
74+
return module.SourceLoc{}, modregistry.ErrNotFound
75+
}
76+
return module.SourceLoc{}, err
77+
}
78+
return c.dirToLocation(dir), nil
79+
}
80+
6781
// Fetch returns the location of the contents for the given module
6882
// version, downloading it if necessary.
69-
func (c *cache) Fetch(ctx context.Context, mv module.Version) (module.SourceLoc, error) {
83+
func (c *Cache) Fetch(ctx context.Context, mv module.Version) (module.SourceLoc, error) {
7084
dir, err := c.downloadDir(mv)
7185
if err == nil {
7286
// The directory has already been completely extracted (no .partial file exists).
@@ -151,12 +165,12 @@ func (c *cache) Fetch(ctx context.Context, mv module.Version) (module.SourceLoc,
151165
}
152166

153167
// ModuleVersions implements [modload.Registry.ModuleVersions].
154-
func (c *cache) ModuleVersions(ctx context.Context, mpath string) ([]string, error) {
168+
func (c *Cache) ModuleVersions(ctx context.Context, mpath string) ([]string, error) {
155169
// TODO should this do any kind of short-term caching?
156170
return c.reg.ModuleVersions(ctx, mpath)
157171
}
158172

159-
func (c *cache) downloadZip(ctx context.Context, mv module.Version) (zipfile string, err error) {
173+
func (c *Cache) downloadZip(ctx context.Context, mv module.Version) (zipfile string, err error) {
160174
return c.downloadZipCache.Do(mv, func() (string, error) {
161175
zipfile, err := c.cachePath(mv, "zip")
162176
if err != nil {
@@ -181,7 +195,7 @@ func (c *cache) downloadZip(ctx context.Context, mv module.Version) (zipfile str
181195
})
182196
}
183197

184-
func (c *cache) downloadZip1(ctx context.Context, mod module.Version, zipfile string) (err error) {
198+
func (c *Cache) downloadZip1(ctx context.Context, mod module.Version, zipfile string) (err error) {
185199
// Double-check that the zipfile was not created while we were waiting for
186200
// the lock in downloadZip.
187201
if _, err := os.Stat(zipfile); err == nil {
@@ -243,7 +257,7 @@ func (c *cache) downloadZip1(ctx context.Context, mod module.Version, zipfile st
243257
return nil
244258
}
245259

246-
func (c *cache) downloadModFile(ctx context.Context, mod module.Version) ([]byte, error) {
260+
func (c *Cache) downloadModFile(ctx context.Context, mod module.Version) ([]byte, error) {
247261
return c.modFileCache.Do(mod.String(), func() ([]byte, error) {
248262
modfile, data, err := c.readDiskModFile(mod)
249263
if err == nil {
@@ -265,7 +279,7 @@ func (c *cache) downloadModFile(ctx context.Context, mod module.Version) ([]byte
265279
})
266280
}
267281

268-
func (c *cache) downloadModFile1(ctx context.Context, mod module.Version, modfile string) ([]byte, error) {
282+
func (c *Cache) downloadModFile1(ctx context.Context, mod module.Version, modfile string) ([]byte, error) {
269283
m, err := c.reg.GetModule(ctx, mod)
270284
if err != nil {
271285
return nil, err
@@ -280,7 +294,7 @@ func (c *cache) downloadModFile1(ctx context.Context, mod module.Version, modfil
280294
return data, nil
281295
}
282296

283-
func (c *cache) dirToLocation(fpath string) module.SourceLoc {
297+
func (c *Cache) dirToLocation(fpath string) module.SourceLoc {
284298
return module.SourceLoc{
285299
FS: module.OSDirFS(fpath),
286300
Dir: ".",

‎mod/modcache/modcache_test.go

+22
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,20 @@ deps: {
7777
qt.Assert(t, qt.Matches(string(data), `(?s).*module: "example.com/foo@v0".*`))
7878
}
7979

80+
func TestFetchFromCacheNotFound(t *testing.T) {
81+
dir := t.TempDir()
82+
t.Cleanup(func() {
83+
RemoveAll(dir)
84+
})
85+
// The cache should never be hit, so just use a nil registry value.
86+
// We'll get a panic if it gets used.
87+
cr, err := New(nil, dir)
88+
qt.Assert(t, qt.IsNil(err))
89+
_, err = cr.FetchFromCache(module.MustNewVersion("example.com/foo", "v0.0.1"))
90+
qt.Assert(t, qt.Not(qt.IsNil(err)))
91+
qt.Assert(t, qt.ErrorIs(err, modregistry.ErrNotFound))
92+
}
93+
8094
func TestFetch(t *testing.T) {
8195
dir := t.TempDir()
8296
t.Cleanup(func() {
@@ -138,6 +152,14 @@ package x
138152
return
139153
}
140154
checkContents(t, loc)
155+
156+
// After Fetch has succeeded, FetchFromCache should also succeed
157+
// and return the same thing.
158+
loc, err = cr.FetchFromCache(module.MustNewVersion("example.com/foo", "v0.0.1"))
159+
if !qt.Check(t, qt.IsNil(err)) {
160+
return
161+
}
162+
checkContents(t, loc)
141163
}
142164
wg.Add(2)
143165
go fetch(r)

‎mod/modconfig/modconfig.go

+15-2
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"cuelang.org/go/internal/cueconfig"
2222
"cuelang.org/go/internal/cueversion"
2323
"cuelang.org/go/internal/mod/modload"
24+
"cuelang.org/go/internal/mod/modpkgload"
2425
"cuelang.org/go/internal/mod/modresolve"
2526
"cuelang.org/go/mod/modcache"
2627
"cuelang.org/go/mod/modregistry"
@@ -42,12 +43,24 @@ type Registry interface {
4243
ModuleVersions(ctx context.Context, mpath string) ([]string, error)
4344
}
4445

46+
// CachedRegistry is optionally implemented by a registry that
47+
// contains a cache.
48+
type CachedRegistry interface {
49+
// FetchFromCache looks up the given module in the cache.
50+
// It returns an error that satisfies [errors.Is]([modregistry.ErrNotFound]) if the
51+
// module is not present in the cache at this version or if there
52+
// is no cache.
53+
FetchFromCache(mv module.Version) (module.SourceLoc, error)
54+
}
55+
4556
// We don't want to make modload part of the cue/load API,
4657
// so we define the above type independently, but we want
4758
// it to be interchangeable, so check that statically here.
4859
var (
49-
_ Registry = modload.Registry(nil)
50-
_ modload.Registry = Registry(nil)
60+
_ Registry = modload.Registry(nil)
61+
_ modload.Registry = Registry(nil)
62+
_ CachedRegistry = modpkgload.CachedRegistry(nil)
63+
_ modpkgload.CachedRegistry = CachedRegistry(nil)
5164
)
5265

5366
// DefaultRegistry is the default registry host.

0 commit comments

Comments
 (0)