[Lldb-commits] [lldb] r157405 - in /lldb/trunk/scripts/Python: build-swig-Python.sh finish-swig-Python-LLDB.sh

[Jim Ingham]

I'm not sure I agree.  All this setting does is disable the Python build.  Disabling Python is a more general feature than building for iOS (though iOS is the only target we currently want to disable Python for.)  So it makes more sense to have this be a separate configuration variable, named for what it actually does.  The only incorrect part IMO is the comment, which wrongly implies that LLDB_DISABLE_PYTHON does anything other than disable Python...  Does this seem clearer to you:

Index: scripts/Python/finish-swig-Python-LLDB.sh
===================================================================
--- scripts/Python/finish-swig-Python-LLDB.sh	(revision 157474)
+++ scripts/Python/finish-swig-Python-LLDB.sh	(working copy)
@@ -33,8 +33,10 @@
 PYTHON_INSTALL_DIR=$4
 debug_flag=$5
 
-# Make sure LLDB_DISABLE_PYTHON is not set, since if it is this is an iOS build where python
-# is disabled
+# If we don't want Python, then just do nothing here.
+# Note, at present iOS doesn't have Python, so if you're building for iOS be sure to
+# set LLDB_DISABLE_PYTHON to 1.
+
 if [ ! $LLDB_DISABLE_PYTHON = "1" ] ; then
 
 if [ -n "$debug_flag" -a "$debug_flag" == "-debug" ]
Index: scripts/Python/build-swig-Python.sh
===================================================================
--- scripts/Python/build-swig-Python.sh	(revision 157474)
+++ scripts/Python/build-swig-Python.sh	(working copy)
@@ -25,7 +25,10 @@
 swig_python_typemaps=${SRC_ROOT}/scripts/Python/python-typemaps.swig
 
 if [ $LLDB_DISABLE_PYTHON = "1" ] ; then
-    # LLDB_DISABLE_PYTHON is set, which currently means iOS cross build where python is disabled
+    # We don't want Python for this build, but touch the output file so we don't have to
+    # conditionalize the build on this as well.
+    # Note, at present iOS doesn't have Python, so if you're building for iOS be sure to
+    # set LLDB_DISABLE_PYTHON to 1.
     rm -rf ${swig_output_file}
     touch ${swig_output_file}
 

?

Jim

On May 25, 2012, at 6:17 AM, Filipe Cabecinhas <filcab at gmail.com> wrote:

> Hi Johnny.  
> 
> I would say that these lines are a bug waiting to happen:
> 
>> if [ $LLDB_DISABLE_PYTHON = "1" ] ; then
>> - # SDKROOT was not empty, which currently means iOS cross build where python is disabled
>> + # LLDB_DISABLE_PYTHON is set, which currently means iOS cross build where python is disabled
> 
>> +# Make sure LLDB_DISABLE_PYTHON is not set, since if it is this is an iOS build where python
>> # is disabled
>> -if [ "x$SDKROOT" = "x" ] ; then
>> +if [ ! $LLDB_DISABLE_PYTHON = "1" ] ; then
> 
> 
> 
> Wouldn't you prefer to use a variable IOS_BUILD or something that would semantically mean “this is an iOS build”? When that variable would be set, we could set LLDB_DISABLE_PYTHON too.
> 
> 
> 
> Regards,
> 
>  Filipe
> 
> 
> On Thursday, May 24, 2012 at 7:14 PM, Johnny Chen wrote:
> 
>> Author: johnny
>> Date: Thu May 24 13:14:18 2012
>> New Revision: 157405
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=157405&view=rev
>> Log:
>> Fix missing Resources/Python directory for macosx build.
>> 
>> Modified:
>> lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh)
>> lldb/trunk/scripts/Python/finish-swig-Python-LLDB.sh (http://finish-swig-Python-LLDB.sh)
>> 
>> Modified: lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh)
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/build-swig-Python.sh?rev=157405&r1=157404&r2=157405&view=diff
>> ==============================================================================
>> --- lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh) (original)
>> +++ lldb/trunk/scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh) Thu May 24 13:14:18 2012
>> @@ -25,7 +25,7 @@
>> swig_python_typemaps=${SRC_ROOT}/scripts/Python/python-typemaps.swig
>> 
>> if [ $LLDB_DISABLE_PYTHON = "1" ] ; then
>> - # SDKROOT was not empty, which currently means iOS cross build where python is disabled
>> + # LLDB_DISABLE_PYTHON is set, which currently means iOS cross build where python is disabled
>> rm -rf ${swig_output_file}
>> touch ${swig_output_file}
>> 
>> 
>> Modified: lldb/trunk/scripts/Python/finish-swig-Python-LLDB.sh (http://finish-swig-Python-LLDB.sh)
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/scripts/Python/finish-swig-Python-LLDB.sh?rev=157405&r1=157404&r2=157405&view=diff
>> ==============================================================================
>> --- lldb/trunk/scripts/Python/finish-swig-Python-LLDB.sh (http://finish-swig-Python-LLDB.sh) (original)
>> +++ lldb/trunk/scripts/Python/finish-swig-Python-LLDB.sh (http://finish-swig-Python-LLDB.sh) Thu May 24 13:14:18 2012
>> @@ -33,9 +33,9 @@
>> PYTHON_INSTALL_DIR=$4
>> debug_flag=$5
>> 
>> -# Make sure SDKROOT is not set, since if it is this is an iOS build where python
>> +# Make sure LLDB_DISABLE_PYTHON is not set, since if it is this is an iOS build where python
>> # is disabled
>> -if [ "x$SDKROOT" = "x" ] ; then
>> +if [ ! $LLDB_DISABLE_PYTHON = "1" ] ; then
>> 
>> if [ -n "$debug_flag" -a "$debug_flag" == "-debug" ]
>> then
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu (mailto:lldb-commits at cs.uiuc.edu)
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 
> 
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits