-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
cmake: Follow unix-like install directories in mingw #3000
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
|
I agree with the idea. I have a bit of concern about execution. The logic of change to line 533 leaves possibility of Unix, Win64, Linux, et al. |
Doesn't previous logic also include those possibility? There is no Win64. WIN32 includes Win64. 1 There is another logic that can be used. Does this look good? @@ -530,7 +530,7 @@ if(WIN32 AND MSVC)
endif()
endif()
-if(NOT WIN32 AND NOT APPLE)
+if(NOT MSVC AND NOT APPLE)
install(TARGETS ${PROJECT_NAME}
RUNTIME DESTINATION bin
LIBRARY DESTINATION libFootnotes |
A little further up in CMake docs is Variables That Describe the System. MSVC is implying a MS Visual Studio Build. This may imply a Windows environment but is not exclusively a Windows system, IIRC. I think what may be best is that rather than an exclusive condition coupled with an inclusive condition (i.e. (NOT a and NOT b) or C) the logic test should just be simply exclusive only (i.e. NOT a and NOT b and NOT...) or inclusive only (i.e. a or b or c). Mixing seems bad to me, IMHO. |
|
My apology, I am out of ideas 😔 |
|
Hmmm, is this a situation where we are potentially rejecting a reasonable PR because the existing approach is too wide open? Looking at the suggested change, it seems to only add |
Right. |
I think that's good though, how does that leave out Unix and stuff, since the |
|
@Biswa96, @longnguyen2004, would this be compatible/consistent with #3068? |
|
Yes it's fine, this is for install directories, and the other one is for better dependency management. |
|
Thanks, @longnguyen2004, and sorry for the long delay. |
No description provided.