-
Notifications
You must be signed in to change notification settings - Fork 215
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
[enhancement] Store windows basekit with symlink-preserving compression for nightly-build #2938
Conversation
Example run using matching main from my fork: https://github.com/icfaust/oneDAL/actions/runs/11263646195 |
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.
LGTM, just one question why the upload-artifact version has been decreased?
Latest run showing valid operation of the nightly-build: https://github.com/icfaust/oneDAL/actions/runs/11373066491 |
Here is the latest run on my scikit-learn-intelex fork with the successful run https://github.com/icfaust/scikit-learn-intelex/actions/runs/11378786935 |
Good question, the version without the later specifiers will now use the latest version of v4, I thought this would be nicer to prevent a large number of renovate-bot PRs for upgrading it. They have corrected the symlink issues with the allowance of relative symlinks allowing for the builds to be as they were, so the version pin to 4.4.0 can be removed. |
Description
actions/upload-artifact#625 included in 4.4.1 fixes an issue to not preserve symlinks (since fixed in later versions). The Windows Intel oneAPI BaseKit installer uses absolute paths in symlinks rather than relative ones for the
latest
folders of various oneAPI components. This makes a maintenance burden on sklearnex-side since they are unusable. Therefore the fully path must be known and maintained to the compiler and tbb versionvars.bat
files. A rather long and complicated batch script line was added to force these to be relative. This removed the maintenance burden by fixing thelatest
symlink to work in sklearnex out-of-the-box. Since it is only one line and fixes what hopefully will only be a temporary issue I don't think necessitates its own script. Because of the complication of it, comments were added for future understanding.This will reduce maintenance burden on sklearnex side by making the CI pipeline run independent of the tbb/mkl/compiler versions
Checklist to comply with before moving PR from draft:
PR completeness and readability
Testing
Performance