classification
Title: distutils compiler switch ignored
Type: behavior Stage: test needed
Components: Distutils Versions: Python 3.5, Python 3.4, Python 2.7
process
Status: open Resolution: accepted
Dependencies: Superseder:
Assigned To: tarek Nosy List: atuining, benjamin.peterson, cmcqueen1975, dalcinl, lemburg, merwok, nicdumz, rpetrov, tarek
Priority: high Keywords: patch

Created on 2009-06-29 18:22 by atuining, last changed 2015-04-03 00:23 by Jean-Paul Calderone.

Files
File name Uploaded Description Edit
distutils_compiler.patch atuining, 2009-06-29 18:22 Patch to fix --compiler switch
compiler.diff tarek, 2009-07-05 14:14
Messages (26)
msg89871 - (view) Author: Anthony Tuininga (atuining) * Date: 2009-06-29 18:22
With the release of Python 3.1 the --compiler switch is ignored in
Lib/distutils/command/build_ext.py. The attached patch fixes that issue.
Once that was fixed there was another issue with get_version() in
cygwincompiler but that appears to be fixed in the 3.1 branch. The
revision that introduced this problem is 72596 (from 72593 in trunk).
msg90045 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-03 07:57
yes, the problem is that this option (string) is also used as an
attribute (compiler object).

But Python itself uses as an attribute..

I have to check its type at this spot, and deprecate the usage of
"compiler" as an attribute.
msg90154 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-05 14:14
Here's the patch. There's no simple way to deprecate the usage of
"compiler" as an attribute, so I have just fixed Python setup.py.
Using it as an attribute is just wrong.

I have put Benjamin in the loop and I'll wait for his greenlight to
commit this change.
msg90155 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2009-07-05 14:41
I think the patch is fine. (Did you try using the descriptor protocol to
add a deprecation warning?)
msg90156 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-05 15:08
Right, turning compiler into a property and adding a warning on the set
would be good enough I guess. Then turn it back into a simple string for
python 3.3 and ... 2.8 :)

I'll add that
msg90176 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-06 12:51
done in r73864, waiting for the buildbots to build trunk, then will be
applied in 3.x
msg90179 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-06 13:55
merged in r73866 in py3k


Thanks for the feedback
msg90274 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2009-07-08 16:53
It looks like this change may have broken some parts of distutils.  For
example, Twisted's setup.py now produces this output when running the
build_ext command:

$ ~/Projects/python/trunk/python setup.py build_ext
running build_ext
Traceback (most recent call last):
  File "setup.py", line 97, in <module>
    main(sys.argv[1:])
  File "setup.py", line 92, in main
    setup(**setup_args)
  File "./twisted/python/dist.py", line 47, in setup
    return core.setup(**get_setup_args(**kw))
  File "/home/exarkun/Projects/python/trunk/Lib/distutils/core.py", line
149, in setup
    dist.run_commands()
  File "/home/exarkun/Projects/python/trunk/Lib/distutils/dist.py", line
926, in run_commands
    self.run_command(cmd)
  File "/home/exarkun/Projects/python/trunk/Lib/distutils/dist.py", line
945, in run_command
    cmd_obj.run()
  File
"/home/exarkun/Projects/python/trunk/Lib/distutils/command/build_ext.py", line
380, in run
    self.build_extensions()
  File "./twisted/python/dist.py", line 327, in build_extensions
    self.prepare_extensions()
  File "./twisted/python/dist.py", line 318, in prepare_extensions
    if x.condition(self)]
  File "twisted/runner/topfiles/setup.py", line 14, in <lambda>
    condition=lambda builder: builder._check_header("rpc/rpc.h")),
  File "./twisted/python/dist.py", line 359, in _check_header
    self.compiler.announce("checking for %s ..." % header_name, 0)
AttributeError: 'NoneType' object has no attribute 'announce'

And PyCrypto produces this output:

running build_ext
Traceback (most recent call last):
  File "setup.py", line 163, in <module>
    core.setup(**kw)
  File "/tmp/python-buildbot/local/lib/python2.7/distutils/core.py",
line 149, in setup
    dist.run_commands()
  File "/tmp/python-buildbot/local/lib/python2.7/distutils/dist.py",
line 926, in run_commands
    self.run_command(cmd)
  File "/tmp/python-buildbot/local/lib/python2.7/distutils/dist.py",
line 945, in run_command
    cmd_obj.run()
  File
"/tmp/python-buildbot/local/lib/python2.7/distutils/command/install.py",
line 580, in run
    self.run_command('build')
  File "/tmp/python-buildbot/local/lib/python2.7/distutils/cmd.py", line
326, in run_command
    self.distribution.run_command(command)
  File "/tmp/python-buildbot/local/lib/python2.7/distutils/dist.py",
line 945, in run_command
    cmd_obj.run()
  File
"/tmp/python-buildbot/local/lib/python2.7/distutils/command/build.py",
line 132, in run
    self.run_command(cmd_name)
  File "/tmp/python-buildbot/local/lib/python2.7/distutils/cmd.py", line
326, in run_command
    self.distribution.run_command(command)
  File "/tmp/python-buildbot/local/lib/python2.7/distutils/dist.py",
line 945, in run_command
    cmd_obj.run()
  File
"/tmp/python-buildbot/local/lib/python2.7/distutils/command/build_ext.py",
line 380, in run
    self.build_extensions()
  File "setup.py", line 115, in build_extensions
    self.detect_modules()
  File "setup.py", line 119, in detect_modules
    lib_dirs = self.compiler.library_dirs + ['/lib', '/usr/lib']
AttributeError: 'NoneType' object has no attribute 'library_dirs'
msg90276 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-08 18:24
Tarek, the .compiler attribute is needed by bdist_ext, so cannot just be
removed or renamed to .compiler_obj. There's a lot of bdist_ext
distutils code out there relying on having the .compiler object available.

A much better fix would be to map the option to a new attribute in
build_ext, e.g. .compiler_name or to remove the option altogether and
always inherit the setting from the global dist option --compiler.

In general, when making such changes, please test these against a few
well-known packages such as twisted, pycrypto or egenix-mx-base which do
include C extensions.

Thanks.
msg90277 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-08 18:50
Note that the config command also uses a .compiler instance for actually
doing work, rather than as command option.
msg90278 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-08 18:51
Added Python 2.7 since it fails there as well.
msg90279 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-08 18:56
Marc-Andre Lemburg wrote:
> Tarek, the .compiler attribute is needed by bdist_ext, so cannot just be
> removed or renamed to .compiler_obj. There's a lot of bdist_ext
> distutils code out there relying on having the .compiler object available.

Sorry, the above should have read build_ext, not bdist_ext...

> A much better fix would be to map the option to a new attribute in
> build_ext, e.g. .compiler_name or to remove the option altogether and
> always inherit the setting from the global dist option --compiler.
msg90280 - (view) Author: Roumen Petrov (rpetrov) * Date: 2009-07-08 19:39
Trunk may be is not affected. I successfully cross-compile with GNU
compiler for windows (see issue 3871).
msg90281 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-08 19:45
Roumen Petrov wrote:
> Roumen Petrov <bugtrack@roumenpetrov.info> added the comment:
> 
> Trunk may be is not affected. I successfully cross-compile with GNU
> compiler for windows (see issue 3871).

It is affected in the sense that .compile was changed
to .compile_obj and that change breaks 3rd party setup.py
code which uses the compiler instance to run its own
compilations or needs to tweak the setup by e.g. adding
more libs or dirs to its configuration.
msg90284 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-08 21:46
I'll set back the compiler attribute when compiler_obj is set too, 
so third-party code will be able to work with it as before.

The current code will deprecate this usage, by displaying a deprecation
warning:

- if the compiler is set to anything else than a string.
- if the compiler is get and happens to be a compiler instance.

so we can keep "compiler" as its initial purpose.

And I'll test it on twisted and egenix. Thanks for the feedback
msg90289 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-08 22:45
done in r73895, r73896. (and tested with twisted trunk).
msg90299 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2009-07-09 02:16
Cool, thanks.  PyCrypto also works again now.
msg90309 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-09 07:55
Tarek Ziadé wrote:
> Tarek Ziadé <ziade.tarek@gmail.com> added the comment:
> 
> I'll set back the compiler attribute when compiler_obj is set too, 
> so third-party code will be able to work with it as before.
> 
> The current code will deprecate this usage, by displaying a deprecation
> warning:
> 
> - if the compiler is set to anything else than a string.
> - if the compiler is get and happens to be a compiler instance.
> 
> so we can keep "compiler" as its initial purpose.

Could you please elaborate a bit on the reasoning behind
deprecating using .compiler for the compiler instance ?

The code did work before, so I'm not sure why you are trying
to fix something that wasn't really broken.

With the change you:

 * make the code more complex just to be able to raise
   a warning

 * introduce an cross-version incompatibility for tools
   using build_ext: they will now have to use .compiler
   for Python 2.3-2.6 and .compiler_obj for 2.7 and up

Wouldn't it be better to either leave things are they have
been for years (without problems) or find another solution ?
msg90313 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-09 08:12
The build_ext command cannot be run twice, because the first time, the
"compiler" option may be set to "unix" for example, or left to None, and
then is transformed into a compiler object. That's the bug.

If you call it again, it'll break because the new_compiler() function
will receive self.compiler, which is supposed to be a string not a
compiler object.

"compiler" is described as the "compiler type" in the options list of
build_ext and should receive a string value.

So what's broken is the fact that third-party code is using "compiler" 
as a compiler object attribute for years, because the command was
creating the compiler object on the compiler option, rather than using
its own attribute for that. And from an architectural point of view, if
you have to tweak the compiler options by tweaking them direclty, it
means that the build_ext command did a bad job with in the options
it provides.

For the cross-version incompatibility you are mentioning, it means that
your code is working with "compiler" as a string option, then continue
to work with it as a compiler object right after the command is run.
But either way, there will be an incompatibility starting at 3.3 because
we are going from the state "compiler is a string and also a compiler
object, depending on when you use it" to "compiler is a only string option"

This is an inconsistent behavior I am fixing here. While the code may be
more complex with the descriptor, this will eventually go away in 3.3
(and 2.8 if it exists).

What is the other solution you where thinking about ? adding a
compiler_type option and keep the compiler option woud introduce more
incompatibilities since "compiler" is used to configure build_ext by
many code out there, while tweaking the compiler object itself is an
advanced usage done by less people, which may do the same thing in an
easy way by using another attribute.
msg90315 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-09 08:47
Tarek Ziadé wrote:
> Tarek Ziadé <ziade.tarek@gmail.com> added the comment:
> 
> The build_ext command cannot be run twice, because the first time, the
> "compiler" option may be set to "unix" for example, or left to None, and
> then is transformed into a compiler object. That's the bug.
> 
> If you call it again, it'll break because the new_compiler() function
> will receive self.compiler, which is supposed to be a string not a
> compiler object.

You never run a command twice unless you explicitly reinitialize it
(which then resets .compiler to None and then fetches the command line
option again), so the above is not a problem.

> "compiler" is described as the "compiler type" in the options list of
> build_ext and should receive a string value.
> 
> So what's broken is the fact that third-party code is using "compiler" 
> as a compiler object attribute for years, because the command was
> creating the compiler object on the compiler option, rather than using
> its own attribute for that. And from an architectural point of view, if
> you have to tweak the compiler options by tweaking them direclty, it
> means that the build_ext command did a bad job with in the options
> it provides.

I agree that it's not exactly ideal to have an attribute first
be a string and then an object.

> For the cross-version incompatibility you are mentioning, it means that
> your code is working with "compiler" as a string option, then continue
> to work with it as a compiler object right after the command is run.

Not quite: extensions of build_ext will likely customize the
way the extension objects are built, ie. override .build_extensions()
which is called after .compiler has been set to the compiler instance.

So they always work with the compiler instance. And they don't really care
about the option at all, since all that information is available from
looking at the compiler object.

> But either way, there will be an incompatibility starting at 3.3 because
> we are going from the state "compiler is a string and also a compiler
> object, depending on when you use it" to "compiler is a only string option"
> 
> This is an inconsistent behavior I am fixing here. While the code may be
> more complex with the descriptor, this will eventually go away in 3.3
> (and 2.8 if it exists).
> 
> What is the other solution you where thinking about ? adding a
> compiler_type option and keep the compiler option woud introduce more
> incompatibilities since "compiler" is used to configure build_ext by
> many code out there, while tweaking the compiler object itself is an
> advanced usage done by less people, which may do the same thing in an
> easy way by using another attribute.

First of all, you only use a single compiler for building an
Python package, so using the global build compiler option will do
just fine (and is also required if you have other commands
rely on this information as well, such as the config command,
the build_clib command or other custom commands).

The "compiler" option on the build_ext and config commands
are not really needed. Their .finalize_options() methods could
easily pull in the build option value and place it into
an .compiler_type attribute which then gets used as basis for
creating the .compiler instance, or even better refactor the
various commands to use a central method on the build
command object to create a compiler object and avoid all
the copy&paste code for this.

Furthermore, the .finalize_options() methods could detect whether
a per-command option as used and deprecate this use instead,
redirecting to the build command option.

Please note that it's common practice in distutils to have
the compiler instance in an .compiler attribute, so either
you change it in all cases or not at all.

The fact that the options on some of those commands is
named --compiler as well, is rather unfortunate.

Regarding wide-spread use of the compiler command line option:
I am only aware of the cygwin/mingw32 case where you'd really
need it. In all other cases I know, the default choice based
on the compiler with which Python itself was compiled will
work fine.
msg90316 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-09 09:18
FWIW: I've changed our mxSetup code to use a method for accessing
the compiler instance.

Perhaps that's the better way to go for standard distutils commands
as well ?! E.g. .get_compiler_object()
msg90334 - (view) Author: Nicolas Dumazet (nicdumz) Date: 2009-07-09 17:07
It seems that the fix is still not perfect.

At the moment ( r73906 ), if you try to build trunk using Python 2.6,
you get:

> python setup.py build
running build
running build_ext
Traceback (most recent call last):
  File "setup.py", line 1901, in <module>
    main()
  File "setup.py", line 1896, in main
    'Lib/smtpd.py']
  File "/usr/lib/python2.6/distutils/core.py", line 152, in setup
    dist.run_commands()
  File "/usr/lib/python2.6/distutils/dist.py", line 975, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python2.6/distutils/dist.py", line 995, in run_command
    cmd_obj.run()
  File "/usr/lib/python2.6/distutils/command/build.py", line 135, in run
    self.run_command(cmd_name)
  File "/usr/lib/python2.6/distutils/cmd.py", line 333, in run_command
    self.distribution.run_command(command)
  File "/usr/lib/python2.6/distutils/dist.py", line 995, in run_command
    cmd_obj.run()
  File "/usr/lib/python2.6/distutils/command/build_ext.py", line 345, in run
    self.build_extensions()
  File "setup.py", line 103, in build_extensions
    missing = self.detect_modules()
  File "setup.py", line 302, in detect_modules
    add_dir_to_list(self.compiler_obj.library_dirs, '/usr/local/lib')
  File "/usr/lib/python2.6/distutils/cmd.py", line 112, in __getattr__
    raise AttributeError, attr
AttributeError: compiler_obj
msg90382 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-10 09:19
@Nicolas : That's because you run it with Python 2.6 distutils, which
doesn't have that change.

If you want to build the current trunk with Python 2.6, you may want to
install a standalone version of distutils. I have a nightly build of the
trunk you may install in 2.6 for this: 

http://nightly.ziade.org/

By the time 2.7 comes out, I'll probably publish an official standalone
version for 2.6 as Marc André suggested in the past.
msg90383 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-10 09:34
> You never run a command twice unless you explicitly reinitialize it
> (which then resets .compiler to None and then fetches the command line
> option again), so the above is not a problem.

In practice yes that's true.

> The "compiler" option on the build_ext and config commands
> are not really needed. Their .finalize_options() methods could
> easily pull in the build option value and place it into
> an .compiler_type attribute which then gets used as basis for
> creating the .compiler instance, or even better refactor the
> various commands to use a central method on the build
> command object to create a compiler object and avoid all
> the copy&paste code for this.
> Furthermore, the .finalize_options() methods could detect whether
> a per-command option as used and deprecate this use instead,
> redirecting to the build command option.

Having a single location sounds like the best idea with the current
behavior, reachable from cmd.get_compiler_object() like you did.

Now the question is, in practice, could someone force a
different compiler in build_ext for instance, or with a different
configuration than in build_clib ?

I don't see a use case in practice for that, but if so, we would need to
keep a different compiler instance per command.
msg90385 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2009-07-10 10:34
Tarek Ziadé wrote:
>> The "compiler" option on the build_ext and config commands
>> are not really needed. Their .finalize_options() methods could
>> easily pull in the build option value and place it into
>> an .compiler_type attribute which then gets used as basis for
>> creating the .compiler instance, or even better refactor the
>> various commands to use a central method on the build
>> command object to create a compiler object and avoid all
>> the copy&paste code for this.
>> Furthermore, the .finalize_options() methods could detect whether
>> a per-command option as used and deprecate this use instead,
>> redirecting to the build command option.
> 
> Having a single location sounds like the best idea with the current
> behavior, reachable from cmd.get_compiler_object() like you did.
> 
> Now the question is, in practice, could someone force a
> different compiler in build_ext for instance, or with a different
> configuration than in build_clib ?
> 
> I don't see a use case in practice for that, but if so, we would need to
> keep a different compiler instance per command.

It is generally a bad idea to mix compiler types when compiling
Python extensions and usually doesn't work unless you are
very careful.

In practice, I don't think that any extension package will use
more than one compiler type for the various build parts.

However, it is well possible that a package may use differently
setup compiler instances on various commands, e.g. to point
it to different libraries, dirs, etc., so having just
one such instance on e.g. the top-level build command object
would not work out.

In fact, a single command object may even use multiple compiler
instances to e.g. build different C libs or extensions using
different sets of include and library search dirs.

I think a workable solution to the problem with the compiler
option would be to remove the option from the build_ext,
build_clib and config commands (plus any others, if there are
more) and only allow it on the build command.

The build.get_compiler_object() could then return an instance
of the chosen compiler type and the command object would then
store it in .compiler.

The various .finalize_options() method would need to propagate
any used "compiler" option back to the build command and reset
the .compiler attribute to None.

You'd still have the situation that .compiler is used as
option string and then as compiler instance, but only for
the short phase between .initialize_options() and
.finalize_options() which is not all that much of a problem
(you cannot make any use of non-finalized command objects
anyway).
msg90439 - (view) Author: Tarek Ziadé (tarek) * (Python committer) Date: 2009-07-12 07:32
> I think a workable solution to the problem with the compiler
> option would be to remove the option from the build_ext,
> build_clib and config commands (plus any others, if there are
> more) and only allow it on the build command.

The problem I see is that sometimes, people are using the build_ext
command on its own:

  $ python setup.py build_ext -i --compiler mingw32

So that means build_ext is not a subcommand of build in this case.
While we can instanciate a build command on the fly to get our compiler
and work with it, using get_command_obj(). 

But the code will have to be careful and not use the cache with this
method e.g. to re-create a new build command to avoid using the same
command instance if another build_ is called.

What about making it all simpler, by creating a base command class that
manages a compiler through some methods (like get_compiler_obj), then
make all our build+build_* command inherit from it ?

That would resolve the code duplication and will make it simpler each
command to deal with a compiler.

This base command would have one single option. e.g. "compiler" 


> You'd still have the situation that .compiler is used as
> option string and then as compiler instance, but only for
> the short phase between .initialize_options() and
> .finalize_options() which is not all that much of a problem

I am not sure to understand how .compiler will become a string 
again after .finalize_options() has been called. Could you provide 
an example ?
History
Date User Action Args
2015-04-03 00:23:17Jean-Paul Calderonesetnosy: - exarkun
2014-07-09 21:49:51BreamoreBoysetversions: + Python 3.4, Python 3.5, - Python 3.1
2010-12-14 02:41:50r.david.murraysetnosy: + merwok

type: behavior
stage: test needed
2009-12-16 02:26:01cmcqueen1975setnosy: + cmcqueen1975
2009-11-02 22:23:34tareksetnosy: + dalcinl
2009-09-07 08:12:04tareksetpriority: high
2009-07-12 07:32:33tareksetmessages: + msg90439
2009-07-10 10:34:52lemburgsetmessages: + msg90385
2009-07-10 09:34:42tareksetmessages: + msg90383
2009-07-10 09:19:47tareksetmessages: + msg90382
2009-07-09 17:07:01nicdumzsetnosy: + nicdumz
messages: + msg90334
2009-07-09 09:18:25lemburgsetmessages: + msg90316
2009-07-09 08:47:55lemburgsetmessages: + msg90315
2009-07-09 08:12:44tareksetmessages: + msg90313
2009-07-09 07:55:14lemburgsetmessages: + msg90309
2009-07-09 02:16:12exarkunsetmessages: + msg90299
2009-07-08 22:45:38tareksetmessages: + msg90289
2009-07-08 21:46:42tareksetstatus: closed -> open

messages: + msg90284
2009-07-08 19:45:08lemburgsetmessages: + msg90281
2009-07-08 19:39:57rpetrovsetnosy: + rpetrov
messages: + msg90280
2009-07-08 18:56:03lemburgsetmessages: + msg90279
2009-07-08 18:51:20lemburgsetmessages: + msg90278
versions: + Python 2.7
2009-07-08 18:50:54lemburgsetmessages: + msg90277
2009-07-08 18:24:31lemburgsetnosy: + lemburg
messages: + msg90276
2009-07-08 16:53:55exarkunsetnosy: + exarkun
messages: + msg90274
2009-07-06 13:55:44tareksetstatus: open -> closed

messages: + msg90179
2009-07-06 12:51:27tareksetmessages: + msg90176
2009-07-05 15:08:48tareksetmessages: + msg90156
2009-07-05 14:41:58benjamin.petersonsetmessages: + msg90155
2009-07-05 14:14:14tareksetfiles: + compiler.diff
nosy: + benjamin.peterson
messages: + msg90154

2009-07-03 07:57:02tareksetresolution: accepted
messages: + msg90045
2009-06-29 18:22:19atuiningcreate