[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-buildsystem
Subject: Re: [PATCH]: Better FindPython Stuff (bug 174806)
From: Alexander Neundorf <neundorf () kde ! org>
Date: 2009-03-15 16:04:42
Message-ID: 200903151704.42123.neundorf () kde ! org
[Download RAW message or body]
On Sunday 15 March 2009, Allen Winter wrote:
> Howdy,
>
> I'd like to get a review of the attached patch to improve Python discovery.
>
> This is Michael Witten's patch posted to
> https://bugs.kde.org/show_bug.cgi?id=174806. I'm just the messenger.
>
> Also, this patch provides a new $PYTHON_SITE_PACKAGES_INSTALL_DIR
> where python packages can be installed (i.e. in a non-root-owned location,
> if desired).
Just some comments:
+# PYTHON_SITE_PACKAGES_INSTALL_DIR - Multiplexer between
PYTHON_{,USER_}SITE_PACKAGES_DIR
This doesn't really explain what it does.
+ # What seemed like error checking was worthless, so it has been removed;
+ # In fact, error checking is so nontrivial in these cases, that it has
+ # mostly not been attempted.
I think comments about something which was here earlier are not useful, this
should just be in the commit message.
+ if(PYTHON_SITE_PACKAGES_INSTALL_DIR)
+ # This variable may be supplied by the user.
+ message( "User supplied ...
This should be "message(STATUS ...")
+ elseif(NOT CMAKE_INSTALL_PREFIX MATCHES "^$ENV{HOME}")
+ set(PYTHON_SITE_PACKAGES_INSTALL_DIR ${PYTHON_SITE_PACKAGES_DIR})
...
+ elseif (PYTHON_USER_SITE_PACKAGES_DIR)
+ set(PYTHON_SITE_PACKAGES_INSTALL_DIR ${PYTHON_USER_SITE_PACKAGES_DIR})
...
This is quite some magic, changing the python install destination depending on
whether CMAKE_INSTALL_PREFIX points inside $HOME or not. Not sure I like
that.
+ else(PYTHON_SITE_PACKAGES_INSTALL_DIR)
+ message(SEND_ERROR
What SEND_ERROR and not FATAL_ERROR ?
I'd prefer FATAL_ERROR, because this aborts immediately, so it's easier to see
why the cmake run aborted (compared to scrolling up a few pages and looking
for the error).
+"You have selected a CMAKE_INSTALL_PREFIX that is under \${HOME}. Usually
+this means you are a KDE developer,
This is too much guessing, and FindPythonLibrary.cmake is usable independent
from KDE, so a message which says that you are probably a KDE developer is
out of place here.
Alex
_______________________________________________
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic