Skip to content
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

Decouple LayerEnv from implicit CNB layer env rules #842

Open
Malax opened this issue Jul 18, 2024 · 0 comments
Open

Decouple LayerEnv from implicit CNB layer env rules #842

Malax opened this issue Jul 18, 2024 · 0 comments
Labels
enhancement New feature or request libcnb

Comments

@Malax
Copy link
Member

Malax commented Jul 18, 2024

LayerEnv is a representation of environment modifications as per the CNB spec. There are also some implicit changes to the environment in the CNB spec to automatically add $layer/bin, $layer/lib, $layer/include and $layer/pkgconfig for each layer.

Currently, LayerEnv::read_from_layer_dir does not only read the explicit values from disk, but also adds the implicit ones from the spec. One can make an argument that this behaviour is correct if we define the implicit values as always present and overwritten by explicit rules, but there are some issues with the current implementation in practice.

The simplest case would be reading LayerEnv from disk and then persisting it to disk without changes. This currently leads to the situation that all the implicit values are written to disk explicitly, changing the state. This is unexpected behaviour and may lead to subtle bugs.

This issues proposes a solution to the problem by removing all handling of the implicit values from LayerEnv. This would make LayerEnv a strict representation of what is present on disk without involving "magic".

However, buildpacks will often need a LayerEnv that does include the explicit entries for subsequent logic, most commonly invoking binaries stored in $layer/bin by searching PATH. We still should make it easy to modify an existing Env (not LayerEnv!) as if the layer was processed by the CNB lifecycle.

For this use-case, we can provide a function on LayerRef with a signature like this:

fn apply_layer_env(&self, &mut Env)

This function would use the LayerEnv data from disk but also the implicit rules defined in the CNB spec. To clarify, read_layer_env would still only return the data from disk, cleanly separating both concerns.

In addition, we should rename LayerEnv to EnvDelta (or similar), clarifying the meaning of the represented data. In addition, LayerRef::write_env would be renamed to LayerRef::write_env_delta. The same applies to LayerRef::read_env_delta.

@Malax Malax changed the title Rename LayerEnv to EnvDelta and steam-line its usage Decouple LayerEnv from implicit CNB layer env rules Jul 18, 2024
@edmorley edmorley added enhancement New feature or request libcnb labels Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request libcnb
Projects
None yet
Development

No branches or pull requests

2 participants