Adding conda build recipe files #188
Conversation
conda/conda-build/meta.yaml
Outdated
| # vNN.NN.NN | ||
| # vNN.NN.NN.dev | ||
| {% elif 'dev' in environ.get('GIT_DESCRIBE_TAG', '') %} | ||
| {% set version = environ.get('GIT_DESCRIBE_TAG', '') ~ environ.get('GIT_DESCRIBE_NUMBER', '') %} |
There was a problem hiding this comment.
Remember to strip the preceding "v" from the tag so that the version number is valid
conda/conda-build/meta.yaml
Outdated
| {% elif 'dev' in environ.get('GIT_DESCRIBE_TAG', '') %} | ||
| {% set version = (environ.get('GIT_DESCRIBE_TAG', '') ~ environ.get('GIT_DESCRIBE_NUMBER', '')).lstrip('v') %} | ||
| {% else %} | ||
| {% set version = environ.get('GIT_DESCRIBE_TAG', '').lstrip('v') %} |
There was a problem hiding this comment.
Either this conditional should have a default statement to handle if it's not running under a git repo, or the environ.get() in the fallback should have a default value.
There was a problem hiding this comment.
This also applies for PR's which are git repos, but do not have tags.
There was a problem hiding this comment.
Hmm, I think that if we are running in one of these situations, we should provide package version through package_version. I don't think that there is really a good default. Let me know what do you think.
There was a problem hiding this comment.
hrm, agreed. We could just set it to 0.0.0?
There was a problem hiding this comment.
We could do that, but I would personally prefer an error message that would remind me to set package_version. WDYT? Unfortunately, for cunumeric we don't have a choice since we need a default to avoid errors in the initial pass where the environment vars are not defined yet. That's why we need a "fake" default there, but it also can prevent error messages.
There was a problem hiding this comment.
I tried to remove the defaults, but that does not work:
TypeError: argument of type 'NoneType' is not iterable
So it seems that without the default, the output of environ.get ends up being None in the first pass. So we do have to keep the default, even if it is simply ''. I will elevate the default into a separate variable.
There was a problem hiding this comment.
Sounds good. Other than that LGTM.
There was a problem hiding this comment.
Should be fixed. Take a look and confirm if ready for merging.
marcinz
changed the title
* Update the default test binary name * Make exception handling eager and put back Python exception support * Custom exception handler to filter legate::TaskException * Address review comment * Address review comments * Support multiple exception classes * Expose the interna list of exceptions as a tuple * Re-raise index errors as runtime errors * Should have retrieved the index class outside the try block
2 participants
Files copied from the utils repository.