heroku / libcnb.rs

A framework for writing Cloud Native Buildpacks in Rust
BSD 3-Clause "New" or "Revised" License
34 stars 6 forks source link

Refactor cross compile assistance #769

Closed runesoerensen closed 7 months ago

runesoerensen commented 7 months ago

We currently support targeting x86_64-unknown-linux-musl and aarch64-unknown-linux-musl on macOS and Linux, but the cross compile assistance provided is inconsistent depending on the host's CPU architecture. This refactoring aims to improve that across all supported target_triple and host OS/architecture combinations by providing helpful assistance for supported scenarios.

It also:

runesoerensen commented 7 months ago

Apart from the mostly minor code comments, I was missing some context due to the fact there is not PR description and the title being very vague (what does it fix, is there an existing issue somewhere?) Again, this is a draft PR and I write my descriptions at the very end as well. If that was to come later - ignore my comment! :)

I have added a more elaborate description now with links to relevant issues. Merging this pull request should close those issues, but let me know if it's better to unlink them and manually close the issues with reference to this pull request!

Should I perhaps add a CHANGELOG entry as well?

There also seem to be some slight different in packages recommended. Have you verified those on the respective systems? For example, libc6-dev-arm64-cross is no longer mentioned. It's possible it is not needed, but from the information I have it could also be an oversight. :)

I initially removed the libc6-dev-arm64-cross package from the help text since, during testing, it was installed along with g++-aarch64-linux-gnu - but only as a recommended package - https://github.com/heroku/libcnb.rs/pull/769/commits/c5a046b0d7699e95351e3c25b03e5267130b9db7 adds it back in and explains why. The following commit adds the equivalent packages for amd64 cross compilation from aarch64 Linux

I'm not sure if musl-tools is still necessary, but also added that to both help texts. I will test further, but overall it's probably best to keep the changes introduced by this pull request minimal (and improve on the help text in another PR).