-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Adding cuda-sanitizer-api recipe #23000
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 ( |
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
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 Alex! 🙏
Had a couple questions below
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.
A few questions, but generally LGTM.
Co-authored-by: jakirkham <jakirkham@gmail.com>
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.
Couple of small questions.
build: | ||
- {{ compiler("c") }} | ||
- {{ compiler("cxx") }} | ||
- arm-variant * {{ arm_variant_type }} # [aarch64] |
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 my information, how does this constraint work? Is the *
for "any version", and the arm_variant_type
selecting a particular build? If so, should it be using the build string to select sbsa? I'm assuming that this is right and that it's revealing some gap in my knowledge.
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.
We set the supported arm_variant_type
in conda_build_config.yaml
John can probably explain better, but I would expect this to match any version including the build string
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.
@jakirkham could you clarify here? I know that we're using the CBC here but I don't understand exactly what the constraint syntax is going to include/exclude in this case. In particular I want to verify that this is basically using the build string to filter sbsa. I think I'm mostly confused because I thought the build string component would need an equals sign, something like arm-variant=*={{arm_variant_type}}
rather than just being space-separated.
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.
Yeah the syntax above works in meta.yaml
. It is equivalent to =
based syntax one would use when calling on the command line
Co-authored-by: Vyas Ramasubramani <vyas.ramasubramani@gmail.com>
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.
I don't have permissions to resolve threads here, but aside from one open question regarding build strings this looks good to me.
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 Alex! 🙏
Maybe we can try adding wrapper scripts to call the .exe
s (as below). This is a strategy we have used for similar situations in the past (though it is always possible the code below needs additional tweaks)
Co-authored-by: jakirkham <jakirkham@gmail.com>
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 Alex! 🙏
Looks like the Windows build is erroring as part of building the package. Perhaps we can add a few lines to get more info about what is failing
Co-authored-by: jakirkham <jakirkham@gmail.com>
…ed-recipes into cuda-sanitizer-api
Thanks all! 🙏 Let's get this in. We can follow up on anything else in the feedstock |
xref: #21382
Checklist
url
) rather than a repo (e.g.git_url
) is used in your recipe (see here for more details).