Issue6377
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2009-06-29 18:22 by atuining, last changed 2022-04-11 14:56 by admin. This issue is now closed.
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 (27) | |||
---|---|---|---|
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) * | 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) * | 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) * | 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) * | 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) * | 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) * | Date: 2009-07-06 13:55 | |
merged in r73866 in py3k Thanks for the feedback |
|||
msg90274 - (view) | Author: Jean-Paul Calderone (exarkun) * | 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) * | 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) * | 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) * | Date: 2009-07-08 18:51 | |
Added Python 2.7 since it fails there as well. |
|||
msg90279 - (view) | Author: Marc-Andre Lemburg (lemburg) * | 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) * | 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) * | 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) * | Date: 2009-07-08 22:45 | |
done in r73895, r73896. (and tested with twisted trunk). |
|||
msg90299 - (view) | Author: Jean-Paul Calderone (exarkun) * | Date: 2009-07-09 02:16 | |
Cool, thanks. PyCrypto also works again now. |
|||
msg90309 - (view) | Author: Marc-Andre Lemburg (lemburg) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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) * | 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 ? |
|||
msg386387 - (view) | Author: Steve Dower (steve.dower) * | Date: 2021-02-03 18:26 | |
Distutils is now deprecated (see PEP 632) and all tagged issues are being closed. From now until removal, only release blocking issues will be considered for distutils. If this issue does not relate to distutils, please remove the component and reopen it. If you believe it still requires a fix, most likely the issue should be re-reported at https://github.com/pypa/setuptools |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-11 14:56:50 | admin | set | github: 50626 |
2021-02-03 18:26:27 | steve.dower | set | status: open -> closed nosy: + steve.dower messages: + msg386387 resolution: accepted -> out of date stage: test needed -> resolved |
2015-04-03 00:23:17 | Jean-Paul Calderone | set | nosy:
- exarkun |
2014-07-09 21:49:51 | BreamoreBoy | set | versions: + Python 3.4, Python 3.5, - Python 3.1 |
2010-12-14 02:41:50 | r.david.murray | set | nosy:
+ eric.araujo type: behavior stage: test needed |
2009-12-16 02:26:01 | cmcqueen1975 | set | nosy:
+ cmcqueen1975 |
2009-11-02 22:23:34 | tarek | set | nosy:
+ dalcinl |
2009-09-07 08:12:04 | tarek | set | priority: high |
2009-07-12 07:32:33 | tarek | set | messages: + msg90439 |
2009-07-10 10:34:52 | lemburg | set | messages: + msg90385 |
2009-07-10 09:34:42 | tarek | set | messages: + msg90383 |
2009-07-10 09:19:47 | tarek | set | messages: + msg90382 |
2009-07-09 17:07:01 | nicdumz | set | nosy:
+ nicdumz messages: + msg90334 |
2009-07-09 09:18:25 | lemburg | set | messages: + msg90316 |
2009-07-09 08:47:55 | lemburg | set | messages: + msg90315 |
2009-07-09 08:12:44 | tarek | set | messages: + msg90313 |
2009-07-09 07:55:14 | lemburg | set | messages: + msg90309 |
2009-07-09 02:16:12 | exarkun | set | messages: + msg90299 |
2009-07-08 22:45:38 | tarek | set | messages: + msg90289 |
2009-07-08 21:46:42 | tarek | set | status: closed -> open messages: + msg90284 |
2009-07-08 19:45:08 | lemburg | set | messages: + msg90281 |
2009-07-08 19:39:57 | rpetrov | set | nosy:
+ rpetrov messages: + msg90280 |
2009-07-08 18:56:03 | lemburg | set | messages: + msg90279 |
2009-07-08 18:51:20 | lemburg | set | messages:
+ msg90278 versions: + Python 2.7 |
2009-07-08 18:50:54 | lemburg | set | messages: + msg90277 |
2009-07-08 18:24:31 | lemburg | set | nosy:
+ lemburg messages: + msg90276 |
2009-07-08 16:53:55 | exarkun | set | nosy:
+ exarkun messages: + msg90274 |
2009-07-06 13:55:44 | tarek | set | status: open -> closed messages: + msg90179 |
2009-07-06 12:51:27 | tarek | set | messages: + msg90176 |
2009-07-05 15:08:48 | tarek | set | messages: + msg90156 |
2009-07-05 14:41:58 | benjamin.peterson | set | messages: + msg90155 |
2009-07-05 14:14:14 | tarek | set | files:
+ compiler.diff nosy: + benjamin.peterson messages: + msg90154 |
2009-07-03 07:57:02 | tarek | set | resolution: accepted messages: + msg90045 |
2009-06-29 18:22:19 | atuining | create |