Skip to content

Commit 857fbf1

Browse files
sammy-dasammy-da
committed
lockfiles: properly deal with builtin libs in dependncies field
1 parent 6f9e720 commit 857fbf1

File tree

7 files changed

+124
-43
lines changed

7 files changed

+124
-43
lines changed

‎cmd/dpm/cmd/lockfile_test.go‎

Lines changed: 40 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,19 @@ import (
1212
"daml.com/x/assistant/pkg/resolution"
1313
"daml.com/x/assistant/pkg/testutil"
1414
"github.com/goccy/go-yaml"
15+
"github.com/samber/lo"
1516
"github.com/stretchr/testify/assert"
1617
"github.com/stretchr/testify/require"
1718
)
1819

20+
func get(t *testing.T, lock *packagelock.PackageLock, s string) *packagelock.Dar {
21+
d, ok := lo.Find(lock.Dars, func(d *packagelock.Dar) bool {
22+
return d.URI.String() == s
23+
})
24+
require.Truef(t, ok, "expected %q dar is missing in lockfile", s)
25+
return d
26+
}
27+
1928
func (suite *MainSuite) TestLockfileUpdate() {
2029
t := suite.T()
2130
ctx := t.Context()
@@ -29,6 +38,13 @@ func (suite *MainSuite) TestLockfileUpdate() {
2938
multiPackageDir := testutil.TestdataPath(t, "simple-multi-package")
3039
t.Setenv(assistantconfig.DamlMultiPackageEnvVar, multiPackageDir)
3140

41+
cleanup := func() {
42+
_ = os.Remove(filepath.Join(multiPackageDir, "a", assistantconfig.DamlLocalFilename))
43+
_ = os.Remove(filepath.Join(multiPackageDir, "b", assistantconfig.DamlLocalFilename))
44+
}
45+
cleanup()
46+
t.Cleanup(cleanup)
47+
3248
// TODO: using a PushComponent() for lack of a PushDar() for now
3349
testutil.PushComponent(t, ctx, reg, "meep", "1.2.3", testutil.TestdataPath(t, "some-dar"), "latest")
3450
testutil.PushComponent(t, ctx, reg, "sheep", "4.5.6", testutil.TestdataPath(t, "some-dar"), "latest")
@@ -38,17 +54,22 @@ func (suite *MainSuite) TestLockfileUpdate() {
3854

3955
aLock, err := packagelock.ReadPackageLock(filepath.Join(multiPackageDir, "a", assistantconfig.DpmLockFileName))
4056
require.NoError(t, err)
41-
assert.Len(t, aLock.Dars, 1)
42-
assert.Equal(t, fmt.Sprintf("oci://%s/components/meep:1.2.3", os.Getenv(assistantconfig.OciRegistryEnvVar)), aLock.Dars[0].URI)
43-
assert.NotEmpty(t, aLock.Dars[0].Digest)
57+
assert.Len(t, aLock.Dars, 2)
58+
d := get(t, aLock, fmt.Sprintf("oci://%s/components/meep:1.2.3", os.Getenv(assistantconfig.OciRegistryEnvVar)))
59+
assert.NotEmpty(t, d.Digest)
60+
61+
d = get(t, aLock, "builtin://daml-script")
62+
assert.Empty(t, d.Digest)
4463

4564
bLock, err := packagelock.ReadPackageLock(filepath.Join(multiPackageDir, "b", assistantconfig.DpmLockFileName))
4665
require.NoError(t, err)
4766
assert.Len(t, bLock.Dars, 2)
48-
assert.Equal(t, fmt.Sprintf("oci://%s/components/meep:1.2.3", os.Getenv(assistantconfig.OciRegistryEnvVar)), bLock.Dars[0].URI)
49-
assert.NotEmpty(t, bLock.Dars[0].Digest)
50-
assert.Equal(t, fmt.Sprintf("oci://%s/components/sheep:4.5.6", os.Getenv(assistantconfig.OciRegistryEnvVar)), bLock.Dars[1].URI)
51-
assert.NotEmpty(t, bLock.Dars[1].Digest)
67+
68+
d = get(t, bLock, fmt.Sprintf("oci://%s/components/meep:1.2.3", os.Getenv(assistantconfig.OciRegistryEnvVar)))
69+
assert.NotEmpty(t, d.Digest)
70+
71+
d = get(t, bLock, fmt.Sprintf("oci://%s/components/sheep:4.5.6", os.Getenv(assistantconfig.OciRegistryEnvVar)))
72+
assert.NotEmpty(t, d.Digest)
5273

5374
t.Run("bump versions", func(t *testing.T) {
5475
testutil.PushComponent(t, ctx, reg, "meep", "2.0.0", testutil.TestdataPath(t, "some-dar"), "latest")
@@ -62,20 +83,20 @@ func (suite *MainSuite) TestLockfileUpdate() {
6283
bLock, err = packagelock.ReadPackageLock(filepath.Join(multiPackageDir, "b", assistantconfig.DpmLockFileName))
6384
require.NoError(t, err)
6485

65-
assert.Len(t, aLock.Dars, 1)
86+
assert.Len(t, aLock.Dars, 2)
6687
assert.Len(t, bLock.Dars, 2)
6788

6889
t.Run("pinned stay pinned", func(t *testing.T) {
69-
assert.Equal(t, fmt.Sprintf("oci://%s/components/meep:1.2.3", os.Getenv(assistantconfig.OciRegistryEnvVar)), aLock.Dars[0].URI)
70-
assert.NotEmpty(t, aLock.Dars[0].Digest)
90+
d = get(t, aLock, fmt.Sprintf("oci://%s/components/meep:1.2.3", os.Getenv(assistantconfig.OciRegistryEnvVar)))
91+
assert.NotEmpty(t, d.Digest)
7192

72-
assert.Equal(t, fmt.Sprintf("oci://%s/components/sheep:4.5.6", os.Getenv(assistantconfig.OciRegistryEnvVar)), bLock.Dars[1].URI)
73-
assert.NotEmpty(t, bLock.Dars[1].Digest)
93+
d = get(t, bLock, fmt.Sprintf("oci://%s/components/sheep:4.5.6", os.Getenv(assistantconfig.OciRegistryEnvVar)))
94+
assert.NotEmpty(t, d.Digest)
7495
})
7596

7697
t.Run("floaty get bumped", func(t *testing.T) {
77-
assert.Equal(t, fmt.Sprintf("oci://%s/components/meep:2.0.0", os.Getenv(assistantconfig.OciRegistryEnvVar)), bLock.Dars[0].URI)
78-
assert.NotEmpty(t, bLock.Dars[0].Digest)
98+
d = get(t, bLock, fmt.Sprintf("oci://%s/components/meep:2.0.0", os.Getenv(assistantconfig.OciRegistryEnvVar)))
99+
assert.NotEmpty(t, d.Digest)
79100
})
80101
})
81102

@@ -90,16 +111,16 @@ func (suite *MainSuite) TestLockfileUpdate() {
90111
deepResolution := resolution.Resolution{}
91112
require.NoError(t, yaml.Unmarshal(output, &deepResolution))
92113

93-
assert.Len(t,
94-
deepResolution.Packages[filepath.Join(multiPackageDir, "a")].Imports[resolution.DarImportsFields],
95-
1,
96-
)
114+
aRes := deepResolution.Packages[filepath.Join(multiPackageDir, "a")].Imports[resolution.DarImportsFields]
115+
assert.Len(t, aRes, 2)
97116
assert.Len(t,
98117
deepResolution.Packages[filepath.Join(multiPackageDir, "b")].Imports[resolution.DarImportsFields],
99118
2,
100119
)
101120

102-
dar, err := os.ReadFile(deepResolution.Packages[filepath.Join(multiPackageDir, "a")].Imports[resolution.DarImportsFields][0])
121+
assert.Contains(t, aRes, "daml-script")
122+
123+
dar, err := os.ReadFile(aRes[1])
103124
require.NoError(t, err)
104125
assert.Contains(t, string(dar), "haha not a real dar")
105126
})

‎pkg/damlpackage/locations.go‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,12 @@ func (p *DamlPackage) computeResolvedDependencies(defaultLocation *ArtifactLocat
9191
continue
9292
}
9393
resolved[d] = &ResolvedDependency{FullUrl: u}
94+
} else if strings.HasPrefix(d, "http://") || strings.HasPrefix(d, "https://") {
95+
errs = append(errs, fmt.Errorf("couldn't parse dependency %q: http dependencies not yet supported", d))
96+
continue
97+
} else if strings.HasPrefix(d, ".") {
98+
errs = append(errs, fmt.Errorf("couldn't parse dependency %q: file paths not yet supported", d))
99+
continue
94100
} else if strings.HasPrefix(d, "@") {
95101
parsed := regex.FindStringSubmatch(d)
96102
if len(parsed) < 2 {
@@ -135,8 +141,18 @@ func (p *DamlPackage) computeResolvedDependencies(defaultLocation *ArtifactLocat
135141
FullUrl: u,
136142
}
137143
} else {
138-
// non-remote dependency
139-
resolved[d] = nil
144+
// builtin libs (like "daml-script")
145+
146+
s := "builtin://" + d
147+
u, err := url.Parse(s)
148+
if err != nil {
149+
errs = append(errs, fmt.Errorf("couldn't parse full url %q for dependency %q: ", s, d))
150+
continue
151+
}
152+
resolved[d] = &ResolvedDependency{
153+
Location: nil,
154+
FullUrl: u,
155+
}
140156
}
141157
}
142158

‎pkg/packagelock/lock.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package packagelock
22

33
import (
44
"fmt"
5+
"net/url"
56
"os"
67
"path/filepath"
78
"strings"
@@ -11,6 +12,7 @@ import (
1112
"daml.com/x/assistant/pkg/schema"
1213
"daml.com/x/assistant/pkg/utils/stringset"
1314
"github.com/goccy/go-yaml"
15+
"github.com/samber/lo"
1416
"oras.land/oras-go/v2/registry"
1517
)
1618

@@ -28,13 +30,21 @@ type PackageLock struct {
2830
}
2931

3032
type Dar struct {
31-
URI string `yaml:"uri"`
32-
Digest string `yaml:"digest,omitempty"`
33-
Path string `yaml:"path"`
33+
URI *url.URL `yaml:"uri"`
34+
Digest string `yaml:"digest,omitempty"`
35+
Path string `yaml:"path"`
3436

3537
Dependency *damlpackage.ResolvedDependency `yaml:"-"`
3638
}
3739

40+
//func (d *Dar) getURI() string {
41+
// if d.Dependency.FullUrl.Scheme == "builtin" {
42+
// return d.Dependency.FullUrl.Host
43+
// }
44+
//
45+
// return d.Dependency.FullUrl.String()
46+
//}
47+
3848
func ReadPackageLock(filePath string) (*PackageLock, error) {
3949
abs, err := filepath.Abs(filePath)
4050
if err != nil {
@@ -70,7 +80,12 @@ func ReadPackageLockContents(contents []byte) (*PackageLock, error) {
7080
func (l *PackageLock) toDiffableMap() (map[string]stringset.StringSet, error) {
7181
m := map[string]stringset.StringSet{}
7282
for _, d := range l.Dars {
73-
ref, err := registry.ParseReference(strings.TrimPrefix(d.URI, "oci://"))
83+
if d.URI.Scheme == "builtin" {
84+
m["builtin://"] = make(stringset.StringSet).Add(d.URI.Host)
85+
continue
86+
}
87+
88+
ref, err := registry.ParseReference(strings.TrimPrefix(d.URI.String(), "oci://"))
7489
if err != nil {
7590
return nil, err
7691
}
@@ -110,6 +125,13 @@ func (l *PackageLock) isInSync(expected *PackageLock) (bool, error) {
110125
return false, nil
111126
}
112127

128+
if k == "builtin://" {
129+
if !lo.ElementsMatch(lo.Keys(xs), lo.Keys(ys)) {
130+
return false, nil
131+
}
132+
continue
133+
}
134+
113135
for x := range xs {
114136
if ocilister.IsFloaty(x) {
115137
continue

‎pkg/packagelock/lock_test.go‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,28 @@
11
package packagelock
22

33
import (
4+
"net/url"
45
"testing"
56

67
"github.com/stretchr/testify/assert"
78
"github.com/stretchr/testify/require"
89
)
910

10-
func mk(uris ...string) *PackageLock {
11+
func mk(t *testing.T, uris ...string) *PackageLock {
1112
pl := &PackageLock{}
1213
for _, u := range uris {
13-
pl.Dars = append(pl.Dars, &Dar{URI: u})
14+
p, err := url.Parse(u)
15+
require.NoError(t, err)
16+
pl.Dars = append(pl.Dars, &Dar{URI: p})
1417
}
1518
return pl
1619
}
1720

18-
func TestDiff(t *testing.T) {
21+
func TestIsInSync(t *testing.T) {
22+
23+
_, err := url.Parse("builtin://asdf")
24+
require.NoError(t, err)
25+
1926
tests := []struct {
2027
name string
2128
expected *PackageLock
@@ -24,34 +31,41 @@ func TestDiff(t *testing.T) {
2431
}{
2532
{
2633
name: "no diff",
27-
expected: mk("oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
28-
existing: mk("oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
34+
expected: mk(t, "oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
35+
existing: mk(t, "oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
2936
want: true,
3037
},
3138
{
3239
name: "only removed",
33-
expected: mk("oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
34-
existing: mk("oci://example1.com/a:latest"),
40+
expected: mk(t, "oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
41+
existing: mk(t, "oci://example1.com/a:latest"),
3542
want: false,
3643
},
3744
{
3845
name: "only added",
39-
expected: mk("oci://example1.com/a:latest"),
40-
existing: mk("oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
46+
expected: mk(t, "oci://example1.com/a:latest"),
47+
existing: mk(t, "oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
4148
want: false,
4249
},
4350
{
4451
name: "added and removed",
45-
expected: mk("oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
46-
existing: mk("oci://example2.com/b:1.2.3", "oci://example3.com/c:4.5.6"),
52+
expected: mk(t, "oci://example1.com/a:latest", "oci://example2.com/b:1.2.3"),
53+
existing: mk(t, "oci://example2.com/b:1.2.3", "oci://example3.com/c:4.5.6"),
4754
want: false,
4855
},
4956
{
5057
name: "only floaty diff",
51-
expected: mk("oci://example2.com/b:latest"),
52-
existing: mk("oci://example2.com/b:1.2.3"),
58+
expected: mk(t, "oci://example2.com/b:latest", "builtin://daml-script"),
59+
existing: mk(t, "oci://example2.com/b:1.2.3", "builtin://daml-script"),
5360
want: true,
5461
},
62+
63+
{
64+
name: "builtin diff",
65+
expected: mk(t, "oci://example2.com/b:latest", "builtin://daml-script"),
66+
existing: mk(t, "oci://example2.com/b:1.2.3", "builtin://foo"),
67+
want: false,
68+
},
5569
}
5670

5771
for _, tt := range tests {

‎pkg/packagelock/locker.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"errors"
66
"fmt"
7+
"net/url"
78
"os"
89
"path/filepath"
910
"slices"
@@ -123,20 +124,25 @@ func (l *Locker) checkLockfile(expectedLockfile *PackageLock, lockfilePath strin
123124

124125
func (l *Locker) create(ctx context.Context, expected *PackageLock, lockfilePath string) (*PackageLock, error) {
125126
for _, d := range expected.Dars {
127+
if d.URI.Scheme == "builtin" {
128+
d.Path = d.URI.Host
129+
continue
130+
}
131+
126132
pulledDar, err := darpuller.New(l.config).PullDar(ctx, d.Dependency)
127133
if err != nil {
128134
return nil, err
129135
}
130136
d.Digest = pulledDar.Descriptor.Digest.String()
131137

132-
ref, err := registry.ParseReference(strings.TrimPrefix(d.URI, "oci://"))
138+
ref, err := registry.ParseReference(strings.TrimPrefix(d.URI.String(), "oci://"))
133139
if err != nil {
134140
return nil, err
135141
}
136142

137143
// TODO this doesn't work for @sha256 pinned refs
138144
resolvedRef := ":" + pulledDar.Version.String()
139-
d.URI = fmt.Sprintf("oci://%s/%s%s", ref.Registry, ref.Repository, resolvedRef)
145+
d.URI, _ = url.Parse(fmt.Sprintf("oci://%s/%s%s", ref.Registry, ref.Repository, resolvedRef))
140146
d.Path = pulledDar.DarFilePath
141147
}
142148

@@ -159,15 +165,15 @@ func computeExpectedLockfile(packageDirAbsPath string) (*PackageLock, error) {
159165
// TODO de-duplicate p.ResolvedDependencies first
160166
expectedDars := lo.MapToSlice(p.ResolvedDependencies, func(_ string, d *damlpackage.ResolvedDependency) *Dar {
161167
return &Dar{
162-
URI: d.FullUrl.String(),
168+
URI: d.FullUrl,
163169
Dependency: d,
164170

165171
// TODO diff digests too
166172
// Digest:
167173
}
168174
})
169175
slices.SortFunc(expectedDars, func(a, b *Dar) int {
170-
return strings.Compare(a.URI, b.URI)
176+
return strings.Compare(a.URI.String(), b.URI.String())
171177
})
172178

173179
return &PackageLock{

‎pkg/testutil/testdata/simple-multi-package/a/daml.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
dependencies:
2+
- daml-script
23
- "@digital-asset/meep:1.2.3"
34

45

‎pkg/utils/stringset/stringset.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ package stringset
22

33
type StringSet map[string]struct{}
44

5-
func (ss StringSet) Add(s string) {
5+
func (ss StringSet) Add(s string) StringSet {
66
ss[s] = struct{}{}
7+
return ss
78
}
89

910
func (ss StringSet) Contains(s string) bool {

0 commit comments

Comments
 (0)