[PATCHv2 1-14/14] Re: Disabling Python Modules

[Andrew Bartlett]

On Sun, 2017-01-29 at 22:52 -0500, Ian Stakenvicius wrote:
> > On Jan 29, 2017, at 10:08 PM, Andrew Bartlett <abartlet at samba.org>
> > wrote:
> > 
> > > On Sun, 2017-01-29 at 14:28 -0500, Ian Stakenvicius wrote:
> > > > On 29/01/17 02:14 PM, Andrew Bartlett wrote:
> > > > 
> > > > I can't get this to build when I set --disable-python.
> > > > 
> > > > I get "ADDC requires python" even if I set --without-ad-dc,
> > > > sorry. 
> > > > Additionally the error string needs to be modified to be "
> > > > --disable-
> > > > python requires --without-ad-dc".
> > > > 
> > > > The attached autobuild patch will keep this working, once we
> > > > sort
> > > > out
> > > > what is going on here.
> > > 
> > > Yes, this is a mistake on my part -- I was using
> > > conf.env.without_ad_dc to obtain the configured value, but that
> > > is
> > > never set from Options.options and so is always false.  I've
> > > adjusted
> > > that to use Options.options.without_ad_dc directly in the same
> > > manner
> > > that it is used later to set the #define.
> > > 
> > > I've also adjusted the error message, and incorporated the pep8
> > > fixes
> > > from the previous email.
> > 
> > I have two issues:
> > 
> > With:
> > 
> > ./configure.developer  --picky-developer --
> > prefix=/tmp/b9407/prefix/samba-nopython  --with-profiling-data  
> > --without-ad-dc --disable-python && make -j
> > 
> > I now get:
> > 
> > In file included from ../lib/replace/replace.h:32:0,
> >                  from ../dynconfig/dynconfig.c:41:
> > default/include/config.h:431:0: error: "PYTHONDIR" redefined [-
> > Werror]
> >  #define PYTHONDIR "/tmp/b9407/prefix/samba-
> > nopython/lib/python2.7/site-packages"
> >  
> > <command-line>:0:0: note: this is the location of the previous
> > definition
> > In file included from ../lib/replace/replace.h:32:0,
> >                  from ../dynconfig/dynconfig.c:41:
> > default/include/config.h:432:0: error: "PYTHONARCHDIR" redefined [-
> > Werror]
> >  #define PYTHONARCHDIR "/tmp/b9407/prefix/samba-
> > nopython/lib64/python2.7/site-packages"
> >  
> > <command-line>:0:0: note: this is the location of the previous
> > definition
> > cc1: all warnings being treated as errors
> > Waf: Leaving directory `/data/samba/git/samba-push/bin'
> > Build failed:  -> task failed (err #1): 
> >    {task: cc dynconfig.c -> dynconfig_1.o}
> > Makefile:8: recipe for target 'all' failed
> > 
> 
> 
> This is the type of thing I saw when there is code depending on
> modules that are disabled.  I'll see what I can do about tracing this
> and any others to the module(s) that need disabling.

Thanks.  I wonder if it due to still running the python headers check?

> > The second is that 
> > 
> > With:
> > ./configure.developer  --picky-developer --
> > prefix=/tmp/b9407/prefix/samba-nopython  --with-profiling-data  
> > --without-ad-dc --extra-python=/usr/bin/python3 --disable-python &&
> > make -j
> > 
> > I get:
> > 
> > ERROR: Target 'pytalloc-util' in directory /data/samba/git/samba-
> > push/lib/talloc re-defined as DISABLED - was DISABLED
> > 
> > Now this is SILLY, because I'm specifying '--extra-
> > python=/usr/bin/python3 --disable-python' but it is what the
> > autobuild
> > patch I gave does, due to building up options in the list.  The
> > issue
> > here is the gen_python_environments() loops.
> > 
> > For this we do need a sensible error about the conflict (and then
> > patch
> > the autobuild patch). 
> 
> What are your thoughts about, in this particular case, just warning
> on config about --extra-python doing nothing when --disable-python is
> set, and forcing the extra-python list to the empty set?  If --extra-
> python turns into some sort of autodetected or pre-defined list in
> waf then it'd be beneficial down the road.

An error and failure to configure is what we need here.

> > 
> > Finally, please do run:
> > 
> > ./script/autobuild.py samba-nopython --testbase=/tmp samba-o3
> > 
> > This will catch a few more of these kind of errors and save some
> > round-
> > trips.  All patches have to pass that regardless.  You might even
> > be
> > able to get github to run that for you in travis ci if you make
> > pull
> > requests or register with travis ci.
> > 
> 
> Thanks, travis via PRs is a great idea, I'll set that up.  

Great!

Andrew Bartlett

-- 
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba