[lldb-dev] [PATCH] Intended install location for python scripts

[Todd Fiala]

Attached please find a patch proposal to fix the following bug:

http://llvm.org/bugs/show_bug.cgi?id=18124

It makes the two changes we discussed:

* when copying scripts during 'make install', if the copying fails, the
error is propagated to make and causes make to fail.

* the lldb python packages are now installed under the install tree,
matching output from lldb -P.

I've tested it both with a parallel build tree and with an in-source build
tree.  I also implemented the make failure propagation before doing the
other change, verifying that my initial failure condition (permission
problem due to lack of 'sudo') correctly fails the make.

I did not do anything with cmake builds.

Feedback is welcomed!

-Todd

On Tue, Dec 3, 2013 at 10:09 AM, Todd Fiala <tfiala at google.com> wrote:

> Excellent.  Thanks, Greg!
>
> I'll get a patch together following that approach unless anything further
> develops here.
>
>
> On Tue, Dec 3, 2013 at 10:00 AM, Greg Clayton <gclayton at apple.com> wrote:
>
>>
>> On Dec 3, 2013, at 9:46 AM, Todd Fiala <tfiala at google.com> wrote:
>>
>> > Hi all,
>> >
>> > I started digging into the 'make install' python script installation
>> issue listed on another thread.  Here's the ticket for it:
>> >
>> > http://llvm.org/bugs/show_bug.cgi?id=18124
>> >
>> > I've added some comments to the bug.  My initial digging leads to a few
>> questions:
>> >
>> > 1. Where do we expect a configure/make/make install to place the python
>> scripts?  I was under the impression the scripts should be installed in the
>> same directory tree implied by the configure --prefix arg, which is also
>> what a `lldb -P` reports.  However, what I'm finding is that the 'make
>> install' step is trying to install the python scripts into the python
>> interpreter's dist-packages location.  I'm getting these failures on
>> install because I'm not doing a 'sudo' on my make install.
>> >
>> >     llvm[4]: Installing Debug+Asserts LLDB python modules
>> >     mkdir: cannot create directory
>> `/usr/lib/python2.7/dist-packages/lldb': Permission denied
>> >     Error: Unable to find or create
>> /usr/lib/python2.7/dist-packages/lldb
>> >     make[4]: Leaving directory
>> `/usr/local/google/home/tfiala/llvm/work/build-debug/tools/lldb/source/Interpreter'
>> >
>> > Do we want the python scripts installing into the python executable's
>> dist-packages or the location reported by lldb -P?
>>
>> Yes, this would allow multiple installs of LLDB to co-exist without
>> stomping on the previous LLDB's python modules (which could be very
>> different).
>>
>> >  My first stab would be it should go in the installation prefix paths
>> as I would not expect any code to be installed outside of the prefix and,
>> possibly, require a sudo when I'm trying to do everything locally without
>> elevated privileges.
>> >
>> > 2. Should 'make install' fail if we fail to copy over python files? My
>> first take is that a failure to install the scripts should qualify as an
>> all-out failure and cause make to fail (i.e. fail early, fail loudly);
>> otherwise, we can get quite a bit past the root cause of scripting-related
>> failures without realizing what's broken.
>> >
>> > Any opinions on this?  My straw-man answers are:
>> >
>> > 1. Install into the tree implied by configure --prefix, not into the
>> python dist-packages dir.  (A close second would be to specify a new
>> configure path variable so the user can configure, possibly adjusting the
>> output of lldb -P to match).
>>
>> The --prefix path sounds right to be for coexistence.
>>
>> >
>> > 2. Change makefiles so that failure to copy scripts fails the install.
>>
>> Agreed.
>>
>> >
>> > I'll fix up whatever we decide here if it requires a change.
>> >
>> > Thanks!
>> >
>> > Sincerely,
>> > Todd Fiala
>> > _______________________________________________
>> > lldb-dev mailing list
>> > lldb-dev at cs.uiuc.edu
>> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20131203/12ffd1ba/attachment.html>
-------------- next part --------------
commit f117dd20c1440b5e4c6a7d0672125e540667c353
Author: Todd Fiala <tfiala at google.com>
Date:   Tue Dec 3 10:29:55 2013 -0800

    Change 'make install' to copy python packages to the prefix dir.
    
    Also makes python script install failures fail out of make.
    
    Fixed up python script install step to install to the prefix dir.

diff --git a/scripts/finish-swig-wrapper-classes.sh b/scripts/finish-swig-wrapper-classes.sh
index f0bca4b..806f286 100755
--- a/scripts/finish-swig-wrapper-classes.sh
+++ b/scripts/finish-swig-wrapper-classes.sh
@@ -89,6 +89,11 @@ do
 
 
             ./finish-swig-${curlang}-LLDB.sh $SRC_ROOT $TARGET_DIR $CONFIG_BUILD_DIR "${PREFIX}" "${debug_flag}" "${makefile_flag}"
+            retval=$?
+            if [ $retval -ne 0 ]; then
+                echo "$(pwd)/finish-swig-${curlang}-LLDB.sh failed with exit code $retval"
+                exit $retval
+            fi
         fi
     fi
 done
diff --git a/source/Interpreter/Makefile b/source/Interpreter/Makefile
index 5a6ffba..f077292 100644
--- a/source/Interpreter/Makefile
+++ b/source/Interpreter/Makefile
@@ -35,7 +35,7 @@ LLDBWrapPython.cpp lldb.py: $(PROJ_SRC_DIR)/$(LLDB_LEVEL)/scripts/Python/modify-
 
 install-local:: lldb.py
 	$(Echo) Installing $(BuildMode) LLDB python modules
-	$(Verb) "$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/scripts/finish-swig-wrapper-classes.sh" "$(PROJ_SRC_DIR)/$(LLDB_LEVEL)" "$(PROJ_OBJ_DIR)" "$(PROJ_OBJ_DIR)" "" -m
+	$(Verb) "$(PROJ_SRC_DIR)/$(LLDB_LEVEL)/scripts/finish-swig-wrapper-classes.sh" "$(PROJ_SRC_DIR)/$(LLDB_LEVEL)" "$(PROJ_OBJ_DIR)" "$(PROJ_OBJ_DIR)" "$(prefix)" -m
 
 clean-local::
 	$(Verb) $(RM) -f LLDBWrapPython.cpp lldb.py