-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
fix: allow for feedstock_root to not be set #350
Conversation
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 ( |
@conda-forge-admin rerender |
…nda-forge-pinning 2024.09.25.18.51.16
Thanks, Matt! We've been seeing weird rerendering errors since this morning conda-forge/openmpi-feedstock#175 (comment), I assume it's the same issue? |
yes it appears to be the same issue. |
Well there are two errors we are seeing:
Matt is working around the first issue for this feedstock specifically We are still investigating what causes either error (or how to fix them) |
Ok think I understand 1. It is due to a conda-build change in 24.7.0. Namely this one: conda/conda-build#5371 Was able to re-render with conda-build 24.5 as show in PR: #352 Think we need to add some handling for re-rendering like this PR to address it At least the cause of the first error is now clearer |
Co-authored-by: jakirkham <jakirkham@gmail.com>
@conda-forge-admin , please re-render (just to reconfirm it works) |
…nda-forge-pinning 2024.09.25.18.51.16
Looks like re-rendering simply renamed variant files. So nothing really changed Main thing is we are able to re-render again |
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.
Thanks Matt! 🙏
LGTM. Should we go ahead and merge?
Note: Some jobs will still fail until issue ( conda-forge/binutils-feedstock#78 ) is resolved
Let's wait to merge and see if we can release a bugfix all at once for both issues. |
@conda-forge-admin restart ci |
@conda-forge-admin rerender |
…nda-forge-pinning 2024.09.26.07.35.45
Nice, what change fixed the rerender error? |
We need to define some vars at the top if they are undefined. |
{% if "FEEDSTOCK_ROOT" in os.environ %} # [linux] | ||
- cp {{ os.environ["FEEDSTOCK_ROOT"] }}/LICENSE.txt ${RECIPE_DIR}/LICENSE.txt # [linux] | ||
{% else %} # [linux] | ||
- echo '${FEEDSTOCK_ROOT} is undefined. Cannot copy license file' # [linux] | ||
- exit 1 # [linux] | ||
{% endif %} # [linux] |
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.
For context this is the guard Matt is referring to
Note this fixes the issue in this recipe. While other recipes can implement similar fixes, the change here alone doesn't fix other recipes
Hi! This is the friendly conda-forge automerge bot! I considered the following status checks when analyzing this PR:
Thus the PR was passing and merged! Have a great day! |
This PR fixes the rerendering bug where FEEDSTOCK_ROOT is not set and smithy chokes.