-
Notifications
You must be signed in to change notification settings - Fork 18
Integrates CbcParser
class into the base recipe parsing class
#400
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
base: main
Are you sure you want to change the base?
Conversation
… RecipeParser class
…ariable determination work. The variable table managed by the recipe parser will NOT be modified with CBC variables
I just had a sporadic failure on the |
I think I have all the testing I want. This should cover the vast majority of cases with the changes. #401 tracks the TODOs left in by this PR. |
for entry in cbc_entries: | ||
selector = entry.get_selector() | ||
if selector is None or selector.does_selector_apply(query): | ||
# TODO: What to do if multiple selectors apply??? |
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.
Could you please clarify this question with an example ?
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.
What happens if the CBC file has errors in it? As an example:
You want to use a value for a package against platform LINUX_64
but the CBC file has a value of 1.2.3 for [unix]
and a value of 1.4.2 for [linux]
. There's an overlap in the set that makes it unclear which value should be applied. Currently, the first value read (from top-to-bottom) will be applied.
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.
It's possible for a variable to intentionally be given multiple values (example) to generate multiple build variants for a single platform.
The current solution seems to assume a single recipe variant for a given platform/CBC file combination. Instead, I think we need a way to generate multiple variants for a given (platform, CBC file): one variant per possible combination of variable values (except for zip_keys).
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've been purposefully kicking the can on zip_keys
. I haven't had time to think about it and didn't want this PR to get too big. You'll see a ton of TODOs around this in the code.
The other reason I've been delaying that work is I think it only ever applies to numpy
. Or at least that's the only key I've ever seen zipped.
This is the first set of changes that allows the The
Recipe*
classes (with the exception ofRecipeParserConvert
) to make variable substitutions with values found in aCbcParser
instance (from aconda_build_config.yaml
file).This at least partially address #399 although I think there are a number of features we need to add to the
CbcParser
class to say that we have a full integration.I'm leaving this in draft until I finish the initial set of tests.