rattler-build icon indicating copy to clipboard operation
rattler-build copied to clipboard

Values of `context` variables can have different types depending on the underlying shell

Open gforsyth opened this issue 9 months ago • 5 comments

I'm working on porting several conda-build recipes over to rattler-build and this particular quirk bit me a few times.

I have a context section like this:

context:
  version: ${{ env.get("RAPIDS_PACKAGE_VERSION") }}
  cuda_version: ${{ (env.get("RAPIDS_CUDA_VERSION") | split("."))[:2] | join(".") }}
  cuda_major: ${{ (env.get("RAPIDS_CUDA_VERSION") | split("."))[0] }}
  date_string: ${{ env.get("RAPIDS_DATE_STRING") }}
  head_rev: ${{ git.head_rev(".")[:8] }}

In the shell environment, representative values are things like RAPIDS_CUDA_VERSION=12.8.0 and RAPIDS_DATE_STRING=25026.

When I build this recipe locally, the value of cuda_major is '12' and date_string is '25026' -- however on a CI run (running bash but that's not the only factor) those variables get implicitly cast to int.

This is an issue, especially since we use them in selectors for choosing dependencies, so something like:

- if: cuda_major == "11"

suddenly isn't triggered because cuda_major=11 instead.

Given that shells, generally, have a very inconsistent view of the types of environment variables, is it possible to force all context variables that make use of env.get to be strings? And if someone wants an integer they can use | int?

For now, I'm forcing it myself like:

  cuda_major: '${{ (env.get("RAPIDS_CUDA_VERSION") | split("."))[0] }}'
  date_string: '${{ env.get("RAPIDS_DATE_STRING") }}'

but that's gross. Alternatively, maybe a str filter function to more cleanly force variables to always be strings?

Thanks!

gforsyth avatar Feb 26 '25 20:02 gforsyth

Wow, good catch. How about making env.get always be string? I think that makes most sense. Sorry about this!

wolfv avatar Feb 27 '25 16:02 wolfv

No problem! Took a bit to track down, but rattler is very nice to iterate with :)

I think env.get always being a string is a good plan, although I think that RAPIDS_CUDA_VERSION was a string ("12.8.0"), but the split and subsequent indexing led to it being cast to an int?

gforsyth avatar Feb 27 '25 16:02 gforsyth

Here is a minimal reproducer:

  context:
    name: fizzbuzz
    version: ${{ env.get("FIZZBUZZ_VERSION") }}
    major_version: ${{ (env.get("FIZZBUZZ_VERSION") | split("."))[0] }}

  package:
    name: ${{ name|lower }}
    version: ${{ version }}

  source:
    path: .

  build:
    number: 0
    script:
      - if: major_version == "1"
        then:
          - echo "This is version 1"
        else:
          - echo "This is not version 1"

If I run this locally I get

$ FIZZBUZZ_VERSION=1.0 rattler-build build
...
 │ ╭─ Running build script
 │ │ + echo 'This is version 1'
 │ │ This is version 1
 │ │
 │ ╰─────────────────── (took 0 seconds)

However, if I use the container that @gforsyth is referring to

$ docker run \
  --rm \
  -e FIZZBUZZ_VERSION=1.0 \
  --network=host \
  --volume $PWD:/repo \
  --workdir /repo \
rapidsai/ci-conda:latest rattler-build build
...
 │ ╭─ Running build script
 │ │ + echo 'This is not version 1'
 │ │ This is not version 1
 │ │
 │ ╰─────────────────── (took 0 seconds)

Hopefully that makes the exact issue easier to track down.

vyasr avatar Feb 27 '25 19:02 vyasr

I am struggling to find a difference with the latest versions of rattler-build between the docker and non-docker execution. Are we sure this was not a "fluke" from using different versions of rattler-build?

The main problem is simply that the environment variable will be inserted "unquoted" and Jinja / YAML will then interpret it as a floating point number or integer.

Quoting the YAML will make sure that it is cast as a string.

wolfv avatar May 02 '25 16:05 wolfv

We have looked at this and a potential solution is here: https://github.com/prefix-dev/rattler-build/pull/1612

Let us know if you have any additional thoughts!

wolfv avatar May 02 '25 17:05 wolfv