Closed jaimergp closed 5 days ago
Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR () and found it was in an excellent condition.
Hi! This is the staged-recipes linter and I found some lint.
It looks like some changes were made outside the recipes/
directory. To ensure everything runs smoothly, please make sure that recipes are only added to the recipes/
directory and no other files are changed.
If these changes are intentional (and you aren't submitting a recipe), please add a maintenance
label to the PR.
File-specific lints and/or hints:
.ci_support/requirements.yaml
:
recipes/
directory..azure-pipelines/azure-pipelines-win.yml
:
recipes/
directory..ci_support/requirements.txt
:
recipes/
directory..gitattributes
:
recipes/
directory..scripts/run_osx_build.sh
:
recipes/
directory..gitignore
:
recipes/
directory..scripts/run_win_build.bat
:
recipes/
directory.Hi! This is the staged-recipes linter and your PR looks excellent! :rocket:
With pixi, importing the dependencies into a new toml and solving from scratch.
macOS needs 24s to deploy base
:
2024-10-02T22:14:28.5022520Z Downloading pixi 0.30.0
2024-10-02T22:14:29.6253360Z + echo 'Creating environment'
2024-10-02T22:14:52.5630520Z ##[group]Configuring conda
On Windows, 26s:
2024-10-02T22:14:34.0143231Z Downloading pixi 0.30.0
2024-10-02T22:14:36.9925817Z Creating environment
2024-10-02T22:15:50.0624206Z ##[group]Configuring conda
What are the original numbers before this PR?
Hi! This is the friendly automated conda-forge-linting service.
I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.
Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.
Hi! This is the friendly automated conda-forge-linting service.
I just wanted to let you know that I linted all conda-recipes in your PR () and found it was in an excellent condition.
Hi! This is the staged-recipes linter and I found some lint.
File-specific lints and/or hints:
environment.yaml
:
pixi project export conda-environment > environment.yaml
.Hi! This is the staged-recipes linter and your PR looks excellent! :rocket:
@jaimergp shall we merge here?
I've been thinking about the build-locally.py
experience and in the current logic I think we would end up provisioning environments twice if one runs pixi run build-locally
. I need to some sentinel variables to prevent that double installation.
Also I'm not sure how some folks would feel about having to use Pixi (although it's true that python build-locally.py
still works the same). The conda-smithy implementation is opt-in via conda-forge.yml
, so maybe it would be good to have it coexist with micromamba (I don't want to impose my personal preference, essentially). So my current compromise is something like:
pixi.toml
is also available for local debugging. If build-locally
is run via pixi
, then we would have planted some env vars to prevent the double installation via micromamba.Eventually we would move to full Pixi in the whole pipeline if that's where the ecosystem is generally going to.
I think I like this option better for now, but it will require some more plumbing. Hopefully not too much complexity, though, but I won't know til I have written the code. WDYT?
That sounds great. Agreed options are good!
Hi! This is the staged-recipes linter and I found some lint.
File-specific lints and/or hints:
environment.yaml
:
environment.yaml
file is out of sync with pixi.toml
. Fix by running pixi project export conda-environment -e build > environment.yaml
.I'm quite happy with the local Pixi workflow now. It's quite neat and integrates nicely with the CI pipelines. With a couple adjustments in the Windows scripts we also got build-locally.py
support there!
Hi! This is the staged-recipes linter and your PR looks excellent! :rocket:
An we add documentation to the readme for now?
Let me know what you think :)
@jaimergp This is amazing to see and I'm thrilled at the idea of getting to much more easily test my local repieces before even opening up a PR now. Thanks a huge amount for this work.
However, it seems there's a bug introduced here (I haven't properly debugged fully) as running
pixi run build-linux linux64
on any PR (e.g. one of mine: https://github.com/conda-forge/staged-recipes/pull/28224) fails with
...
+ echo 'Finding recipes merged in main and removing them from the build.'
Finding recipes merged in main and removing them from the build.
+ pushd /home/conda/staged-recipes/recipes
/home/conda/staged-recipes/.scripts/build_steps.sh: line 58: CI: unbound variable
Traceback (most recent call last):
File "/home/feickert/Code/GitHub/conda-forge/staged-recipes/build-locally.py", line 115, in <module>
main()
File "/home/feickert/Code/GitHub/conda-forge/staged-recipes/build-locally.py", line 107, in main
run_docker_build(ns)
File "/home/feickert/Code/GitHub/conda-forge/staged-recipes/build-locally.py", line 39, in run_docker_build
subprocess.check_call([script])
File "/home/feickert/.pyenv/versions/3.11.7/lib/python3.11/subprocess.py", line 413, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['.scripts/run_docker_build.sh']' returned non-zero exit status 1.
where
was introduced in this PR (and of course
is set).
Like I said, I haven't rolled back in history yet to see where the issue first pops up as I'm time limited / in transit currently, but as you were recently doing some great work here I thought I'd at least raise this.
Oops, thanks for the report. I think you can work around by setting CI=0
for now, but I'll add a fix.
Should have been fixed with https://github.com/conda-forge/staged-recipes/pull/28245 @matthewfeickert. Can you check? π
Can you check? π
Thanks for being super fast @jaimergp. PR https://github.com/conda-forge/staged-recipes/pull/28224 is now able to build fine with
docker system prune -f && pixi run build-linux linux64
:rocket: Thanks for this great work!
Hey, awesome to see pixi support for local debugging!
I just tested this on windows, but unfortunately I got an error related to 'setup_conda_rc' not being found. Which is strange, becuase when I run pixi shell -e win
the setup_conda_rc
executable is found. Anyone else seeing this on windows? Or am I doing something wrong?
(base) C:\Work\code\staged-recipes>pixi run build-win win64
β¨ Pixi task (build-win in win): python build-locally.py win64
filtering for 'win*.yaml' configs
valid configs are {'win64'}
Using win64 configuration
Provisioning build tools
Build tools already installed at C:\Work\code\staged-recipes\.pixi\envs\win.
Configuring conda
Activating "C:\Work\code\staged-recipes\.pixi\envs\win"
Setting up configuration
'setup_conda_rc' is not recognized as an internal or external command,
operable program or batch file.
Traceback (most recent call last):
File "C:\Work\code\staged-recipes\build-locally.py", line 115, in <module>
main()
File "C:\Work\code\staged-recipes\build-locally.py", line 111, in main
run_win_build(ns)
File "C:\Work\code\staged-recipes\build-locally.py", line 49, in run_win_build
subprocess.check_call(["cmd", "/D", "/Q", "/C", f"CALL {script}"])
File "C:\Work\code\staged-recipes\.pixi\envs\win\Lib\subprocess.py", line 413, in check_call
raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['cmd', '/D', '/Q', '/C', 'CALL .scripts/run_win_build.bat']' returned non-zero exit status 9009.
However, it finds setup_conda_rc
when running from the shell:
(base) C:\Work\code\staged-recipes>pixi shell -e win
(staged-recipes:win) C:\Work\code\staged-recipes>setup_conda_rc
Usage: setup_conda_rc [OPTIONS] FEEDSTOCK_ROOT RECIPE_ROOT CONFIG_FILE
Try 'setup_conda_rc --help' for help.
Error: Missing argument 'FEEDSTOCK_ROOT'.
Hey @Krande, I just tested it out on my machine and I can't reproduce your error π€ Can you remote the .pixi
directory and the pixi.lock
file and try again? If this doesn't fix it, please open a new issue and tag me there so we can keep track of each incident separately. Thanks!
Added some more tasks: https://github.com/conda-forge/staged-recipes/pull/28264
Hey @Krande, I just tested it out on my machine and I can't reproduce your error π€ Can you remote the
.pixi
directory and thepixi.lock
file and try again? If this doesn't fix it, please open a new issue and tag me there so we can keep track of each incident separately. Thanks!
Doh, it was just me being an idiot! I tried running pixi from inside a terminal where miniforge was already activated. When running pixi from an unitialized terminal it worked as expected! Thanks again for the work on adding pixi support!
This a remix of https://github.com/conda-forge/conda-smithy/pull/2099 and https://github.com/conda-forge/conda-smithy/pull/2075.
Essentially, we are still using
micromamba
on CI but we will detect ifMINIFORGE_HOME
was set and points to a valid conda environment. If that's the case, we will assume it contains the dependencies we need. If it doesn't exist, then micromamba (which will be downloaded only if not found locally) will provision the build tools in that location, as we were doing before.Now, the fun stuff: we can provide
MINIFORGE_HOME
with Pixi tasks, along with the necessary dependencies in each platform. On Linux, it's just Python; elsewhere, it's the whole conda-build + rattler mix. We also take this opportunity to set a couple env vars to keep things tidy (SDKs in macOS go under.pixi
; all conda-build artifacts end up inbuild_artifacts
, regardless the platform).I also adjusted the
build-locally.py
script so we can filter which platforms are offered on each machine (depending on the base system). This way we can dopixi run build-linux
on every platform (because we "only" need Docker), but it will only allow us to select thelinux*
configs. Conversely,pixi run build-osx
is only available on macOS and only allow us to chooseosx*
configs. Same for Windows.If folks need to refresh their environments, they just need to remove
pixi.lock
and Pixi will do the rest.pixi.toml
andenvironment.yaml
are kept in sync by a linter check. Note we bumped some dependencies as part of this change.