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

feat: delay jinja evaluation for script #894

Merged
merged 2 commits into from
Sep 24, 2024

Conversation

wolfv
Copy link
Member

@wolfv wolfv commented May 31, 2024

This delays the Jinja evaluation for the script contents. That means that we can also inject a lot of variables into the script by using jinja.

Such as ${{ PREFIX }} or ${{ PYTHON }}.

For example the following two examples work on Windows and on Unix:

build:
  script: cargo install --root ${{ PREFIX }} .
  # or
  script: ${{ PYTHON }} -m pip install .

These variables are also available as environment variables. However, since we are using cmd.exe and bash on the different operating systems, the syntax to expand environment variables is not the same (%PREFIX% vs $PREFIX).

Alternatives we considered (or are considering):

  • add a jinja function env("PREFIX") that expands to either %PREFIX% or $PREFIX depending on execution context. Might not work well with different interpreters.
  • automatically convert $PREFIX to %PREFIX% as a string replacement on cmd.exe

TODO

  • missing values from context
  • Adjust the recipe rendering CEP because now we need the variables from the Context later as well

@wolfv wolfv changed the title delay jinja evaluation for script feat: delay jinja evaluation for script May 31, 2024
@wolfv
Copy link
Member Author

wolfv commented Jun 2, 2024

Something that we might want to think about is that this does not solve the case for e.g. CMake or Meson that both have ${MESON_ARGS} or ${CMAKE_ARGS} defined in their activation scripts that make them work quite OK cross-platform.

@wolfv
Copy link
Member Author

wolfv commented Jun 2, 2024

We could still add the env function, e.g.:

script:
  - cmake ${{ env("CMAKE_ARGS") }} .
  - ninja install

@wolfv
Copy link
Member Author

wolfv commented Jun 6, 2024

Or we change the way we do activation completely (to work like pixi) and add everything to the Jinja context :)

@baszalmstra
Copy link
Contributor

Or we change the way we do activation completely (to work like pixi) and add everything to the Jinja context :)

To me, from a user perspective, this makes the most sense. Adding all the environment variables after activation to the env jinja object that is.

It might also be slightly confusing to users which variables are available when but at least we provide a consistent way to access environment variables.

@isuruf
Copy link

isuruf commented Sep 20, 2024

Would this have an expanded path hard-coded when the recipe is rendered? That would make rebuilding a conda package like with reproducible-builds impossible.

@wolfv
Copy link
Member Author

wolfv commented Sep 20, 2024

Actually, it's a requirement for the reproducible builds to build the packages in the exact same prefix regardless, so that's not really an issue. A lot of packages ship files with the prefix in some text files (such as .pc files etc.) and e.g. Debian also builds packages in the same folder always afaik. So I hope that being stable against changes in the filesystem location is not a requirement for reproducible builds.

@wolfv wolfv merged commit f5b4b11 into prefix-dev:main Sep 24, 2024
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants