msg137481 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-06-02 15:28 |
It would be useful to have an environment marker which matched the compiler being used when compiling a C extension.
Sometimes different compilers require different options (different lib names, different flags, etc); distutils1 setup.py files which have to deal with this currently go through some convolutions to determine the current compiler, and then dynamically modify extra_compile_args before handing them off to setup.
Since packaging is moving to a setup.cfg file, it would be useful to be able to use an environment marker along the lines of 'extra_compile_args = -Dmingw_specific_flag ; packaging.c_compiler == "mingw"', allowing for special options for mingw vs msvc, etc.
Since issue 11921 made it sound like environment markers had been expanded to the extensions sections, I'm hoping this isn't too complicated (but don't know the codebase well enough to write a patch myself).
|
msg137749 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-06 16:16 |
What would the value be for non-C Python implementations?
If the need for compiler-specific options is very common, we could consider either improving the compiler system or implement this request; if it’s not common, letting people use hooks would be enough.
|
msg137764 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-06-06 18:47 |
> What would the value be for non-C Python implementations?
I'm not really sure how this idea could have any value for those implementations, at least for the ones that can't make use of C extensions at all (I think PyPy can; so I guess that would leave Jython & IronPython). Then again, I don't see how anything inside "disutils2.compiler" could be relevant to those implementations either, so maybe I don't understand the question.
> If the need for compiler-specific options is very common, we could
> consider either improving the compiler system or implement this
> request; if it’s not common, letting people use hooks would be enough.
I'm not entirely sure how common this is; even within those people who write C extensions, but I'm pretty sure some problems are unavoidable, such as compiler-specific options when linking to external libraries or compiler-specific ways of specifying compatibility options. Case in point: this stackoverflow question ( http://stackoverflow.com/questions/724664/python-distutils-how-to-get-a-compiler-that-is-going-to-be-used ) had a few solutions... and it seems to me that the cleanest one (from 'Jon') made much more sense as an environment marker.
A similar case is this email thread ( http://comments.gmane.org/gmane.comp.python.devel/123768 ), which presents a situation where the person needs to set different options for a specific *version* of a compiler. In that case, the main solution people proposed was "see if setup() fails, and try again with different options". This is the same solution SQLAlchemy & SimpleJson have been using to implement "optional" c extensions under distutils1; and I wasn't sure if hacks like that were even going to be possible using the new hooks system.
Mind you, the person in that email thread needed to distinguish different *versions* of particular compilers, which I didn't propose in this issue - mainly because the compilers already have unique names assigned to them, and would only require making that info available; whereas gathering version strings and comparing them would require much more work & testing to implement properly.
|
msg137829 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-07 14:41 |
>> What would the value be for non-C Python implementations?
> I'm not really sure how this idea could have any value for those
> implementations, at least for the ones that can't make use of C
> extensions at all
The question was about the meaning of a new “compiler” environment marker. Would it be set to the empty string in Jython? Not available? Or do you think that there is no issue, since Jython would not try to compiler C files?
> I'm not entirely sure how common this is; even within those people
> who write C extensions, but I'm pretty sure some problems are
> unavoidable, such as compiler-specific options when linking to
> external libraries or compiler-specific ways of specifying
> compatibility options.
The SO page contains some pretty crude hacks.
Instead of putting more things into the PEP 345 environment markers, why not extend the extra_compile_args field to take a dictionary mapping regexes to match compiler names and versions to arguments?
In code:
Extension(..., extra_compile_args={'gcc': '-fo',
'gcc .* 4.2': '-foo'})
In setup.cfg:
[build_ext] (or maybe extensions section)
extra_args =
gcc = -fo
gcc .* 4.2 = -foo
|
msg137876 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-06-07 18:36 |
> The question was about the meaning of a new “compiler” environment
> marker. Would it be set to the empty string in Jython? Not available?
> Or do you think that there is no issue, since Jython would not try to
> compiler C files?
Ah, my bad. I _had_ misunderstood your question. Those cases would definitely need to have more properly defined behavior. As well, my attempt at my original idea convinced me that it may be difficult/impossible to implement an environment marker whose value may depend on per-command options. All told, I think your dict idea is probably a much better route to take (and more powerful to boot).
I'll try to work up a patch implementing something along the lines you suggested. One issue is that multiple compiler patterns may match, so it might be better to use a list instead of a dict... and since extra_compile_args is already a list, it might be cleaner (and break less existing code) if I implemented a new argument entirely, eg "specific_compile_args" / "specific_link_args"; which took [ (pattern, option_str) ... ], and used the first pattern that matches (if any).
|
msg137954 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-09 12:03 |
> One issue is that multiple compiler patterns may match,
Right, we can’t say “first matches” win if it’s unordered, and we won’t have OrderedDict in all versions supported by distutils2. Make it a list of tuples. First match wins, so people will have to write more specific regexes first.
> and break less existing code
That’s not a concern; extensions in setup.cfg are a new thing, and for the Python side, we already break a lot of code.
|
msg138872 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-06-23 19:50 |
Attached is a patch that implements this enhancement along the lines of what was last discussed. The behavior introduced in the patch is as follows:
* It adds a stub method named get_compiler_version() to CCompiler,
and implementations of that method for the various compilers.
This method returns the name of the compiler and a version number
(eg "gcc 4.5.2"), exact format described in docstring
of stub method.
* It adds two new arguments to the Extension object:
specific_compile_args, and specific_link_args.
These act similarly to extra_compile_args & extra_link_args,
except that (if specified) they should contain a list of
(regexp string, args) tuples.
When build_ext runs, it retrieves the version string from
get_compiler_version() for the current compiler object.
It matches this against all patterns listed in specific_compile_args.
The args corresponding to the first pattern that matches
are appended to extra_compile_args.
If no pattern matches, extra_compile_args is used unchanged.
If the matched args list contains the magic string "{common_args}",
any extra_compile_args will be inserted at that location
instead of appending the matched args, allowing
for position-dependant arguments.
specific_link_args is handled in a parallel manner.
* The Config object has been modified to parse
specific_compile_args and specific_link_args
in any extension sections.
Both expect a multiline list of k=v pairs,
as in the following example:
[extension=foo]
...other settings...
specific_compile_args =
gcc = --optiona --optionb ; some-env-marker = 'value'
gcc = --optiona
.* = --fallbackoption
These are translated into the format required by Extension()
While this patch works, I have a couple of questions / issues:
1. This patch was made against the hg.python.org/cpython repo.
There appear to be a number of distutils2 repos floating about
(hg.python.org/distutils2, and Tarek has some mirrors on bitbucket).
I'm unclear which one I should be writing this patch against.
If another repo is more appropriate, I can rebase the patch
and resubmit.
2. I'm not sure where I should add documentation about this new behavior.
I added some documentation to the Extension class itself,
but that probably isn't the only place it's needed.
3. This feature could probably use some unittests to be added as well.
I can try my hand at that, but wanted to make sure the design
was acceptable first.
4. This patch uses ";" to separate k=v pairs from environment markers.
This is in line with the PEP and the metadata sections.
However, extra_compile_args's _pop_values() function seems
to use "--" as it's environment marker separator.
This doesn't seem right to me, as "--" is frequently found in options
strings, and I'd expect it to cause problems.
If this is a bug in extra_compile_args,
I'm happy to create a separate bug and submit a patch.
However, if there's some reason that "--" should be used,
I can modify this patch to use "--" as well
(though that route seems fraught with parsing problems).
Please let me know what more I can do.
|
msg138912 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-06-24 11:20 |
Thanks!
> This patch was made against the hg.python.org/cpython repo.
This is good. See http://wiki.python.org/moin/Distutils/Contributing
> I'm not sure where I should add documentation about this new behavior.
At least in Doc/library/packaging.compiler.rst and Doc/packaging/setupcfg.rst, maybe in Doc/install too.
> This feature could probably use some unittests to be added as well.
Yes. You can see how we use mocking in test_unixccompiler.py or test_util.py to test real-world program output and make sure our code parses it right.
> This doesn't seem right to me, as "--" is frequently found in options
> strings, and I'd expect it to cause problems. If this is a bug in
> extra_compile_args, I'm happy to create a separate bug and submit a patch.
Please do. I’ll look into the history to see whether there was a comment about that, but this really looks like a bug. The separator should be something that can’t be part of options strings, like “;”.
|
msg144558 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-09-27 22:31 |
Attached is a diff (04610583238f.diff) containing a second revision of my implementation of this feature. It incorporates suggestions made during code review, as well as adds unittests and documentation of the new features.
This patch retains the same basic behavior as the previous patch, with only a few small changes:
* packaging.config.Config now uses ";;" as the environment marker separator for specific_compile_args and specific_link_args (to match the patch I submitted under issue 12424).
* BCPPCompiler.get_compiler_version() has been implemented. This was the only compiler lacking an implementation under my previous patch.
* UnixCCompiler.get_compiler_version() has been enhanced to parse more version string formats, to better handle unknown compilers.
Outside of that, this patch adds unittests for all the new functionality; as well as documents CCompiler.get_compiler_version(), the new keywords added to Extension, how they both affect build_ext.
This should cover all the previously identified issues, let me know if there's anything more I can do to improve the patch.
|
msg144585 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-09-29 04:05 |
Attached is a diff (ba08e4a70631.diff) containing a third revision of my patch. This hopefully addresses all the issues brought up in the code review of the second revision, and changes little else.
|
msg144627 - (view) |
Author: Roumen Petrov (rpetrov) * |
Date: 2011-09-29 20:13 |
What is result is i use GNU compiler by example with name arm-linux-androideabi-gcc ?
|
msg144628 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-09-29 20:21 |
Attached is a fourth revision (ca53ff77ce6f.diff). This covers the typos identified in the last review, but I'm submitting a new patch because I ran a quick proofreading regexp, and identified a few more formatting mistakes.
|
msg144634 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-09-29 20:32 |
> What is result is i use GNU compiler by example with name arm-linux-androideabi-gcc ?
If the executable is named "arm-linux-androideabi-gcc", the code would currently call "arm-linux-androideabi-gcc --version". If that resulted in (for example) "arm-linux-androideabi-gcc (Some/Platform 4.5.2-1) 4.5.2", then the version string returned for regexp matching would be "arm-linux-androideabi-gcc 4.5.2".
While the patch was getting close to finished, if it seems like a useful idea to people, I could add a "--get-compiler-info" command to build_ext which would return the name of the active compiler, and the output of get_compiler_version().
|
msg144695 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-09-30 17:18 |
Attached is a fifth revision (963a2cce068d.diff) of my patch.
I was trying to avoid submitting any more revisions if possible, since this I think this patch is essentially feature-complete. However, this revision adds two things which I felt were needed:
* This patch fixes a serious oversight where UnixCCompiler.get_compiler_version() wouldn't correctly handle compiler names which contains an absolute file path, or suffixes such as ".exe" (which could happen under mingw). This corrects for that, and adds tests for those cases.
* After thinking about it, I realized the "--get-compiler-info" option I mentioned in my last post was in fact sorely needed... otherwise there is no easy way for users to determine what string specific_compile_args is using to match against its regexp patterns; such as in Roumen's question. (I hope I wasn't overstepping any bounds by going ahead and implementing this)
The option can be run via "pysetup3 -q run build_ext --get-compiler-info". Instead of compiling any extensions, it causes build_ext to print out two lines: the first containing the name of the active compiler class, and the second containing the exact string that will be matched by specific_compile_args. I've added references to this in the documentation, as well as unittests to make sure it behaves properly.
|
msg148014 - (view) |
Author: Eli Collins (eli.collins) * |
Date: 2011-11-20 23:26 |
Attached is a sixth revision (4e67e7205aba.diff) of my patch. This revision makes a couple of minor changes requested by Éric Araujo.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:18 | admin | set | github: 56451 |
2014-03-13 03:42:26 | eric.araujo | set | status: open -> closed nosy:
+ ncoghlan
resolution: out of date stage: resolved |
2011-11-20 23:26:29 | eli.collins | set | messages:
+ msg148014 |
2011-11-20 23:24:37 | eli.collins | set | files:
+ 4e67e7205aba.diff |
2011-09-30 17:18:31 | eli.collins | set | messages:
+ msg144695 |
2011-09-30 17:05:04 | eli.collins | set | files:
+ 963a2cce068d.diff |
2011-09-29 20:32:22 | eli.collins | set | messages:
+ msg144634 |
2011-09-29 20:21:50 | eli.collins | set | messages:
+ msg144628 |
2011-09-29 20:20:00 | eli.collins | set | files:
+ ca53ff77ce6f.diff |
2011-09-29 20:13:59 | rpetrov | set | nosy:
+ rpetrov messages:
+ msg144627
|
2011-09-29 04:05:58 | eli.collins | set | messages:
+ msg144585 |
2011-09-29 04:04:23 | eli.collins | set | files:
+ ba08e4a70631.diff |
2011-09-27 22:31:35 | eli.collins | set | messages:
+ msg144558 |
2011-09-27 22:29:31 | eli.collins | set | files:
+ 04610583238f.diff |
2011-09-27 22:28:23 | eli.collins | set | hgrepos:
+ hgrepo71 |
2011-06-24 11:20:34 | eric.araujo | set | assignee: tarek -> eric.araujo messages:
+ msg138912 |
2011-06-23 19:50:21 | eli.collins | set | files:
+ cpython_issue12242.patch keywords:
+ patch messages:
+ msg138872
|
2011-06-09 12:03:26 | eric.araujo | set | messages:
+ msg137954 |
2011-06-07 18:36:10 | eli.collins | set | messages:
+ msg137876 |
2011-06-07 14:41:52 | eric.araujo | set | messages:
+ msg137829 |
2011-06-06 18:47:56 | eli.collins | set | messages:
+ msg137764 |
2011-06-06 16:16:03 | eric.araujo | set | messages:
+ msg137749 versions:
+ Python 3.3 |
2011-06-02 15:28:43 | eli.collins | create | |