-
Notifications
You must be signed in to change notification settings - Fork 797
[CI] Add "Nightly" releases support for manual runs of sycl-linux-run-tests.yml
#19690
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
Conversation
…n-tests.yml` We also have "official" releases like "v6.1.0" but they have different archive name format *and* lack necessary tools for running E2E tests (like `FileCheck`, etc.). As such, limit support for "Nightly" releases format.
@@ -299,6 +298,17 @@ jobs: | |||
rm -f ${{ inputs.toolchain_artifact_filename }} | |||
echo PATH=$PWD/toolchain/bin/:$PATH >> $GITHUB_ENV | |||
echo LD_LIBRARY_PATH=$PWD/toolchain/lib/:$LD_LIBRARY_PATH >> $GITHUB_ENV | |||
- name: Download SYCL toolchain using release tag |
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.
Should this step and the previous be mutually exclusive? What does it mean if both Extract/Setup SYCL toolchain
and Download SYCL toolchain using release tag
run?
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.
Well, they kind of are exclusive. Artifact is only set for workflow_call
trigger and release tag for workflow_dispatch
.
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.
can we set up the toolchain in a single step with an bash if statement to check which variable is set?
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 had something like that in my first version locally, but I didn't like it because the artifact download code is still in separate steps. See lines 258 and 263/286. I could have had three different downloads steps and a single export PATH
step, but that wouldn't be any better than what I have in this PR, IMO.
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.
ok yeah i see the problem, combining the three cases (using build step artifact from workflow call, using build step artifact from workflow dispatch, using nightly for workflow dispatch) would be pretty annoying, i guess some of it is our fault for trying to support a ton of cases but github definitely does not make it easy
We also have "official" releases like "v6.1.0" but they have different archive name format and lack necessary tools for running E2E tests (like
FileCheck
, etc.). As such, limit support for "Nightly" releases format.