cloudfoundry / java-buildpack

Cloud Foundry buildpack for running Java applications
Apache License 2.0
435 stars 2.58k forks source link

Add PLATFORMS var for offline bundling #991

Open AnnaAAL opened 1 year ago

AnnaAAL commented 1 year ago

As discussed in https://github.com/cloudfoundry/java-buildpack/issues/986 there's a need to have PLATFORMS var to allow selecting between bionic and jammy for cflinuxfs3 and cflinuxfs4 stacks.

linux-foundation-easycla[bot] commented 1 year ago

CLA Signed

The committers listed above are authorized under a signed CLA.

AnnaAAL commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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?