On 8/10/05, David James <james82@gmail.com> wrote:> On 8/10/05, Branko Èibej <brane@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 pro
gram, 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 automat
ically 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
Received on Sun Aug 21 08:56:13 2005