Correction for QHULL and Python #49
Conversation
| @@ -47,6 +47,11 @@ EXECUTE_PROCESS( | |||
| # Remove final \n of the variable PYTHON_SITELIB | |||
| STRING(REPLACE "\n" "" PYTHON_SITELIB "${PYTHON_SITELIB}") | |||
|
|
|||
| # In Win32, replace the \ character by / (can create issue with nmake). | |||
| IF(WIN32) | |||
| STRING(REPLACE "\\" "/" PYTHON_SITELIB "${PYTHON_SITELIB}") | |||
There was a problem hiding this comment.
This kind of thing should be better in "portability.cmake"... (I mean like providing the right slash in a single variable independently of the platform).
Remember: if you have a per-platform switch in a general purpose algorithm then you're doing it "wrong" ;)
There was a problem hiding this comment.
You mean, in portability.cmake there should be a method CORRECT_PATH (or else) that do this string conversion,
and this method should be called here (rather than specifying the conversion directly here).
There was a problem hiding this comment.
Yes defining a variable and propagating it in all the cmake files (by grepping slashes for instance) should be better and preserve us from future similar mistakes.
If this string replacement is used several times then yes adding a macro will do no harm. Again grep is great tool to look for code duplication!
There was a problem hiding this comment.
+1, this is likely to be useful elsewhere since there are tons of things that were not tested on Windows.
It is likely that the user installed it using easy_install. http://sphinx-doc.org/install.html#windows-install-python-and-sphinx
|
I applied the recommanded modifications + I enhanced detection of sphinx in Win32 |
|
Thanks François, I am ok with this one, please merge :) |
|
I have no comment and will follow Thomas advice. |
|
👍 |
Correction for QHULL and Python
No description provided.