Fix output directory on Windows with non-VS generators #924
Conversation
kbenzie
changed the title
kbenzie
force-pushed
the
benie/windows-fix-output-dir
branch
from
3b1ef5f to
7fd639c
Compare
| set(CUSTOM_COMMAND_BINARY_DIR ${CMAKE_UMF_OUTPUT_DIRECTORY}) | ||
| if(MSVC) | ||
| # MSVC implicitly adds $<CONFIG> to the output path | ||
| set(CUSTOM_COMMAND_BINARY_DIR ${CUSTOM_COMMAND_BINARY_DIR}/$<CONFIG>) |
There was a problem hiding this comment.
I removed this variable since it does not appear to be getting used anywhere, it's also adding $<CONFIG> when it's already be added which seems wrong.
Happy to reinstate if it is being used somewhere I missed.
There was a problem hiding this comment.
I don't recall using CUSTOM_COMMAND_BINARY_DIR anywhere explicitly
There was a problem hiding this comment.
@PatKamin, pehaps we should verify (locally?) our internal sycl-test - it uses Ninja generator...? we would confirm it's not breaking the internal testing...
LGTM 👍
| set(CUSTOM_COMMAND_BINARY_DIR ${CMAKE_UMF_OUTPUT_DIRECTORY}) | ||
| if(MSVC) | ||
| # MSVC implicitly adds $<CONFIG> to the output path | ||
| set(CUSTOM_COMMAND_BINARY_DIR ${CUSTOM_COMMAND_BINARY_DIR}/$<CONFIG>) |
There was a problem hiding this comment.
I don't recall using CUSTOM_COMMAND_BINARY_DIR anywhere explicitly
This test does not involve building UMF from sources, so this change shouldn't affect |
kbenzie
force-pushed
the
benie/windows-fix-output-dir
branch
from
7fd639c to
ced8f13
Compare
On Windows when configuring CMake with the Ninja generator and
`-DUMF_BUILD_SHARED_LIBRARY=ON` the `umf.dll` is output to the
`${CMAKE_BINARY_DIR}/bin/$<CONFIG>` directory, this is
problematic as it breaks tools like `urinfo.exe` and `sycl-ls.exe` in
development builds as they rely on `umf.dll` residing in the same
directory in order to be loaded.
This behavior is desirable when using Visual Studio generators, however.
Therefore, this patch changes the logic to check if `CMAKE_GENERATOR`
matches the `"Visual Studio"` string before appending `$<CONFIG>` to the
output directory.
kbenzie
force-pushed
the
benie/windows-fix-output-dir
branch
from
ced8f13 to
8d81df9
Compare
Problem with the runner? |
yeah, don't you worry about this one; re-run should fix it |
|
@kbenzie thanks! |
|
How should we get these changes into UR then intel/llvm? Ideally this would be in time to merge to intel/llvm in time for pulldown on Tuesday. I tried using UMF main branch tag in UR locally but am getting errors like this: |
We've introduced on main first bunch of changes around our configs; the error you see after bumping UMF is related to: #901 We planned to update UMF in UR when all changes related to configs are merged (at least PRs with label v0.10.x). When you said Tuesday - did you mean this one, a.k.a. tomorrow? |
That's when the pulldown is scheduled for, yeah. If it's not possible by then it can be cherry-picked. As long as its included in the next release. |
|
Ok, we were aiming for the next pulldown with all these changes, but no worries, we decided to prepare a special branch with a couple of cherry-picks for UR - I'll make a PR for UR in just a moment. |
Description
On Windows when configuring CMake with the Ninja generator and
-DUMF_BUILD_SHARED_LIBRARY=ONtheumf.dllis output to the${CMAKE_BINARY_DIR}/bin/$<CONFIG>directory, this is problematic as it breaks tools likeurinfo.exeandsycl-ls.exein development builds as they rely onumf.dllresiding in the samedirectory in order to be loaded.
This behavior is desirable when using Visual Studio generators, however. Therefore, this patch changes the logic to check if
CMAKE_GENERATORmatches the"Visual Studio"string before appending$<CONFIG>to the output directory.Checklist