Unify Common CI Code for Windows and Linux #645
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
/ok to test |
1 similar comment
|
/ok to test |
|
/ok to test |
|
/ok to test |
|
/ok to test |
|
/ok to test |
1 similar comment
|
/ok to test |
This comment has been minimized.
This comment has been minimized.
|
/ok to test |
|
/ok to test |
|
/ok to test |
|
/ok to test |
1 similar comment
|
/ok to test |
|
/ok to test |
|
/ok to test |
|
/ok to test |
cryos
changed the title
|
LGTM so far. I see the assumption here is that the env vars are set in the CI and we just run the scripts, so the scripts are not meant to be run outside of the CI. This is how we avoid param checks (which would add a ton of boilerplates). Let me know whenever you're happy to merge! |
Thanks Leo! There are a few other bits I wanted to centralize first, I was proving out that things would work as expected on Windows first. I can split that and merge this if preferred, then create follow ups later to avoid it growing too big. |
|
Up to you -- so far I've been able to follow just fine 🙂 |
| # We don't test compute-sanitizer on CTK<12 because backporting fixes is too much effort | ||
| # We only test compute-sanitizer on python 3.12 arbitrarily; we don't need to use sanitizer on the entire matrix | ||
| # Only local ctk installs have compute-sanitizer; there is no wheel for it | ||
| if [[ "${PY_VER}" == "3.12" && "${CUDA_VER}" != "11.8.0" && "${LOCAL_CTK}" == 1 && "${HOST_PLATFORM}" == linux* ]]; then |
There was a problem hiding this comment.
Thanks, good catch, this will fix #647.
|
Auto-sync is disabled for ready for review pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
| SETUP_SANITIZER=1 | ||
| else | ||
| SETUP_SANITIZER=0 | ||
| echo "SANITIZER_CMD=" >> $GITHUB_ENV |
There was a problem hiding this comment.
nit: no need to set it at this stage?
There was a problem hiding this comment.
You still want this for workflows that do not call setup-sanitizer as the variable would be unbound. It is nice not to force all workflows to call setup-sanitizer. We can remove it if we have all workflows call the setup, which thinking about it we could just do from the start of run-tests as it relies on the sanitizer stuff being ready there.
|
Description
Unify CI code to share common code between different workflows, closes #603.
Checklist