-
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Adds rattler-build recipe compatibility (and adds jolt-physics) #27008
Conversation
Hi! This is the friendly automated conda-forge-linting service. I was trying to look for recipes to lint for you, but couldn't find any. |
Hi! Thanks for your contribution to conda-forge. If these changes are intentional (and you aren't submitting a recipe), please attach a |
90c1d13
to
93640c6
Compare
93640c6
to
3879b6e
Compare
8f87e11
to
7a8c1d7
Compare
.ci_support/build_all.py
Outdated
found_cuda = False | ||
found_centos7 = False | ||
for folder in folders: | ||
meta_yaml = os.path.join(recipes_dir, folder, "meta.yaml") | ||
if os.path.exists(meta_yaml): | ||
has_meta_yaml = True | ||
with(open(meta_yaml, "r", encoding="utf-8")) as f: | ||
text = ''.join(f.readlines()) | ||
if 'cuda' in text: | ||
found_cuda = True | ||
if 'sysroot_linux-64' in text: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to get out of date very soon after stdlib('c')
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something you want me to change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, no, don't worry. Just noticed it while reading, so flagged it for the curious readers. Not part of the scope of this PR!
Also I found other workflows with references to meta.yaml that you might like to adjust for nicer UX / less confusion: staged-recipes/.github/workflows/correct_directory.yml Lines 30 to 36 in c4f138c
This block below reminds me we might want an example file for
The "new PR template":
|
7a8c1d7
to
9ac4efc
Compare
Thanks for the early review @jaimergp ! Much appreciated! The biggest issue I face atm is that |
9ac4efc
to
70b6472
Compare
There are some elements to consider, but I'm not fully sure of how this works, so just a couple ideas:
The code is here: staged-recipes/.ci_support/build_all.py Lines 154 to 171 in d998ffa
So you can either replicate that logic in rattler-build, or get ahold of |
456e36d
to
f83d4d5
Compare
f83d4d5
to
79ac2ca
Compare
I see two warnings in the macOS logs, maybe related to the SDK versions, deployment targets, etc etc. During build:
During analysis:
Not sure if they are related, or just noise, or whether important, but I thought I'd flag it in case there's a problem with how the CBCs are being loaded etc. |
Also, there's some funky slash dance in the artifact content listing on Windows, although I guess that's rattler-build specific, not the implementation in this setup.
|
.ci_support/build_all.py
Outdated
from itertools import chain | ||
import json | ||
from shutil import rmtree | ||
import tempfile | ||
import conda.base.context | ||
import conda.core.index | ||
import conda.resolve | ||
import conda_build.api | ||
import conda_build.variants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import block makes me wanna run isort
. Maybe in another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran ruff format
and isort
..
for example_recipe in EXAMPLE_RECIPE_FOLDERS: | ||
rmtree(os.path.join(recipes_dir, example_recipe), ignore_errors=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would become a problem locally if the user accidentally commits after debugging something, but I think we copy the staged-recipes folder before we start the script, so we are ok.
.ci_support/build_all.py
Outdated
# to rattler-build. | ||
with tempfile.NamedTemporaryFile(delete=False) as fp: | ||
fp.write(variant_config.encode('utf-8')) | ||
fp.close() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fp.close() |
No need; once you exit the context, it's done for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I close it because rattler-build will need to read it, but if the file is already open for writing it won't be able to open the file.
|
||
# Construct a temporary file where we write the combined variant config. We can then pass that | ||
# to rattler-build. | ||
with tempfile.NamedTemporaryFile(delete=False) as fp: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with tempfile.NamedTemporaryFile(delete=False) as fp: | |
with tempfile.NamedTemporaryFile(delete=False) as fp: |
Maybe we should register this with atexit
so it's removed once done.
# This example shows how to define a recipe using the new YAML based recipe format introduced by | ||
# CEP 13. | ||
|
||
# For more information about this format see: https://prefix-dev.github.io/rattler-build/latest/reference/recipe_file/ | ||
|
||
# Note: there are many handy hints in comments in this example -- remove them when you've finalized your recipe | ||
|
||
# Define variables in this section that you can use in other parts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this block should also mention the main differences across formats. Namely:
- No selectors, but if-then-else expressions (and link to docs)
- Jinja uses
${{ }}
# Add the line "skip: True # [py<35]" (for example) to limit to Python 3.5 and newer, or "skip: True # [not win]" to limit to Windows. | ||
# More info about selectors can be found in the conda-build docs: | ||
# https://docs.conda.io/projects/conda-build/en/latest/resources/define-metadata.html#preprocessing-selectors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update this block so it refers to the new syntax for selectors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added information to the top block.
Co-authored-by: jaimergp <[email protected]>
Co-authored-by: jaimergp <[email protected]>
WIP: This PR is a testbed to test support for rattler-build.
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).