Skip to content
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

Treat MinGW CMake installation paths as Unix instead of MSYS #414

Merged

Conversation

antoniovazquezblanco
Copy link
Contributor

Based on @Biswa96 review on #413, a better fix is proposed.

Sorry about the double PR! :)

@antoniovazquezblanco
Copy link
Contributor Author

Ping :)

@antoniovazquezblanco
Copy link
Contributor Author

Ping @ncorgan :)

@guruofquality
Copy link
Contributor

seems almost like WIN32 doing set(CMAKE_LIB_DEST cmake) is just the odd man out and maybe should go away. I dont know if this is still the default search path thing for a visual studio install. Or maybe they should all just be set(CMAKE_LIB_DEST ${CMAKE_INSTALL_DATADIR}/cmake/${PROJECT_NAME})

But for now in the spirit of just fixing this for MINGW, should this have been UNIX OR MSYS OR MINGW?

@antoniovazquezblanco
Copy link
Contributor Author

@Biswa96, what do you think about @guruofquality suggestion?

Thanks

@Biswa96
Copy link

Biswa96 commented Apr 22, 2024

But for now in the spirit of just fixing this for MINGW, should this have been UNIX OR MSYS OR MINGW?

I do not know if this project supports msys or cygwin environment. At least, the above suggestion would not hurt mingw. So, it's ok I guess.

@antoniovazquezblanco
Copy link
Contributor Author

Done

@guruofquality guruofquality merged commit b5227a9 into pothosware:master Apr 22, 2024
37 of 61 checks passed
@antoniovazquezblanco
Copy link
Contributor Author

Thank you very much!

@antoniovazquezblanco antoniovazquezblanco deleted the dev/cmake_dest_fix_2 branch April 22, 2024 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants