Skip to content

feat: enhance command handling in recipe parser with conditional error management #1562

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

zelosleone
Copy link
Collaborator

fixes #960

I think mapping the if conditions and going with a special case was the correct answer here. I also added an e2e test to make sure it worked, on windows it will not render the script if the if condition is for Unix, so the test doesn’t look for it

@zelosleone
Copy link
Collaborator Author

Oh okay, there is more work to be done here since i am on windows, I will switch to linux as well to fix this, in the mean time i will mark this as draft

@zelosleone zelosleone marked this pull request as draft April 15, 2025 09:40
@zelosleone zelosleone marked this pull request as ready for review April 15, 2025 12:20
@zelosleone
Copy link
Collaborator Author

zelosleone commented Apr 15, 2025

So I think I solved the issue; first of all the problem was trying to parse everything together, then trying to modify it. This, obviously, didn’t work in this specific case where a more refined approach was needed. So what i did was, I would parse the things we need (script field, requirement etc.) field-by-field, and while doing that, i would condition it for if conditions by mapping them to their respective system target.

Now, this brought another problem though. Crazy characters test! I thought there was a problem with my approach at first, but after thinking about it some time and trying to debug it, I realized windows by default just completely blocks exceeding the file path limit for file creation! (The test was only failing for the windows) I also realized by default, github runners will just use default-windows path settings, it would fail there as well. (I only caught it because yesterday night i formatted my laptop and using pure default settings today.) After some thinking, I removed the file path part of the test script for windows, decided to check if it works correctly on ubuntu runner and yeah, it worked. I think there was a parsing issue with the same characters on windows, that's why it passed before but cannot be entirely sure. I would be also thankful if there is some flaw in my logic and you could point it to the correct direction @wolfv

On windows, now the test i wrote passes with the output:

test/end-to-end/test_simple.py::test_conditional_script  ⠁ determining virtual packages

 ╭─ Finding outputs from recipe
 │ Found 1 variants
 │
 │ Build variant: test-if-script-0.1.0-h9490d1a_0
 │
 │ ╭─────────────────┬──────────╮
 │ │ Variant         ┆ Version  │
 │ ╞═════════════════╪══════════╡
 │ │ target_platform ┆ "win-64" │
 │ ╰─────────────────┴──────────╯
 │
 ╰─────────────────── (took 0 seconds)

 ╭─ Running build for recipe: test-if-script-0.1.0-h9490d1a_0
 │
 │ ╭─ Fetching source code
 │ │ No sources to fetch
 │ │
 │ ╰─────────────────── (took 0 seconds)
 │
 │ ╭─ Resolving environments
 │ │
 │ │ Finalized run dependencies: this output has no run dependencies
 │ │
 │ ╰─────────────────── (took 0 seconds)
 │
 │ Installing build environment
 │ ✔ Successfully updated the build environment
 │
 │ Installing host environment
 │ ✔ Successfully updated the host environment
 │
 │ ╭─ Running build script
 │ │ %SRC_DIR%>IF "" == "" (
 │ │
 │ │  call %SRC_DIR%\build_env.bat
 │ │ )
 │ │ %SRC_DIR%>set "LIB=C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.43.34808\lib\x64
 │ │ ;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\lib\um\x64;C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.
 │ │ 0\ucrt\x64;C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x64"
 │ │ %SRC_DIR%>set "CPATH="
 │ │
 │ ╰─────────────────── (took 0 seconds)
 │
 │ ╭─ Packaging new files
 │ │ Copying done!
 │ │ Post-processing done!
 │ │ Writing test files
 │ │ Writing metadata for package
 │ │ Copying license files
 │ │ Copying recipe files
 │ │ Creating entry points
 │ │
 │ │ Files in package:
 │ │   - info/about.json
 │ │   - info/hash_input.json
 │ │   - info/index.json
 │ │   - info/paths.json
 │ │   - info/recipe/recipe.yaml
 │ │   - info/recipe/rendered_recipe.yaml
 │ │   - info/recipe/variant_config.yaml
 │ │   - info/tests/tests.yaml
 │ │ Creating target folder 'C:\Users\deniz\AppData\Local\Temp\pytest-of-deniz\pytest-26\test_conditional_script0\win-64
 │ │ '
 │ │ Compressing archive...
 │ │ Archive written to 'C:\Users\deniz\AppData\Local\Temp\pytest-of-deniz\pytest-26\test_conditional_script0\win-64\tes
 │ │ t-if-script-0.1.0-h9490d1a_0.tar.bz2'
 │ │
 │ ╰─────────────────── (took 0 seconds)
 │
 ╰─────────────────── (took 0 seconds)
 Removing previously cached package 'C:\Users\deniz\AppData\Local\rattler\cache\pkgs\test-if-script-0.1.0-h9490d1a_0'
 Creating test environment in 'C:\Users\deniz\AppData\Local\Temp\pytest-of-deniz\pytest-26\test_conditional_script0\bld\
 rattler-build_test-if-script_1744719995\work\test'
 Collecting tests from 'C:\Users\deniz\AppData\Local\rattler\cache\pkgs\test-if-script-0.1.0-h9490d1a_0'

 ╭─ Running script test for recipe: test-if-script-0.1.0-h9490d1a_0.tar.bz2
 │
 │ Resolving test environment:
 │   Platform: win-64 [__win=10.0.26100=0, __cuda=12.7=0, __archspec=1=skylake]
 │   Channels:
 │    - file:///C:/Users/deniz/AppData/Local/Temp/.tmpWchtxR/
 │    - file:///C:/Users/deniz/AppData/Local/Temp/pytest-of-deniz/pytest-26/test_conditional_script0/
 │    - conda-forge
 │   Specs:
 │    - bzip2
 │    - test-if-script ==0.1.0 h9490d1a_0
 │
 │ ╭────────────────┬──────────────┬─────────────┬─────────────┬────────────╮
 │ │ Package        ┆ Version      ┆ Build       ┆ Channel     ┆       Size │
 │ ╞════════════════╪══════════════╪═════════════╪═════════════╪════════════╡
 │ │ bzip2          ┆ 1.0.8        ┆ h2466b09_7  ┆ conda-forge ┆  53.64 KiB │
 │ │ test-if-script ┆ 0.1.0        ┆ h9490d1a_0  ┆ .tmpWchtxR  ┆   1.18 KiB │
 │ │ ucrt           ┆ 10.0.22621.0 ┆ h57928b3_1  ┆ conda-forge ┆ 546.59 KiB │
 │ │ vc             ┆ 14.3         ┆ h2b53caa_26 ┆ conda-forge ┆  17.47 KiB │
 │ │ vc14_runtime   ┆ 14.42.34438  ┆ hfd919c2_26 ┆ conda-forge ┆ 733.14 KiB │
 │ ╰────────────────┴──────────────┴─────────────┴─────────────┴────────────╯
 │
 │ Installing test environment
 │ ✔ Successfully updated the test environment
 │ Testing commands:
 │ %SRC_DIR%>IF "" == "" (
 │
 │  call %SRC_DIR%\build_env.bat
 │ )
 │ %SRC_DIR%>set "LIB=C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools\VC\Tools\MSVC\14.43.34808\lib\x64;C
 │ :\Program Files (x86)\Windows Kits\NETFXSDK\4.8\lib\um\x64;C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.0\uc
 │ rt\x64;C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x64"
 │ %SRC_DIR%>set "CPATH="
 │
 ╰─────────────────── (took 1 second)
 ✔ all tests passed!

 ╭─ Build summary
 │
 │ ╭─ Build summary for recipe: test-if-script-0.1.0-h9490d1a_0
 │ │ Artifact: C:\Users\deniz\AppData\Local\Temp\pytest-of-deniz\pytest-26\test_conditional_script0\win-64\test-if-scrip
 │ │ t-0.1.0-h9490d1a_0.tar.bz2 (1.18 KiB)
 │ │ Variant configuration (hash: h9490d1a_0):
 │ │ ╭─────────────────┬──────────╮
 │ │ │ target_platform ┆ "win-64" │
 │ │ ╰─────────────────┴──────────╯
 │ │
 │ │
 │ ╰─────────────────── (took 0 seconds)
 │
 ╰─────────────────── (took 0 seconds)

@zelosleone zelosleone marked this pull request as draft April 17, 2025 08:05
@zelosleone zelosleone marked this pull request as ready for review April 17, 2025 08:54
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.

Test scripts expressions are wrong evaluated
1 participant