conda / ceps

Conda Enhancement Proposals
Creative Commons Zero v1.0 Universal
20 stars 24 forks source link

add cep for definition of new keys & values in build format #56

Closed wolfv closed 9 months ago

dhirschfeld commented 1 year ago

One thing that I would love to have is an env_vars section which could then be used to automatically create activate and deactivate scripts for all shells which would take care of backing up existing values and setting the new ones on activation and restoring the backed up versions on deactivation.

e.g.

env_vars:
  - DOTNET_ROOT=$CONDA_PREFIX/lib/dotnet
  - DOTNET_TOOLS=$CONDA_PREFIX/lib/dotnet/tools

Potentially a big enough change to deserve it's own CEP though 🤔

wolfv commented 1 year ago

@baszalmstra did some awesome work at making a pydantic model for the schema presented in this PR. What's awesome is that it allows us to get feedback and inline-help in the VSCode editor (check out this video) with all the red squiggly lines:

https://github.com/conda-incubator/ceps/assets/885054/54d247b7-980c-4854-8ecc-3730da23c36d

The repository for the pydantic file and the JSON schema is here: https://github.com/prefix-dev/recipe-format

If you start a new YAML file in VSCode and add the following line to the top, you should have it, too:

# yaml-language-server: $schema=https://raw.githubusercontent.com/prefix-dev/recipe-format/main/schema.json

We also changed a couple of definitions. Most notably the script_env is now:

- script_env:
    # list of environment variables to pass through to the script
    passthrough: [string]
    # dictionary of key/value of environment variables to set in the script
    env: {string: string}
    # list of secrets to pass through from the environment
    secrets: [string]

In the about section we renamed a couple of fields:

description is now either 
description:
  file: path # path to read description from, e.g. ${SRC_DIR}/README.md

 -- or --  
description: string

renamed:

home -> homepage
doc_url -> documentation
dev_url -> repository

And we split the definition of multi-output recipes into a "complex recipe". Different rules apply to complex aka multi-output recipes:

AntoinePrv commented 1 year ago

Two more things I could think of:

Versioning the recipe format

We could have a metadata section but version is the only useful thing that comes to mind.

recipe:
  version: "1.0"

Parameters

Maybe that would be it's own CEP, but I think it could be valuable to pass values to the recipe from conda-build/boa/ratter-build. Two use case that come to mind:

carterbox commented 1 year ago

I agree with @AntoinePrv that a new format should be accompanied by a version metadata so that parsers can explicitly query which version of the format they are parsing.

dhirschfeld commented 1 year ago

I agree with @AntoinePrv that a new format should be accompanied by a version metadata

Perhaps named schema_version (or similar) as version is a bit overloaded.

wolfv commented 1 year ago

An alternative to having the schema_version in the file would be to encode the schema version in the filename (recipe.v1.yaml etc...). But I am fine with having an explicit schema version in the recipe (I think docker-compose does it, too).

jezdez commented 1 year ago

An alternative to having the schema_version in the file would be to encode the schema version in the filename (recipe.v1.yaml etc...). But I am fine with having an explicit schema version in the recipe (I think docker-compose does it, too).

I'm in favor of having it inside the file rather than the filename carry it

wolfv commented 1 year ago

I just added the definition for the ScriptEnv that we want to extend and make more explicit vs. what's currently in conda. There are three values in ScriptEnv:

For passthrough and secrets we can also warn the user if those are not available at build time.

wolfv commented 11 months ago

I just pushed a relatively large update to the build section. I categorized most options further. I think it helps giving structure to the large number of options. Would love to hear thoughts of others on this.

Downside is that it makes the migration a bit more complicated.

wolfv commented 11 months ago

Dear @conda-incubator/steering – this CEP has been open for a long time and has received quite some discussions.

@baszalmstra and I spent a day last week to go over the entire CEP and adjust some more names.

I feel that this CEP is ready to go. I wanted to give time until Wednesday for some last discussions, and open the vote on Wednesday right after the conda steering council call.

Please feel free to comment any last-minute requests beforehand! We are already hard at work implementing this in rattler-build.

danpetry commented 11 months ago

I understand that I've jumped in quite late with some comments and don't have an issue with moving to voting without them necessarily being fully addressed, FWIW.

I think they can be addressed in future iterations.

wolfv commented 11 months ago

I just renamed shared_libraries to dynamic_linking (a suggestion by @mbargull). I think that captures better that also other shared object (such as binaries) are concerned.

However, the other section was called link_options and I renamed that to install_options which is better differentiated.

Now remaining question is wether we should rename:

post_link_script -> post_install_script
pre_unlink_scipt -> pre_uninstall_script
pre_link_message -> pre_install_message

wdyt @mbargull ?

wolfv commented 11 months ago

Two more things that came up:

mbargull commented 11 months ago

However, the other section was called link_options and I renamed that to install_options which is better differentiated.

Now remaining question is wether we should rename:

post_link_script -> post_install_script
pre_unlink_scipt -> pre_uninstall_script
pre_link_message -> pre_install_message

wdyt @mbargull ?

Ah, sorry, missed the notification. There is some certain nomenclature in conda about "install"/"link" -- I'm not too certain how consistently it's used and if I remember correctly, but I think it was "install" meaning the whole installation process and "link" meaning only the file linking things. So, this would mean that we'd have some diverging nomenclature here.

... That being said, I actually wouldn't mind the name change since I think the "link" naming is probably too technical. (Other package managers also just call it post-install or something with similar semantics.)

wolfv commented 11 months ago

@mbargull I removed that section entirely because it is not clear why we have a pre/post-link options but no options for activation env vars or scripts.

The pre-post link stuff can also be achieved by dropping the files in the right locations manually during setup.

We'll come up with another proposal reintroducing these + proper options for activation scripts (I especially want to make it much simpler to set "simple" activation variables using the json activation variable thing).

mbargull commented 11 months ago

We'll come up with another proposal reintroducing these + proper options for activation scripts (I especially want to make it much simpler to set "simple" activation variables using the json activation variable thing).

Sounds good to me! I think it makes much sense to have these iterations on the format so we end up with a more coherent and user-friendly format. Could you add a comment on the removed options saying that replacements for them will be re-introduced later?

wolfv commented 10 months ago

@conda-incubator/steering

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-01-18, End of Day, Anywhere on Earth (AoE).

Note: This is an extended voting period to accommodate the start of the year and the complexity of the proposal.

baszalmstra commented 10 months ago

yes

wolfv commented 10 months ago

yes

jezdez commented 10 months ago

yes

mariusvniekerk commented 10 months ago

yes

future machine readable versions of the spec should also clearly state what is optional and what isn't. Presently the general state is basically everything is optional but not really stated as such.

msarahan commented 10 months ago

Yes. Nothing blocking. Just some comments.

xhochy commented 10 months ago

Yes

ocefpaf commented 10 months ago

Yes

wolfv commented 10 months ago

@CJ-Wright @goanpeca @chenghlee @marcelotrevisani @jakirkham @mbargull @kkraus14 @jaimergp this is a reminder to please vote with YES / NO / ABSTAIN on this CEP :)

mbargull commented 10 months ago

Could you add a comment on the removed options saying that replacements for them will be re-introduced later?

ping on this

goanpeca commented 10 months ago

Yes 👍🏼

jaimergp commented 10 months ago

Yes.

wolfv commented 10 months ago

@cj-wright @chenghlee @marcelotrevisani @kkraus14

if you could please vote (even if you vote with abstain), that would be great!!

wolfv commented 10 months ago

Forgot @jakirkham

marcelotrevisani commented 10 months ago

Yes

chenghlee commented 10 months ago

Left comments, but other than _maybe) making schema_version: N required, none of them would block a "yes" vote from me.

CJ-Wright commented 10 months ago

Yes I am making this comment solely in my personal capacity and am not conveying any rights to any intellectual property of any third parties.

wolfv commented 10 months ago

Hi everyone! I have counted the votes:

That means 13 people have voted. The council consists of 15 people.

That means that: 86% have voted, and 100% have voted yes! This CEP is accepted! 🎉

chenghlee commented 10 months ago

@wolfv: you can count me as a "yes". 😄