-
Notifications
You must be signed in to change notification settings - Fork 340
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
Drop find_programs gs #728
Conversation
CMakeLists.txt
Outdated
@@ -72,7 +72,7 @@ set (CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/modules/" | |||
# Find UNIX commands | |||
include (FindUnixCommands) | |||
find_package (Git) | |||
find_program (GS gs gswin64) | |||
find_program (GS NAMES gs gswin64 gswin32) |
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.
Actually I think what is used on Windows are the gswin64c/gswin32c but I don't know why do we search for the ghost on Windows. The only module that uses it is psconvert, and it finds the executable via a registry inquire, which was a complicated thing to achieve because Windows64 have two registries one for 32 and another 64 and the Ghost will be in either one or the other depending on its bitage.
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.
You're right. Seems we even don't need to search gs on Linux. The variable GS
is never used in the cmake configuration or the psconvert source code. Maybe we can remove the find_program (GS ...)
line?
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.
Indeed, it looks we can drop it.
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.
Tried on Win and apparently nothing breaks, so good to go for me.
Question: Since GMT requires gs to be present, especially modern mode, I think finding gs was our proxy for making sure the user has gs installed. Will this go away now? I.e., users can build GMT if they do not have gs installed? |
|
But I doubt it ever detected the Ghost presence on Windows that does not install itself in PATH. This is what we do in psconvert
|
If you want to make sure gs is installed, we need something like
|
Well, I think we need to; it is not an optional install in my mind since psconvert calls it, and movie calls psconvert indirectly per frame via gmt end. |
Strictly speaking, gs is a runtime dependency but not necessary when building GMT. I agree we need to check if gs is available. |
Yes, and I dont know what the standard protocol is for tools that depends on executables. There must be other packages that depends on executables instead of includes and libraries. What do they do? |
I don't know if there are any standard way to do that. But we have a FindSphinx.cmake in cmake/modules. And this file (https://github.com/Kitware/CMake/blob/master/Modules/FindCygwin.cmake) shows that we can search gs from registry. |
This PR is already merged and there is no way to reopen it. I've opened an issue #733 in order not to forget it. |
What you think, @joa-quim ? Is this something to pursue or is it just more work for no gain? |
Let's drop it. I see no real gain in detecting gs presence while building on Win |
* Add cartesian.py * Add polar.py
gswin32
is another possible name of ghostscript.