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.

classification
Title: distutils2 environment marker for current compiler
Type: enhancement Stage: resolved
Components: Distutils2 Versions: Python 3.3
process
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: alexis, eli.collins, eric.araujo, ncoghlan, rpetrov, tarek
Priority: normal Keywords: patch

Created on 2011-06-02 15:28 by eli.collins, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
cpython_issue12242.patch eli.collins, 2011-06-23 19:50 patch implementing this enhancement review
04610583238f.diff eli.collins, 2011-09-27 22:29 review
ba08e4a70631.diff eli.collins, 2011-09-29 04:04 review
ca53ff77ce6f.diff eli.collins, 2011-09-29 20:20 review
963a2cce068d.diff eli.collins, 2011-09-30 17:04 review
4e67e7205aba.diff eli.collins, 2011-11-20 23:24 review
Repositories containing patches
http://bitbucket.org/ecollins/cpython#issue-12242-dev
Messages (15)
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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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.
History
Date User Action Args
2022-04-11 14:57:18adminsetgithub: 56451
2014-03-13 03:42:26eric.araujosetstatus: open -> closed
nosy: + ncoghlan

resolution: out of date
stage: resolved
2011-11-20 23:26:29eli.collinssetmessages: + msg148014
2011-11-20 23:24:37eli.collinssetfiles: + 4e67e7205aba.diff
2011-09-30 17:18:31eli.collinssetmessages: + msg144695
2011-09-30 17:05:04eli.collinssetfiles: + 963a2cce068d.diff
2011-09-29 20:32:22eli.collinssetmessages: + msg144634
2011-09-29 20:21:50eli.collinssetmessages: + msg144628
2011-09-29 20:20:00eli.collinssetfiles: + ca53ff77ce6f.diff
2011-09-29 20:13:59rpetrovsetnosy: + rpetrov
messages: + msg144627
2011-09-29 04:05:58eli.collinssetmessages: + msg144585
2011-09-29 04:04:23eli.collinssetfiles: + ba08e4a70631.diff
2011-09-27 22:31:35eli.collinssetmessages: + msg144558
2011-09-27 22:29:31eli.collinssetfiles: + 04610583238f.diff
2011-09-27 22:28:23eli.collinssethgrepos: + hgrepo71
2011-06-24 11:20:34eric.araujosetassignee: tarek -> eric.araujo
messages: + msg138912
2011-06-23 19:50:21eli.collinssetfiles: + cpython_issue12242.patch
keywords: + patch
messages: + msg138872
2011-06-09 12:03:26eric.araujosetmessages: + msg137954
2011-06-07 18:36:10eli.collinssetmessages: + msg137876
2011-06-07 14:41:52eric.araujosetmessages: + msg137829
2011-06-06 18:47:56eli.collinssetmessages: + msg137764
2011-06-06 16:16:03eric.araujosetmessages: + msg137749
versions: + Python 3.3
2011-06-02 15:28:43eli.collinscreate