-
Notifications
You must be signed in to change notification settings - Fork 8
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 additional_buildpack_binary_path
macro
#320
Conversation
This seems fine to me for now, since:
|
::std::env::var("CNB_BUILDPACK_DIR") | ||
.map(::std::path::PathBuf::from) | ||
.expect("Could not read CNB_BUILDPACK_DIR environment variable") | ||
.join(".libcnb-cargo") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a comment here and in libcnb-cargo
about keeping these path constants in sync?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's quite obvious that these need to be in sync when you come across this code. 🤔
Follow-up to: #314
Resolves the path to an additional buildpack binary by Cargo target name when the buildpack is packaged with
libcnb-cargo
orlibcnb-test
.This can be used to copy additional binaries to layers or use them for exec.d. To add an additional binary to a buildpack, add a new file with a main function to
bin/
. Cargo will automatically configure it as a binary target with the name of file.Note: This only works properly if the buildpack is packaged with
libcnb-cargo
/libcnb-test
.Implementation notes:
This follows the pattern for the existing proc macro where the proc macro itself only concerns itself with the logic that strictly requires a proc macro. In this case, verifying that the given binary target name is valid. Additional logic is implemented in a regular macro (here: additional_buildpack_binary_path).
Strictly speaking, the macro should live in
libcnb-cargo
as it only works when a buildpack is packaged withlibcnb-cargo
. However, sincelibcnb-cargo
already is a library for implementing packaging, adding a dependency to it from an actual buildpack seemed weird. It also would would hinder discoverability. Therefore, I decided to add it tolibcnb
proper.I'm slightly unhappy with the hardcoding of the path to the
additional-bin
directory inlibcnb
. I considered adding shared code betweenlibcnb-cargo
andlibcnb
that concerns itself with the construction of that path. To make that work, we would've needed to create a new shared crate to avoid circular dependencies (i.e.libcnb-common
). For now, this seems to be overkill but should be reconsidered when we have more use-cases for a shared crate.Closes GUS-W-10695287