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).

Some comments:


+    # 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, who intends to install everything
+without the need for greater permissions (as given by, `sudo make install'
+and its ilk). However, your version of python (${PYTHON_SHORT_VERSION})
+doesn't seem to provide a *user* site-packages directory into which the
+KDE python modules can be installed without special priveleges; this is
+usually something like ~/.local/lib/python2.6/site-packages as per PEP 370,
+which is understod by python>=2.6 (read 
http://docs.python.org/library/site.html).
+Currently, the only viable site-packages directory is 
(${PYTHON_SITE_PACKAGES_DIR}),
+which may require special access priveleges. If you think this is a gross
+misunderstanding, simply run cmake again with:
+
+    -DPYTHON_SITE_PACKAGES_INSTALL_DIR=$some_path
+
+Make sure that you have write access to $some_path (so that `make install'
+won't fail) and make sure $some_path ends up in sys.path when python
+runs")
+    endif(PYTHON_SITE_PACKAGES_INSTALL_DIR)
_______________________________________________
Kde-buildsystem mailing list
Kde-buildsystem@kde.org
https://mail.kde.org/mailman/listinfo/kde-buildsystem