libcnb.rs icon indicating copy to clipboard operation
libcnb.rs copied to clipboard

`BuildPlanBuilder::or()` can add empty `Or` entries

Open edmorley opened this issue 3 years ago • 0 comments

BuildPlanBuilder is meant to be used roughly like so:

let build_plan = BuildPlanBuilder::new()
    .provides("python")
    .provides("pip")
    .or()
    .provides("python")
    .build();

With the resultant build plan eventually being serialised to the TOML format here: https://github.com/buildpacks/spec/blob/main/buildpack.md#build-plan-toml

However if the builder API is mis-used like so:

let build_plan = BuildPlanBuilder::new().or().or().build();

Then the resultant BuildPlan contains empty Or entries, like so:

BuildPlan { provides: [], requires: [], or: [Or { provides: [], requires: [] }, Or { provides: [], requires: [] }] }

The empty requires and provides Vecs are not serialised, however or is:

[[or]]

[[or]]

Whilst this use of the API is clearly not correct, it feels like we should:

  1. Have BuildPlanBuilder::or() skip the push_back() iff both current_provides and current_requires is empty
  2. Consider adjusting BuildPlan to more accurately represent the data model. eg: Should the fields all be an Option?

edmorley avatar Feb 23 '22 21:02 edmorley