Overhaul platform, build/detect API #2
Conversation
|
|
||
| fn main() { | ||
| libcnb::detect::cnb_runtime_detect(detect) | ||
| } | ||
|
|
||
| fn detect(_context: DetectContext<GenericPlatform>) -> DetectResult { | ||
| fn detect(_context: GenericDetectContext) -> Result<DetectOutcome, std::io::Error> { |
There was a problem hiding this comment.
Should we return our own Error type here?
There was a problem hiding this comment.
It is a custom error type defined here: https://github.com/Malax/libcnb/pull/2/files#diff-c11e72174294250d95d68429cbb5e9ec58c16ac64087d428736be92bd22cd688R24-R26
I wanted it to be flexible so buildpack authors don't need to know anything about our error types. We can also implement the BuildpackError for other Error libraries if we need to. Using io::Error here was basically just a test if that works as I expected.
Do you want me to change the example?
There was a problem hiding this comment.
nah, I don't care about custom error types on the example. I expect a buildpack author to be "sloppy" and do convenience things to start.
| vars: HashMap<OsString, String>, | ||
| } | ||
|
|
||
| impl PlatformEnv { |
There was a problem hiding this comment.
should we implement a fn vars or do you think people won't want an iterator?
There was a problem hiding this comment.
I think I stopped there since I couldn't properly emulate the std::env API and we discussed we wanted that. I'll leave this unimplemented for now.
* Refactor background timer logic The background timer prints dots to the UI while the buildpack performs some expensive operation. This requires we use threading and Rust really wants to be sure that we're not going to have data races and that our data is secure. The original version of the code relied on wrapping our IO in an Arc<Mutex<W>> so that we don't "lose" it when we pass it to the background thread. This worked, but was overly complicated based on my limited understanding of working with lifetimes and threads. This version instead relies on the ability to retrieve a struct when we join the thread to get the IO object back out. This reduces complexity that the BuildLog interface needs to know about. # This is the commit message #2: Inject tick state The tick values for the printer was hardcoded. Passing in the values allows us to remove the `style` module dependency and makes the tests simpler. # This is the commit message #3: Rename modules and add docs The prior name `print_dots` isn't technically valid anymore as it could be printing something else. I also moved the guard struct into it's own module to make it a bit safer (ensure that Option is always Some at creation time). # This is the commit message #4: Clarify intention that option is Some in code The debug_assert adds a stronger signal that the option must be Some when it is being constructed. * [Stacked PR] Remove warn_later from output The `warn_later` functionality relies on global state in a way that we're not thrilled about putting into everyone's build packs by default. This removes the feature, plain and simple.
Platformnow resides in its own file and was overhauled to make implementation of newPlatforms easier.GenericDetectContextandGenericBuildContextfor the default use-case. This removes generics for the standard use-case.BuildFromPathhas been removed since we don't need the additional complexity it adds.platform.rsnow has some rustdocdetectandbuildfunction API, both return aResult<_, BuildpackError>nowBuildpackErrortrait with implementation for allstd::err::Errortraits. We can add support for error handling frameworks by adding trait implementations for those frameworks.