CEP: Jinja functionality in v1 recipes
In https://github.com/conda/conda-build/issues/5053 you had said:
I do think that for the future (ie. rattler-build) we'll have an upcoming CEP to define this feature better.
Is this that CEP? In any case, I don't see {{ stdlib(...) }} in the diff, and I think it needs to be covered in some way, given that we're now rolling this out at scale.
Yep, this is the one. Do you want to have it as a separate function or rolled into compiler? I prefer to roll it into compiler...
Yep, this is the one. Do you want to have it as a separate function or rolled into
compiler? I prefer to roll it intocompiler...
I'd prefer a separate function, because it's less magic to have to understand (i.e. "why do I need to specify c_stdlib_version and how does that affect my build environment?" is much easier to answer if there is a {{ stdlib("c") }} under build:); also, the coupling of compiler and stdlib versions is entirely incidental (at least for C), so I'd like to not conflate them conceptually.
My points:
- There are already two variables for compilers (
c_compilerandc_compiler_version). Adding additional 2 variables (that are non-mandatory) is fine IMO, especially if it's well-documented. - It complicates things for beginners. I think recipes would continue to work without adding the stdlib (correct?). So it's something they should add but don't have to add. For
compilerit's clear - the recipe will not work without a compiler. - In most cases, it will always be added when there is a compiler.
I can imagine two behaviors:
- We control the stdlib behavior from the
variant_config. This means, if the variant config contains entries forc_stdlib/c_stdlib_versionwe use them from thecompilerfunction. - We add additional arguments to the jinja function, so that
compiler('c', stdlib='glibc').
Note that while you are correctly pointing out a dislike about magical behavior, we are printing a lot more information, clearly displayed, with rattler-build. So we can make it obvious where the stdlib comes from (we already point out that the clangxx_osx-arm64 package comes from the compiler, for example).
And lastly, I just think it's a matter of writing really good documentation.
I think recipes would continue to work without adding the stdlib (correct?).
Define "work". When we increase the c_stdlib_version on linux and macos later this year, the artefacts produced by a recipe without {{ stdlib("c") }} would continue to work on modern systems, but would be broken on systems with a too-old C standard library, because it's through the meta-packages pulled in by stdlib that we set the correct run-exports in the artefact metadata.
It would only affect very old systems, but I'd still consider that broken.
So it's something they should add but don't have to add. For
compilerit's clear - the recipe will not work without a compiler.
For me it's the opposite here - a compiled recipe needs (in 99.9% of cases) a C standard library just as much as it needs a compiler. Up until now this was just an ambient assumption built-in to our infrastructure, but I'd prefer people to learn this at the same time as the necessity of {{ compiler(...) }} - the two go hand in hand (as implied by having the same "c" argument, and the same way the keys are constructed), even though the relevant versions are completely independent from each other.
I can imagine two behaviors:
- We add additional arguments to the jinja function, so that
compiler('c', stdlib='glibc').
I really dislike this, because it would replace the nicely universal
- {{ compiler('c') }}
- {{ stdlib('c') }}
with a horrible mess of platform-specific selectors (or however the syntax for that ends up looking like in the new recipe format):
- {{ compiler('c', stdlib='glibc') }} # [linux]
- {{ compiler('c', stdlib='macosx_deployment_target') }} # [osx]
- {{ compiler('c', stdlib='vs') }} # [win]
- We control the stdlib behavior from the
variant_config. This means, if the variant config contains entries forc_stdlib/c_stdlib_versionwe use them from thecompilerfunction.
I agree that this is technically feasible, and with good output and documentation would be a workable solution. When I say I prefer to keep {{ stdlib("c") }} explicit, this is not an all-or-nothing statement, but more a 70:30 kind of preference over the implicit handling through the compiler.
One reason that pushes this from a 60:40 (based on "less magic") to a 70:30 preference is that the C stdlib has a special role, in the sense that it can become necessary even without an explicit C compiler. In other words, we'd have to cover cases where a recipe with only {{ compiler("cxx") }} or {{ compiler("rust") }} also includes c_stdlib{,_version}.
@h-vetinari since we don't have a CEP for stdlib it might be good to define it here.
What are the default values for the different operating systems?
since we don't have a CEP for stdlib it might be good to define it here.
Sounds good!
What are the default values for the different operating systems?
I was under the impression that defaults here aren't necessarily a good thing? Perhaps even more than the compiler packages, the stdlib-metapackages don't have a "canonical" naming. Are you planning to set defaults for {{ compiler(...) }} as well? Looking at the diff here it doesn't seem like there are default being proposed?
In any case, here are the specs that we're using in conda-forge. They won't change substantially anymore, barring unforeseen events.
c_stdlib:
- sysroot # [linux]
- macosx_deployment_target # [osx]
- vs # [win]
m2w64_c_stdlib: # [win]
- m2w64-toolchain # [win] <-- might change to m2w64-sysroot in the future
c_stdlib_version: # [unix]
- 2.17 # [linux] <-- per July 2024
- 10.13 # [osx and x86_64]
- 11.0 # [osx and arm64]
I expanded the content a bit.
Some things that are left to decide:
- Limit the filters from Jinja to a default subset?
- Should we add a
split(str)filter (useful for version numbers, so that one can do"${{ version | split(".") | slice(0, 2) | join(".") }}or something along those lines. We could also create a more specialized function/filter for version string manipulation of this sort. - Should
envwork more like Pythonenv-
env["FOO"]instead ofenv.get("FOO") -
env.get("FOO", "default")instead ofenv.get_default("FOO", "default")
-
- Or should it be more like Github Actions
-
env.FOOinstead ofenv.get("FOO") -
env.FOO or "default"for default values
-
- Should
${{ compiler('c') }}evaluate directly togcc_linux-64or should there be an intermediate representation (as is now) - For
pin_subpackageit will be impossible to evaluate it directly (due to self-referential nature of the function). - Should we add
lower_boundandupper_boundto the pin functions? And how should they work (in conjunction withmin_pinandmax_pin)? - The final name for the
cmpfunction.
Limit the filters from Jinja to a default subset?
Should we add a
split(str)filter (useful for version numbers, so that one can do"${{ version | split(".") | slice(0, 2) | join(".") }}or something along those lines. We could also create a more specialized function/filter for version string manipulation of this sort.Should
envwork more like Pythonenv
env["FOO"]instead ofenv.get("FOO")env.get("FOO", "default")instead ofenv.get_default("FOO", "default")Or should it be more like Github Actions
env.FOOinstead ofenv.get("FOO")env.FOO or "default"for default values
I can't speak as much for the needs of package builders, but I will say I think any allowed JINJA syntax needs to minimized. It adds a significant level of complication in the development of automated tooling (namely package editing). But if we must have them, we should limit the syntax and make the rules of using the features very clear. The more variety available, the more developers will use the tools in "clever" ways, the more breakages we will have when trying to automatically edit recipes.
@conda/steering-council
This vote falls under the "Enhancement Proposal Approval" policy of the conda governance policy, please vote and/or comment on this proposal at your earliest convenience.
It needs 60% of the Steering Council to vote yes to pass.
To vote, please leave yes, no or abstain as comments below.
If you have questions concerning the proposal, you may also leave a comment or code review.
This vote will end on 2024-07-16, End of Day, Anywhere on Earth (AoE). This is an extended voting period due to summer holiday time in the Northern Hemisphere.
yes
Please use the following form to vote:
@xhochy (Uwe Korn)
- [x] yes
- [ ] no
- [ ] abstain
@cj-wright (Christopher J. 'CJ' Wright)
- [ ] yes
- [ ] no
- [ ] abstain
@mariusvniekerk (Marius van Niekerk)
- [x] yes
- [ ] no
- [ ] abstain
@goanpeca (Gonzalo Peña-Castellanos)
- [x] yes
- [ ] no
- [ ] abstain
@chenghlee (Cheng H. Lee)
- [ ] yes
- [x] no
- [ ] abstain
@ocefpaf (Filipe Fernandes)
- [x] yes
- [ ] no
- [ ] abstain
@marcelotrevisani (Marcelo Duarte Trevisani)
- [ ] yes
- [ ] no
- [ ] abstain
@msarahan (Michael Sarahan)
- [x] yes
- [ ] no
- [ ] abstain
@mbargull (Marcel Bargull)
- [ ] yes
- [ ] no
- [x] abstain
@jakirkham (John Kirkham)
- [x] yes
- [ ] no
- [ ] abstain
@jezdez (Jannis Leidel)
- [x] yes
- [ ] no
- [ ] abstain
@wolfv (Wolf Vollprecht)
- [x] yes
- [ ] no
- [ ] abstain
@jaimergp (Jaime Rodríguez-Guerra)
- [x] yes
- [ ] no
- [ ] abstain
@kkraus14 (Keith Kraus)
- [x] yes
- [ ] no
- [ ] abstain
@baszalmstra (Bas Zalmstra)
- [x] yes
- [ ] no
- [ ] abstain
@marcelotrevisani @jakirkham @mbargull @CJ-Wright last chance to vote - you can also choose abstain which would still help with the quorum.
NP on the title of this PR: Since this is schema_version: 1 I've been referring to this as the V1 recipe format and the pre-CEP-13 format as V0. Do we want to keep consistent with that nomenclature?
I'm also not sure if it is worth codifying somewhere to prevent confusion. Maybe it is also worth it to add a section to the top of the CEP that specifies which schema_version a CEP belongs to, to prevent future confusion?
For the record: voted "no" because while I like the ideas presented in this CEP, I don't think the CEP as written is ready to be adopted as a specification. IMO, various unanswered questions/comments need to be addressed before adoption (e.g., top-down evaluation of the context object).
The vote has been closed with the following result:
Total voters: 15 (valid: 13 = 86.67%)
Yes votes (11 / 84.62%):
- @xhochy (Uwe Korn)
- @mariusvniekerk (Marius van Niekerk)
- @goanpeca (Gonzalo Peña-Castellanos)
- @ocefpaf (Filipe Fernandes)
- @msarahan (Michael Sarahan)
- @jakirkham (John Kirkham)
- @jezdez (Jannis Leidel)
- @wolfv (Wolf Vollprecht)
- @jaimergp (Jaime Rodríguez-Guerra)
- @kkraus14 (Keith Kraus)
- @baszalmstra (Bas Zalmstra)
No votes (1 / 7.69%)):
- @chenghlee (Cheng H. Lee)
Abstain votes (1 / 7.69%):
- @mbargull (Marcel Bargull)
Not voted (2):
- @cj-wright (Christopher J. 'CJ' Wright)
- @marcelotrevisani (Marcelo Duarte Trevisani)
Invalid votes (0):
We reached quorum, and enough YES votes for this CEP to be accepted. 🎉