-
Notifications
You must be signed in to change notification settings - Fork 44
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
BUG: Variables from variants not populated in build scripts #1056
Comments
Yes, you'll have to use the variable in the cache inside the YAML somehow. I think there is a force use variables option you could try, for example. |
Or you can pass it as an env var in the script context. We don't scan build scripts for used variables. |
It's not a question of scanning the build scripts, which I agree is not necessary, or even a good idea. But the variables coming from the variant config should be defined during the build scripts, unless there's a very strong reason not to do this...? A huge amount of recipes depend on this, it'll be really painful to work around this IMO. |
It's not the way conda build works. Variables have to be used to influence the variant. |
You can have a very long variant configuration file but only a subset will apply to your recipe. Conda forge just pre processes all of this in conda-smithy. |
I understand that; but it's enough to use it in the recipe
conda-build does populate variants (picked up from the recipe) also for the build script. Which is my point, because a lot of recipes rely on things that are defined in the |
In the case of conda-forge/zlib-feedstock#84, a rerender with rattler-build populates The only thing to do AFAIU is to pre-populate the environment variables present during the build scripts with whatever is in the respective |
But that would be wrong since we don't create a variant for it... You need to use the variant key in the output to make it appear in the build script. Otherwise the behavior would be super broken. |
For example, you could have multiple outputs. One depends on python and one does not. Only the one that depends on python will build the python variant packages. The python variant value will not be set during the build for the non-python output (even though it appears in the variant config). The value would be garbage anyways. Does that make sense? |
I understand that aspect I think. But most of those cases are completely benign. Populating an env var for I really think we need to match conda-build here, because not picking up certain flags in build scripts can lead to arbitrarily painful behaviour. The build failing is the easiest case, but it could lead to subtly broken behaviour because (for example) a platform-specific branch that was added after debugging a previous problem suddenly doesn't trigger anymore. Again, we could warn on these things and help people improve their recipes by making things explicit, but IMO we cannot realistically transition to v1 recipes in any reasonable timeframe if there are such fundamental and subtle behaviour changes. Beyond that point, how would one specify that |
The most obvious way would be to do: build:
script:
file: foo.sh
env:
CROSS_TARGET_PLATFORM: ${{ cross_target_platform }} In the appropriate output. The other way would be to use build:
variant:
use_keys: ["cross_target_platform"] In conda-build this probably works because they do analyze the |
Thanks. I tried this, but it still doesn't set the value properly:
(logs) |
And the other way works? :) |
Not tested yet, but I already don't like it, because it creates yet another indirection and divergence from conda-build. 😑 |
So first I did: script:
# rattler-build figures out the {.sh,.bat} extensions
- build_global
+ # see https://github.com/prefix-dev/rattler-build/issues/1056
+ env:
+ CROSS_TARGET_PLATFORM: ${{ cross_target_platform }} but that doesn't work because then one needs to shift script:
- build_global to script:
file: build_global This is pretty bad dev experience IMO (I had two other failed attempts in between, because of invisible list-vs-scalar expectations). Why not make PS. It does work in the end, but I don't think it's a feasible to do a migration with this. Footnotes
|
What you typed is also not valid YAML though (mixing list & map). I don't see the trap there, sorry (any editor would put red squiggly lines below that). |
Sure (and I did try with |
OK, I think I initially understood your request slightly wrongly. The bug you have discovered is about making the variant key/values available as environment vars in the build script. That is something that seems to be broken in rattler-build indeed. I'll double check if that is also a problem for a regular single-output build. We will definitely fix that! :) |
👍 Sorry I didn't manage to communicate this more clearly. |
cache:
?)
Fixed by #1088 |
While trying to build zlib for emscripten, I'm setting the following in CBC
and have a branch in the build script à la:
However, when building this with rattler-build, the variables end up not being set, i.e. the log shows
where the
''
part should not be empty.That's despite this being picked up for the various outputs - but perhaps not for
cache:
? 🤔The text was updated successfully, but these errors were encountered: