Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for anystack buildpacks #776

Merged
merged 1 commit into from
Jul 20, 2021
Merged

add support for anystack buildpacks #776

merged 1 commit into from
Jul 20, 2021

Conversation

tomkennedy513
Copy link
Collaborator

No description provided.

@tomkennedy513 tomkennedy513 force-pushed the anystack-support branch 2 times, most recently from b12a330 to 59883f2 Compare July 19, 2021 18:40
@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2021

Codecov Report

Merging #776 (a9c9e0b) into main (27ea452) will increase coverage by 0.86%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #776      +/-   ##
==========================================
+ Coverage   68.54%   69.41%   +0.86%     
==========================================
  Files          93      117      +24     
  Lines        4130     5074     +944     
==========================================
+ Hits         2831     3522     +691     
- Misses        951     1185     +234     
- Partials      348      367      +19     
Impacted Files Coverage Δ
pkg/cnb/buildpack_validation.go 92.59% <66.66%> (-7.41%) ⬇️
pkg/apis/build/v1alpha2/builder_validation.go 100.00% <0.00%> (ø)
pkg/apis/build/v1alpha2/image_validation.go 97.75% <0.00%> (ø)
...kg/apis/build/v1alpha2/cluster_store_validation.go 91.66% <0.00%> (ø)
pkg/apis/build/v1alpha2/image_builds.go 58.97% <0.00%> (ø)
pkg/apis/build/v1alpha2/source_resolver_types.go 0.00% <0.00%> (ø)
pkg/apis/build/v1alpha2/cluster_store_types.go 0.00% <0.00%> (ø)
pkg/apis/build/v1alpha2/register.go 0.00% <0.00%> (ø)
pkg/apis/build/v1alpha2/build_validation.go 88.70% <0.00%> (ø)
pkg/apis/build/v1alpha2/cluster_builder_types.go 0.00% <0.00%> (ø)
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 27ea452...a9c9e0b. Read the comment docs.

@@ -16,7 +17,9 @@ func (bl BuildpackLayerInfo) supports(buildpackApis []string, id string, mixins
}

for _, s := range bl.Stacks {
if s.ID == id {
iaAnystack := s.ID == "*" && api.MustParse(bl.API).Compare(api.MustParse("0.5")) >= 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MustParse will panic on a not parseable input which is actually an underlying user input. api.NewVersion is probably safer.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already pull in semver to parse buildpack versions. Could we use that instead of adding another dependency on the lifecycle?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ya good call

@tomkennedy513 tomkennedy513 force-pushed the anystack-support branch 2 times, most recently from 83504bf to ed7e942 Compare July 20, 2021 18:47
return err
}

isAnystack := s.ID == "*" && buildpackVersion.Compare(anyStackMinimumVersion) >= 0
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe pull this out to method?

@@ -15,8 +16,19 @@ func (bl BuildpackLayerInfo) supports(buildpackApis []string, id string, mixins
return errors.Errorf("unsupported buildpack api: %s, expecting: %s", bl.API, strings.Join(buildpackApis, ", "))
}

anyStackMinimumVersion, err := semver.NewVersion("0.5")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could MustParse this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And pull it out as var

@@ -46,3 +54,7 @@ func present(haystack []string, needle string) bool {
}
return false
}

func isAnystack(stackId string, buildpackVersion *semver.Version) bool {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@tomkennedy513 tomkennedy513 merged commit 4ed5f88 into main Jul 20, 2021
@tomkennedy513 tomkennedy513 deleted the anystack-support branch July 20, 2021 20:27
@matthewmcnew
Copy link
Collaborator

Closes #741

@dumez-k dumez-k linked an issue Jul 27, 2021 that may be closed by this pull request
@dumez-k dumez-k removed a link to an issue Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants