cloudfoundry / ibm-websphere-liberty-buildpack

IBM WebSphere Application Server Liberty Buildpack
Apache License 2.0
72 stars 190 forks source link

cflinuxfs4 support #555

Closed krismarc closed 1 year ago

krismarc commented 1 year ago

Dear @kevin-ortega and all other contributors,

by any chance, do you know if current version of the buildpack will be compatible with cflinuxfs4? Other buildpacks are getting prepared. Not sure what's the effort (if any) to get this done for Liberty one. (if I am not mistaken, there were no changes required for the move from cflinuxfs2 to cflinuxfs3) eg. https://github.com/cloudfoundry/java-buildpack/releases/tag/v4.52

Unfortunately, due to my access limitations I can't perform any tests until the new stack is available in our environment (not in my scope). Would you be able to give it a try and share your feedback?

Best regards, K.M.

kevin-ortega commented 1 year ago

I anticipate that the buildpack will run on cflinuxfs4 without any changes. We did not make any modifications moving from cflinuxfs2 to cflinuxfs3.

Unfortunately, I do not have access to a cflinuxfs4 environment and am not able to validate the buildpack will run. It is not likely IBM Cloud will install cflinuxfs4 but that is only speculation on my part.

While the buildpack itself was unmodified when cflinuxfs3 came out, there was one change I did have to make in the script used to build an admin buildpack.

image

I suspect a similar change will need to be made to work with cflinuxfs4, bionic -> jammy.

kevin-ortega commented 1 year ago

I did confirm with the IBM CLOUD CF folks, there are no plans to add cflinuxfs4

krismarc commented 1 year ago

Hi @kevin-ortega,

I just started to work on migration to cflinuxfs4. Thx for your last tip, however, it's not enough. The new stack is shipped with newer Ruby and that one has deprecated some functions eg. URI.escape.

DEBUG -- /tmp/buildpacks/4673edf2538a200f257ae471aca83576/lib/liberty_buildpack/buildpack.rb:51:in `rescue in drive_buildpack_with_logger': Exception #<RuntimeError: IBM JRE error: undefined method `escape' for URI:Module> backtrace:

https://stackoverflow.com/questions/68174351/undefined-method-escape-for-urimodule https://github.com/ruby/uri/commit/61c6a47ebf1f2726b60a2bbd70964d64e14b1f98

I've replaced it with CGI.escape. Not sure if that's fully correct one. However, this seems to work for us.

https://github.com/cloudfoundry/ibm-websphere-liberty-buildpack/blob/988eca77da13c54527b56bc5971b50adf326a93c/lib/liberty_buildpack/util/cache/cached_file.rb#L36

https://github.com/cloudfoundry/ibm-websphere-liberty-buildpack/blob/988eca77da13c54527b56bc5971b50adf326a93c/resources/download_buildpack_cache.rb#L169

btw. CGI.escape accepts only 1 argument, so I've deleted the second.

ps. you could replace hardcoded value of ubuntu code version with this: system("lsb_release -c -s") in repository_root function.

Best regards, K.M.

kevin-ortega commented 1 year ago

@krismarc Thank you for the heads up on cflinuxfs4. I'm still struggling trying to figure out how I'm going to test this buildpack with cflinuxfs4 since CF on IBM Cloud is not going to upgrade.

I think I need to keep the hardcoded ubuntu version because building the buildpack can occur on any machine, not just ubuntu. I do all my development work on macos and lsb_release doesn't exist. I will likely cut a release that has bionic so people building for cflinuxfs2 can continue if/when I add support for cflinuxfs4.

krismarc commented 1 year ago

@kevin-ortega, what's their problem? I'd understand having this pushed to become default stack. However, having it optional shouldn't be a pain for them? or just tell them to enable it in their lab/exp and don't even set it to default one. This should be enough for testing.

I think I need to keep the hardcoded ubuntu version because building the buildpack can occur on any machine, not just ubuntu. I do all my development work on macos and lsb_release doesn't exist. I will likely cut a release that has bionic so people building for cflinuxfs2 can continue if/when I add support for cflinuxfs4.

Ok, I though it's just needed during execution/interpretation when buildpack is actually used within CF staging.