-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add patch to create versioned DLL on Windows #41
base: main
Are you sure you want to change the base?
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
recipe/bld.bat
Outdated
@@ -22,6 +22,10 @@ REM Install step | |||
cmake --build . --config "%BUILD_CONFIG%" --target install | |||
if errorlevel 1 exit 1 | |||
|
|||
REM Make copy of versioned DLL with unversioned name for compatibility | |||
REM with packages built against the older unversioned one | |||
cmake -E copy %LIBRARY_BIN%\OpenCL-1.dll %LIBRARY_BIN%\OpenCL.dll |
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 don't think this is a good idea. If two libraries bring them to the same process, what would happen?
Maybe we need to start with a new version of the loader, constrain all downstream packages to the new package and then rebuild with the new version.
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.
Fair point, I didn't think of that.
I'm trying to wrap my head around your proposed transition, and I don't understand it yet. So I'll try to break it down. We can't have versioned and unversioned installed at the same time, so the easiest way to get that is having them provided by the same package name and with the change happening at a version boundary (i.e. wait until a new release?). Then we need to have all current downstream packages require the version pre-boundary (repodata patch?), and then rebuild all of them with the new version which will come with its own minimum version requirement (migration?). How to do the rebuild/migration I guess is the part that I don't follow with my knowledge of conda-forge tools, since the package isn't pinned and we wouldn't just be changing a major version or some such. I guess rebuilding manually probably isn't that bad if that's what you're thinking.
Downstream packages newly built against this package will now search for the versioned DLL at runtime, eliminating an issue where non-conda OpenCL.dll (e.g. NVIDIA's own ICD loader) would interfere with the conda-packaged DLL because it has a higher priority on the PATH.
I've updated this so that it is just the patch to create a versioned DLL, with the intent that it be added on top of the next release. |
Checklist
[ ] Reset the build number to0
(if the version changed)conda-smithy
(Use the phrase@conda-forge-admin, please rerender
in a comment in this PR for automated rerendering)Fixes #11.