[Distutils] setuptools special case Pyrex and break Cython

[Stefan Behnel]

Phillip J. Eby wrote:
> At 08:00 PM 9/5/2007 +0200, Stefan Behnel wrote:
> 
>> Phillip J. Eby wrote:
>> > At 08:30 AM 9/5/2007 +0200, Stefan Behnel wrote:
>> >> > Perhaps you'd care to produce a patch to implement that "cleaner
>> step"?
>> >> > It's not at all obvious to me how to do that without introducing
>> >> > instability that would be unsuitable for an 0.6cN release.
>> >>
>> >> One way of implementing the above change would be to move the
>> >> replacement code
>> >> into build_ext rather than Extension. Something like the (untested)
>> >> build_ext-patcher.py I attached. Note the type check that tests for
>> >> build_ext
>> >> being subclassed.
>> >
>> > You're illustrating my point.  It's easy to hand-wave about how it
>> > should be done, but not so easy to actually *do*.  Did you look at
>> where
>> > all the .sources attribute gets used?  How the build_ext command can
>> get
>> > called by other commands?
>>
>> Sort of. Do you doubt it should work that way?
> 
> I know that other commands use the .sources of build_ext indirectly, at
> a time when run() has not yet been called.  That means that your
> approach may change those other commands' behavior in non-obvious ways. 

I bet there's a method that you can safely override to do what I meant. Might
take me a few minutes longer to find than you, but I actually doubt I
currently want to invest that. You shouldn't forget that it's your code that's
broken.


>> > You're also ignoring my larger point: the current mechanism allows you
>> > to write setup scripts that *don't* need to subclass build_ext.  A
>> > setuptools-based setup script just refers to '.pyx' files, and
>> > everything else happens automatically.
>>
>> The script I posted doesn't change that. Read it.
> 
> Actually, now that I've read it in detail, I see that it's also broken
> in other ways, such as confusing modules and classes.

I guess you meant the sys.modules bit. That should be easy to fix. It's
untested, as I said. It was just meant to make clear what I was proposing.


> But it's also broken in the way I said: it *requires* subclassing of
> build_ext in setup.py, which the current mechanism does not.

No, it just prevents .pyx file mangling *if* you have subclassed build_ext.
"If" meaning: "in the case that", so it's a thing that may be, but doesn't
have to be. Not very close to the definition of "requires".

*If* build_ext was subclassed, doing nothing is a safe bet, as a subclass can
do whatever it likes and should not suffer from setuptools patching stuff it
might not even know about. If you do not subclass, it behaves as before
(probably minus some problems, as you suggested above).


>> You're actually forgetting the point you stressed most. If we prevent
>> users
>> from installing Cython and Pyrex at the same time, they will have to
>> start
>> modifying setup.py scripts. So it's pushing the burden on them.
> 
> Actually, I think you need to decide whether you are really providing an
> extended Pyrex compiler.  If so, then use .pyx as the extension and
> include a Pyrex-compatible build_ext module, and then there is no need
> to have both installed on the same system, and everything works
> splendidly for everyone.
> 
> If on the other hand you are *not* providing a Pyrex replacement and
> must co-exist, then use a different file extension (.cy, perhaps?) and
> you get exactly the behavior you wanted -- i.e., setuptools doesn't
> touch your files, and explicit action is required to change the file
> extensions.
> 
> And there is still a third solution: provide your own Extension class
> for users to import, since they will have to import from Cython in order
> to use your extended build_ext class anyway, nothing stops them from
> importing the Extension type too.  (Just have your Extension.__init__
> save the sources and restore them after calling the superclass; it'll
> then work whether the patched version is present or not.)

Patch wars, eh? :)

You're still forgetting that that pushes the burden on each writer of a
setup.py script. They have to take caution to support Pyrex *and* Cython (*if*
they want to, which is currently up to them), and they have to care about the
case where none of them is importable to get a build_ext replacement, which is
required by both Pyrex and Cython. I still can't see the case where your
extension.py patching becomes truly beneficial to anyone. It would be if
people really had to do nothing, but that's not the case.

But maybe we should just start shipping Cython with a fake Pyrex that people
can import and still get Cython instead. Although that doesn't really fix the
bug in setuptools. But then, that's your code.

Stefan