paketo-buildpacks / libpak

An opinionated extension to the libcnb Cloud Native Buildpack Library
Apache License 2.0
15 stars 17 forks source link

Introduce struct with metadata to identify a dependency #283

Closed c0d1ngm0nk3y closed 10 months ago

c0d1ngm0nk3y commented 1 year ago

related to https://github.com/paketo-buildpacks/libpak/pull/233#issuecomment-1528865966 related to https://github.com/paketo-buildpacks/libpak/pull/267#discussion_r1284727496

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

c0d1ngm0nk3y commented 12 months ago

@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 commented 11 months ago

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

dmikusa commented 11 months ago

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 https://github.com/paketo-buildpacks/libpak/pull/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.

c0d1ngm0nk3y commented 11 months ago

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.