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 PLATFORMS var for offline bundling #991

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

AnnaAAL
Copy link

@AnnaAAL AnnaAAL commented Jan 18, 2023

As discussed in #986 there's a need to have PLATFORMS var to allow selecting between bionic and jammy for cflinuxfs3 and cflinuxfs4 stacks.

Allowing usage of different platforms for offlinization bundling of the buildpack.
Add information to the Offline package section for PLATFORMS variable.
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 18, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@AnnaAAL
Copy link
Author

AnnaAAL commented Jan 24, 2023

Hi @dmikusa @pivotal-david-osullivan, would you consider this suggestion for fix to #986 ? With this proposal there's no need to compile both bionic and jammy, as the buildpack size is getting larger.

@dmikusa
Copy link
Contributor

dmikusa commented Jan 24, 2023

Thanks for putting this together! I think it's a great solution, and after talking with @pivotal-david-osullivan we can definitely include it.

The feedback we have is that we'd like to see a different default value applied. We'd prefer, for the time being, to have the default be both bionic and jammy.

David also did some testing and found that if thePLATFORM env is just empty then the default value is not applied. Checking against platform_var.empty? would be a good idea. If it's empty, assign the default.

You might also want to trim whitespace after you split on the ,. I didn't try it, but if someone enters bionic, jammy the extra space might cause issues.

Thanks again for the PR!

@pivotal-david-osullivan
Copy link
Contributor

Hi @AnnaAAL, thanks again for the contribution! Are you able to make the adjustments suggested above to handle empty values/whitespace so we can get this merged?

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.

3 participants