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

[Filipe Cabecinhas]

Yes, that way is better. Since I was only looking at the diff, I spoke about another variable, assuming the comment was correct. The new version is better :-)  

Thanks,  

  Filipe


On Friday, May 25, 2012 at 4:51 PM, Jim Ingham wrote:

> 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 (http://finish-swig-Python-LLDB.sh)
> ===================================================================
> --- scripts/Python/finish-swig-Python-LLDB.sh (http://finish-swig-Python-LLDB.sh) (revision 157474)
> +++ scripts/Python/finish-swig-Python-LLDB.sh (http://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 (http://build-swig-Python.sh)
> ===================================================================
> --- scripts/Python/build-swig-Python.sh (http://build-swig-Python.sh) (revision 157474)
> +++ scripts/Python/build-swig-Python.sh (http://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 (mailto: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 (mailto:lldb-commits at cs.uiuc.edu)
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>