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

Introduce struct with metadata to identify a dependency #283

Merged

Conversation

c0d1ngm0nk3y
Copy link
Contributor

related to #233 (comment)
related to #267 (comment)

Summary

This introduces a Metadata struct with all relevant properties to identify a dependency and still be descriptive.

Use Cases

This allow users (e.g. libjvm) to use this metadata as layer metadata and not the whole dependency. That would
a) Simplify comparison if there is a cache hit (special logic for deprecation_data)
b) Ensure that no cache miss happens because some metadata changed (e.g. url)

Checklist

  • I have viewed, signed, and submitted the Contributor License Agreement.
  • I have linked issue(s) that this PR should close using keywords or the Github UI (See docs)
  • I have added an integration test, if necessary.
  • I have reviewed the styleguide for guidance on my code quality.
  • I'm happy with the commit history on this PR (I have rebased/squashed as needed).

@c0d1ngm0nk3y
Copy link
Contributor Author

@paketo-buildpacks/java-maintainers Is it worth to continue on this and the related PRs or doesn't it have anyway no real change to get merged. I started originally with #267 in v1 to simplify things. But after some feedback, I welt for v2. I still things it could simplify things and get rid of a workaround.

@c0d1ngm0nk3y
Copy link
Contributor Author

@paketo-buildpacks/java-maintainers Would be great to know how/if to continue with that before I check the conflicts.

@dmikusa
Copy link
Contributor

dmikusa commented Oct 5, 2023

Sorry, I've been working on some other things and meant to get back around to this one.

a) Simplify comparison if there is a cache hit (special logic for deprecation_data)
b) Ensure that no cache miss happens because some metadata changed (e.g. URL)

Do these use cases still stand up since we've merged #266 into v2?

I'm OK merging this into v2, but I'd like to confirm the use cases listed. and also see how this would change the API. Could you post a diff or before/after of what a buildpack would look like with this change? Or is this completely self contained and would not require changes to consuming buildpacks? Disregard this last bit, I saw your other PR to libjvm that has this info.

@dmikusa dmikusa added type:enhancement A general enhancement semver:major A change requiring a major version bump labels Oct 5, 2023
@c0d1ngm0nk3y
Copy link
Contributor Author

Do these use cases still stand up since we've merged #266 into v2?

I think we have to distinguish between two aspects, but feel free to correct me if I am wrong.

a) (DependencyCache)When a dependency is downloaded, it will first check if it is already available. Instead of comparing the whole metadata, checking sha256 is enough. This is addressed by #266 This is for example relevant for offline buildpacks.

b) (LayerContributor) When reusing a layer from an existing image, the metadata is checked. This change allows other to not add the complete metadata, but only the relevant parts. see libjvm/pull/312. This pr would be step 1/3 like describe in this comment.

@dmikusa dmikusa merged commit 8365f81 into paketo-buildpacks:release-2.x Oct 23, 2023
3 checks passed
@c0d1ngm0nk3y c0d1ngm0nk3y deleted the dep-metadata-v2 branch October 27, 2023 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:major A change requiring a major version bump type:enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants