You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@xbc.nu> on 2005/08/10 09:42:28 UTC
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
djames@tigris.org wrote:
>Author: djames
>Date: Tue Aug 9 20:57:37 2005
>New Revision: 15653
>
>Modified:
> branches/python-bindings-improvements/build/generator/gen_swig.py
>
>Log:
>* build/generator/gen_swig.py:
> (_get_swig_version): Fallback to os.popen on Python 1.x, where os.popen3
> is not available. This workaround only works on Unix systems because it
> uses the shell to redirect stderr.
>
>
Is this really a problem? gen_win.py has a find_swig function that
unconditionally ises os.popen4 to get the swig version. Nobody ever
complained.
And wouldn't it be nice if we had _one_ function in the generator for
finding the swig version?
I've finally found time to take a quick look at the generator changes
and I'm ... worried. gen_swig.py's generator inherits from the
unix-makefile generator, but a) contains windows-specific stuff and b)
contains utilities (such as $PATH search??? what on earth for?) that, at
best, belong in a generator utilities module or in gen_base.py.
This is the sort of spaghetti code we've been struggling to keep out of
the generator. Merging this to trunk as it is now would make me very
uncomfortable.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by Max Bowsher <ma...@ukf.net>.
David James wrote:
> It's rare to see Windows users who have Python 1.x installed. However,
> on Unix, it's common to have Python 1.x on your system, installed as
> "python". As such, it's a good idea to keep the Unix Makefile
> generators Python 1.x compatible.
Actually, we've been documenting (in INSTALL) that we require Python 2.0 for
a while. We've been preserving Python 1.5.2 support only on an unofficial
"let's do it if it's not too hard" basis. If there is significant code
cleanliness to be gained by using Python 2.0+ features, let's do that.
Python 1.x is absurdly ancient at this point.
Max.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by David James <ja...@gmail.com>.
On 8/10/05, David James <ja...@gmail.com> wrote:
> We could probably simplify the above code to:
>
> elif key == '--with-swig':
> dirs = [ "", value, os.path.join(value, "bin") ]
> self.swig_path = self._find_executable("swig", dirs)
(Oops. Here's a working implementation. It searches for the absolute
path "value". If that is not found, it searches in "value" and
"value/bin" for the swig executable. Since _find_executable is aware
of .exe extensions, it's not necessary to have logic regarding the
.exe extension here.)
elif key == '--with-swig':
dirs = [ value, os.path.join(value, "bin") ]
self.swig_path = self._find_executable(value, [""]) or \
self._find_executable("swig", dirs)
Cheers,
David
--
David James -- http://www.cs.toronto.edu/~james
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by Branko Čibej <br...@xbc.nu>.
David James wrote:
>On 8/22/05, Branko Čibej <br...@xbc.nu> wrote:
>
>
>>BTW, don't remove --with-swig from gen-make.py. Even though it's no
>>longer needed on Unix, we'll want it on Windows where we don't have a
>>configure step.
>>
>>
>This feature would be useful on Windows, but, in order for it to work,
>we need to patch gen_win.py to look at the "--with-swig" option. If I
>implemented this, would you have time to test it?
>
>
I'm doing it now, and will commit on trunk. So, just don't remove that
option to avoid merge conflicts, that's all.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15653 -
branches/python-bindings-improvements/build/generator
Posted by Philip Martin <ph...@codematters.co.uk>.
David James <ja...@gmail.com> writes:
> r15879, if it works on Windows. I'd also like to make sure it works
> under Philip's VPATH setup with multiple versions of SWIG before I
> merge.
I've not reviewed the code, but I tried it and "it works for me".
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by David James <ja...@gmail.com>.
On 8/22/05, Branko Čibej <br...@xbc.nu> wrote:> >I've addressed your concerns in r15855 on the> >python-bindings-improvements branch. Does this look good to you? I'm> >particularly interested in finding out whether this code works on> >Windows.> I see it's been merged to trunk. I'll test it soon.It hasn't been merged to trunk yet, but it looks ready to merge as ofr15879, if it works on Windows. I'd also like to make sure it worksunder Philip's VPATH setup with multiple versions of SWIG before Imerge.
> BTW, don't remove --with-swig from gen-make.py. Even though it's no> longer needed on Unix, we'll want it on Windows where we don't have a> configure step.This feature would be useful on Windows, but, in order for it to work,we need to patch gen_win.py to look at the "--with-swig" option. If Iimplemented this, would you have time to test it?
Cheers,
David
-- David James -- http://www.cs.toronto.edu/~james
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by Branko Čibej <br...@xbc.nu>.
David James wrote:
>On 8/10/05, David James <ja...@gmail.com> wrote:
>
>
>Hi Brane,
>
>I've addressed your concerns in r15855 on the
>python-bindings-improvements branch. Does this look good to you? I'm
>particularly interested in finding out whether this code works on
>Windows.
>
>
I see it's been merged to trunk. I'll test it soon.
BTW, don't remove --with-swig from gen-make.py. Even though it's no
longer needed on Unix, we'll want it on Windows where we don't have a
configure step.
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by David James <ja...@gmail.com>.
On 8/10/05, David James <ja...@gmail.com> wrote:> On 8/10/05, Branko Čibej <br...@xbc.nu> wrote:> > >Really? Where? This module is cross-platform, so it needs to work on> > >both Unix and Windows.> > I was thinking of the *.exe tracking, and write_long_long_fix.> Oh, I understand you now.> > Do you think that such code belongs in a separate, windows-specific> module? The "*.exe" tracking does not benefit Unix users, but it does> not hurt them either. The write_long_long_fix may potentially benefit> some Unix users, if they are using a compiler which does not support> "long long".> > > >>and b)> > >>contains utilities (such as $PATH search??? what on earth for?)> > >>> > >>> > >$PATH search can be useful for verifying the existence of an> > >executable. We currently use this feature to double check whether the> > >"apr-config" executable exists and is in your path. It's an easy> > >sanity check.> > >> > >> > And for the swig executable... but you don't need path searching for> > this. You simply run the program, and if it's in $PATH, it'll work; if> > it's not, it won't.> >> > If you see a --with-foo option, then of course you don't search the> > $PATH at all.> For SWIG, we don't search $PATH. We search a list of potential> directories where we think SWIG might be. If the user specifies> --with-swig=$VALUE, we search for SWIG in the following paths:> $VALUE, $VALUE/swig, $VALUE/swig/bin/swig> > If the user accidentally specifies the "directory" containing the SWIG> executable instead of the executable itself, this implementation will> *just work*. So there's no need for tech support. See the code that> implements this below:> > elif key == '--with-swig':> if self._is_executable(value):> self.swig_path = value> elif self._is_executable("%s.exe" % value):> self.swig_path = "%s.exe" % value> else:> dirs = [ value, os.path.join(value, "bin") ]> self.swig_path = self._find_executable("swig", dirs)> > We could probably simplify the above code to:> > elif key == '--with-swig':> dirs = [ "", value, os.path.join(value, "bin") ]> self.swig_path = self._find_executable("swig", dirs)> > > >>This is the sort of spaghetti code we've been struggling to keep out of> > >>the generator. Merging this to trunk as it is now would make me very> > >>uncomfortable.> > >>> > >>> > >I'm new to the Makefile generator, so I made some newbie design> > >mistakes. However, these are fortunately easy to fix. Here's my plan:> > >1. Move cross-platform path-finding functionality from gen_swig.py and> > >gen_win.py into a common base class, so that both Unix and Windows> > >implementations can benefit. I'll call this "gen_path.py".> > >2. Move SWIG file generation from gen_swig.py into a separate> > >command-line utility,> > >> > I suggest you make this loadable as a python module as well.> Will do.> > >> > > which can be called from both the Unix and the> > >Windows generators and makefiles. As a side-benefit, this change will> > >allow us to automatically regenerate the SWIG-wrappers for each ".h"> > >header file on-the-fly when the user types "make".> > >> > >> > O.K.> >> > >3. Move Makefile generation from gen_swig.py into gen_make.py. After> > >this, there'll be nothing left in gen_swig.py, so we can delete it.> > >> > >> > O.K.> > Great! I'll implement these ideas, then. I don't have the environment> to test changes to the Windows build system, so I will have to be> extra careful while refactoring. Your feedback is very helpful.
Hi Brane,
I've addressed your concerns in r15855 on thepython-bindings-improvements branch. Does this look good to you? I'mparticularly interested in finding out whether this code works onWindows.
Cheers,
David
-- David James -- http://www.cs.toronto.edu/~james
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by David James <ja...@gmail.com>.
On 8/10/05, Branko Čibej <br...@xbc.nu> wrote:> >Really? Where? This module is cross-platform, so it needs to work on> >both Unix and Windows.> I was thinking of the *.exe tracking, and write_long_long_fix.Oh, I understand you now.
Do you think that such code belongs in a separate, windows-specificmodule? The "*.exe" tracking does not benefit Unix users, but it doesnot hurt them either. The write_long_long_fix may potentially benefitsome Unix users, if they are using a compiler which does not support"long long".
> >>and b)> >>contains utilities (such as $PATH search??? what on earth for?)> >>> >>> >$PATH search can be useful for verifying the existence of an> >executable. We currently use this feature to double check whether the> >"apr-config" executable exists and is in your path. It's an easy> >sanity check.> >> >> And for the swig executable... but you don't need path searching for> this. You simply run the program, and if it's in $PATH, it'll work; if> it's not, it won't.> > If you see a --with-foo option, then of course you don't search the> $PATH at all.For SWIG, we don't search $PATH. We search a list of potentialdirectories where we think SWIG might be. If the user specifies--with-swig=$VALUE, we search for SWIG in the following paths: $VALUE, $VALUE/swig, $VALUE/swig/bin/swig
If the user accidentally specifies the "directory" containing the SWIGexecutable instead of the executable itself, this implementation will*just work*. So there's no need for tech support. See the code thatimplements this below:
elif key == '--with-swig': if self._is_executable(value): self.swig_path = value elif self._is_executable("%s.exe" % value): self.swig_path = "%s.exe" % value else: dirs = [ value, os.path.join(value, "bin") ] self.swig_path = self._find_executable("swig", dirs)
We could probably simplify the above code to:
elif key == '--with-swig': dirs = [ "", value, os.path.join(value, "bin") ] self.swig_path = self._find_executable("swig", dirs)
> >>This is the sort of spaghetti code we've been struggling to keep out of> >>the generator. Merging this to trunk as it is now would make me very> >>uncomfortable.> >>> >>> >I'm new to the Makefile generator, so I made some newbie design> >mistakes. However, these are fortunately easy to fix. Here's my plan:> >1. Move cross-platform path-finding functionality from gen_swig.py and> >gen_win.py into a common base class, so that both Unix and Windows> >implementations can benefit. I'll call this "gen_path.py".> >2. Move SWIG file generation from gen_swig.py into a separate> >command-line utility,> >> I suggest you make this loadable as a python module as well.Will do.
> > > which can be called from both the Unix and the> >Windows generators and makefiles. As a side-benefit, this change will> >allow us to automatically regenerate the SWIG-wrappers for each ".h"> >header file on-the-fly when the user types "make".> >> >> O.K.> > >3. Move Makefile generation from gen_swig.py into gen_make.py. After> >this, there'll be nothing left in gen_swig.py, so we can delete it.> >> >> O.K.
Great! I'll implement these ideas, then. I don't have the environmentto test changes to the Windows build system, so I will have to beextra careful while refactoring. Your feedback is very helpful.
Cheers,
David
-- David James -- http://www.cs.toronto.edu/~james
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by Branko Čibej <br...@xbc.nu>.
David James wrote:
>>but a) contains windows-specific stuff
>>
>>
>Really? Where? This module is cross-platform, so it needs to work on
>both Unix and Windows.
>
>
I was thinking of the *.exe tracking, and write_long_long_fix.
>>and b)
>>contains utilities (such as $PATH search??? what on earth for?)
>>
>>
>$PATH search can be useful for verifying the existence of an
>executable. We currently use this feature to double check whether the
>"apr-config" executable exists and is in your path. It's an easy
>sanity check.
>
>
And for the swig executable... but you don't need path searching for
this. You simply run the program, and if it's in $PATH, it'll work; if
it's not, it won't.
If you see a --with-foo option, then of course you don't search the
$PATH at all.
>>This is the sort of spaghetti code we've been struggling to keep out of
>>the generator. Merging this to trunk as it is now would make me very
>>uncomfortable.
>>
>>
>I'm new to the Makefile generator, so I made some newbie design
>mistakes. However, these are fortunately easy to fix. Here's my plan:
>1. Move cross-platform path-finding functionality from gen_swig.py and
>gen_win.py into a common base class, so that both Unix and Windows
>implementations can benefit. I'll call this "gen_path.py".
>2. Move SWIG file generation from gen_swig.py into a separate
>command-line utility,
>
I suggest you make this loadable as a python module as well.
> which can be called from both the Unix and the
>Windows generators and makefiles. As a side-benefit, this change will
>allow us to automatically regenerate the SWIG-wrappers for each ".h"
>header file on-the-fly when the user types "make".
>
>
O.K.
>3. Move Makefile generation from gen_swig.py into gen_make.py. After
>this, there'll be nothing left in gen_swig.py, so we can delete it.
>
>
O.K.
>Thanks for your help!
>
>
Yeah, sorry I didn't look at this earlier, but time is something other
people have. :-(
-- Brane
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r15653 - branches/python-bindings-improvements/build/generator
Posted by David James <ja...@gmail.com>.
Hi Branko,
Thanks for all your feedback! This is very helpful. More details below.
On 8/10/05, Branko Čibej <br...@xbc.nu> wrote:> Is this really a problem? gen_win.py has a find_swig function that> unconditionally ises os.popen4 to get the swig version. Nobody ever> complained.It's rare to see Windows users who have Python 1.x installed. However,on Unix, it's common to have Python 1.x on your system, installed as"python". As such, it's a good idea to keep the Unix Makefilegenerators Python 1.x compatible.
> And wouldn't it be nice if we had _one_ function in the generator for> finding the swig version?Absolutely. I didn't notice the "windows" implementation of find_swigwhen I first wrote this function. It'd be much better for both theUnix and Windows implementations to inherit from a sharedcross-platform implementation. I'll implement this.
> I've finally found time to take a quick look at the generator changes> and I'm ... worried. gen_swig.py's generator inherits from the> unix-makefile generatorOops. This file doesn't use any Unix specific functionality, so itwould work fine if it inherited from "gen_base" instead of gen_make.
> but a) contains windows-specific stuffReally? Where? This module is cross-platform, so it needs to work onboth Unix and Windows.
> and b)> contains utilities (such as $PATH search??? what on earth for?)$PATH search can be useful for verifying the existence of anexecutable. We currently use this feature to double check whether the"apr-config" executable exists and is in your path. It's an easysanity check.
> that, at> best, belong in a generator utilities module or in gen_base.py.Definitely! This code is cross-platform, so it really belongs ineither a generator utilities module or in gen_base.py.
> This is the sort of spaghetti code we've been struggling to keep out of> the generator. Merging this to trunk as it is now would make me very> uncomfortable.I'm new to the Makefile generator, so I made some newbie designmistakes. However, these are fortunately easy to fix. Here's my plan:1. Move cross-platform path-finding functionality from gen_swig.py andgen_win.py into a common base class, so that both Unix and Windowsimplementations can benefit. I'll call this "gen_path.py".2. Move SWIG file generation from gen_swig.py into a separatecommand-line utility, which can be called from both the Unix and theWindows generators and makefiles. As a side-benefit, this change willallow us to automatically regenerate the SWIG-wrappers for each ".h"header file on-the-fly when the user types "make".3. Move Makefile generation from gen_swig.py into gen_make.py. Afterthis, there'll be nothing left in gen_swig.py, so we can delete it.
Thanks for your help!
David
-- David James -- http://www.cs.toronto.edu/~james