[manifest] Fill default field values for packages
`jiri manifest` doesn't include non-empty default values for package
elements, which seems to have been a simple oversight in forgetting to
call FillDefaults() for the packages.
CIPD package rollers need to know the default platforms for a package so
that they know which CIPD packages to check when searching for a common
tag to pin all the packages to.
Bug: 60085
Change-Id: I9ea319cb8d91553094faa15933eb6a63aaf198a8
Reviewed-on: https://fuchsia-review.googlesource.com/c/jiri/+/428042
Reviewed-by: Haowei Wu <haowei@google.com>
Commit-Queue: Oliver Newman <olivernewman@google.com>
diff --git a/cmd/jiri/manifest_test.go b/cmd/jiri/manifest_test.go
index c68de29..fc503d1 100644
--- a/cmd/jiri/manifest_test.go
+++ b/cmd/jiri/manifest_test.go
@@ -10,6 +10,7 @@
"strings"
"testing"
+ "fuchsia.googlesource.com/jiri/cipd"
"fuchsia.googlesource.com/jiri/jiritest"
)
@@ -41,7 +42,7 @@
historydepth="2"/>
</projects>
<packages>
- <package name="the_package"
+ <package name="the_package/${platform}"
version="the_package_version"
path="path/to/the_package"
internal="false" />
@@ -93,7 +94,7 @@
// Compare stdout to the expected value.
if strings.Trim(stdout, " \n") != expectedValue {
- t.Errorf("expected %s, got %s", expectedValue, stdout)
+ t.Errorf("expected %q, got %q", expectedValue, stdout)
}
}
@@ -220,28 +221,39 @@
t.Run("should read <package> attributes", func(t *testing.T) {
expectAttributeValue(t, []string{
- "-element=the_package",
+ "-element=the_package/${platform}",
"-template={{.Name}}",
testManifestFile.Name(),
},
- "the_package")
+ "the_package/${platform}")
expectAttributeValue(t, []string{
- "-element=the_package",
+ "-element=the_package/${platform}",
"-template={{.Version}}",
testManifestFile.Name(),
},
"the_package_version")
expectAttributeValue(t, []string{
- "-element=the_package",
+ "-element=the_package/${platform}",
"-template={{.Path}}",
testManifestFile.Name(),
},
"path/to/the_package")
+ var defaultPlatforms []string
+ for _, p := range cipd.DefaultPlatforms() {
+ defaultPlatforms = append(defaultPlatforms, p.String())
+ }
expectAttributeValue(t, []string{
- "-element=the_package",
+ "-element=the_package/${platform}",
+ "-template={{.Platforms}}",
+ testManifestFile.Name(),
+ },
+ strings.Join(defaultPlatforms, ","))
+
+ expectAttributeValue(t, []string{
+ "-element=the_package/${platform}",
"-template={{.Internal}}",
testManifestFile.Name(),
},
diff --git a/project/manifest.go b/project/manifest.go
index 558ffd0..a40755d 100644
--- a/project/manifest.go
+++ b/project/manifest.go
@@ -204,6 +204,11 @@
return err
}
}
+ for index := range m.Packages {
+ if err := m.Packages[index].FillDefaults(); err != nil {
+ return err
+ }
+ }
return nil
}
@@ -233,6 +238,11 @@
return err
}
}
+ for index := range m.Packages {
+ if err := m.Packages[index].unfillDefaults(); err != nil {
+ return err
+ }
+ }
return nil
}
@@ -552,16 +562,26 @@
// Package struct if it is not defined and path is using template.
func (p *Package) FillDefaults() error {
if cipd.MustExpand(p.Name) && p.Platforms == "" {
- for _, v := range cipd.DefaultPlatforms() {
- p.Platforms += v.String() + ","
- }
- if p.Platforms[len(p.Platforms)-1] == ',' {
- p.Platforms = p.Platforms[:len(p.Platforms)-1]
- }
+ p.Platforms = p.defaultPlatforms()
}
return nil
}
+func (p *Package) unfillDefaults() error {
+ if cipd.MustExpand(p.Name) && p.Platforms == p.defaultPlatforms() {
+ p.Platforms = ""
+ }
+ return nil
+}
+
+func (p *Package) defaultPlatforms() string {
+ var platforms []string
+ for _, v := range cipd.DefaultPlatforms() {
+ platforms = append(platforms, v.String())
+ }
+ return strings.Join(platforms, ",")
+}
+
// GetPath returns the relative path that Package p should be
// downloaded to.
func (p *Package) GetPath() (string, error) {
