[issue26876] Extend MSVCCompiler class to respect environment variables

[Steve Dower]

Steve Dower  added the comment:

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

--
resolution:  -> out of date
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Steve Dower]

Steve Dower added the comment:

Taking another look at the patch, I'm not real keen on the C/LDFLAGS section. I 
think if we want to support setting these, we should fully override the default 
settings (otherwise you can't specify certain options that are in the 
defaults), and avoid splitting them (which probably means changing the spawn() 
call to not quote arguments - right now the .split() logic is just wrong).

I'm sorry this is tough to get together - the distutils compilers are really 
fragile, which is why we would really rather migrate people off them onto some 
other build framework. Right now though, that framework doesn't exist. (One 
potential starting point is my msbuildcompiler at 
https://github.com/zooba/pyfindvs/tree/master/pyfindvs/msbuildcompiler will 
generate a .vcxproj file and build it, which allows much more reliable 
behaviour for options and detecting installs.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Rohit Jamuar added the comment:

Steve, I've limited the changes to _msvccompiler for the master-branch's patch. 
Is it alright?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Changes by Rohit Jamuar :


Added file: http://bugs.python.org/file45881/distutils_patch_master.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Changes by Rohit Jamuar :


Removed file: http://bugs.python.org/file45880/distutils_patch_master.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Changes by Rohit Jamuar :


Added file: http://bugs.python.org/file45880/distutils_patch_master.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Changes by Rohit Jamuar :


Removed file: http://bugs.python.org/file42725/distutils_patch_master.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Steve Dower]

Steve Dower added the comment:

I'd rather only take the change to _msvccompiler and not the ones that are 
there for historical interest (a.k.a. backwards compatibility with people who 
never expect internal implementation details to change).

If any tools break because they're using the wrong compiler class, please file 
a bug against them.

I didn't check the 2.7 file, but the one for master is fine.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Zachary Ware]

Zachary Ware added the comment:

Steve, does the latest patch look good to you?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Rohit Jamuar added the comment:

Bump! The changes, that Steve was asking for, have been incorporated. If 
something was missed, please inform. Otherwise, could this be merged?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Steve Dower]

Steve Dower added the comment:

I still want the behavior I described, since there's no value in overriding 
just the executable name but not the rest of the path.

For 2.7 I think this'll help with long term maintainability enough to be the 
Right Thing. For 3.5 I'm not as sure, but it's probably worth keeping the 
implementations consistent as long as we can justify it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Zachary Ware]

Zachary Ware added the comment:

The patch looks fine to me, but frankly I'm a bit scared to commit anything to 
distutils :).  It would be nice to have tests for this.

I'm also not sure if we can backport this to 2.7 and 3.5: this is pretty 
clearly a new feature, but dips its toes into the grey area of "supporting new 
platforms".  Anyone else have any thoughts?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Zachary Ware]

Zachary Ware added the comment:

(The version field is for the whole issue, not per patch.)

--
versions: +Python 3.5, Python 3.6

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Changes by Rohit Jamuar :


--
versions: +Python 2.7 -Python 3.5, Python 3.6
Added file: http://bugs.python.org/file42726/distutils_patch_27.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Changes by Rohit Jamuar :


--
versions: +Python 3.6 -Python 2.7
Added file: http://bugs.python.org/file42725/distutils_patch_master.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Changes by Rohit Jamuar :


Removed file: http://bugs.python.org/file42642/msvc_respect_env_flags.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Steve Dower]

Steve Dower added the comment:

I was thinking more like this:

if DISTUTILS_USE_SDK:
assume PATH is configured
execute os.getenv("CC", "cl.exe") directly
else:
find VS in the registry
execute "cl.exe" etc.

The only change here from the current situation is preferring the CC variable 
in the former case when it is set. When we look up VS in the registry, we know 
what executable we are going to find and overriding them via the environment is 
only going to cause issues/vulnerabilities.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

Rohit Jamuar added the comment:

Just so that I understand it clearly - Inside MSVCCompiler class (in 
_msvccompiler.py / msvccompiler.py / msvc9compiler.py ), current implementation 
of find_exe() finds compiler / linker / ... after parsing PATH. Should the 
changes be so, that if DISTUTILS_USE_SDK is set to the environment, the values 
set to CC / AR / LD, etc. are used verbatim? Or did you mean to say, that just 
as CC, LD and AR are being read from the environment, the same way rc.exe, 
mc.exe and mt.exe should be read as well, in case DISTUTILS_USE_SDK is set to 
the environment?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Steve Dower]

Steve Dower added the comment:

I'm neutral about applying it to 2.7, though I can see the value in doing so.

For 3.5 it definitely has to modify _msvccompiler - the other two modules are 
just left behind to distract people apparently :) (but actually in case someone 
accidentally(?) too a direct dependency on an undocumented class).

One thing I'd like to see is an environment variable to switch it on - probably 
DISTUTILS_USE_SDK is fine for this. In the default case, we know exactly where 
we are looking for the tools, and so we know what they are called. If you're 
intending to override this completely, you probably need to override 
everything, and DISTUTILS_USE_SDK is the flag for this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Zachary Ware]

Zachary Ware added the comment:

I can't seem to get the patch to apply, could you please regenerate it against 
a fresh checkout of https://hg.python.org/cpython#default (or 'master' of 
github.com/python/cpython)?

This looks fairly straightforward, but I'm far from an expert on distutils :).  
I can see issues arising from attempts to use a compiler/linker/etc. that 
doesn't recognize options that we assume are going to cl/link/etc., but I'm not 
sure how big a deal that is: I'd be inclined to say that if you're trying to 
tell distutils where cl is, don't get mad when what you told it doesn't work.

Also, I don't think Lib/distutils/msvccompiler.py is used anymore, but rather 
Lib/distutils/_msvccompiler.py in 3.5+.

I'm generally in favor of this, but would feel much better about it with buy-in 
from Steve.

There has also been mention of creating a new ICCCompiler class, but I'm not 
sure whether that has gotten anywhere.

--
components: +Windows
nosy: +paul.moore, steve.dower, tim.golden
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Zachary Ware]

Changes by Zachary Ware :


--
nosy: +christopher.hogan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue26876] Extend MSVCCompiler class to respect environment variables

[Rohit Jamuar]

New submission from Rohit Jamuar:

The UnixCompiler class respects flags (CC, LD, AR, CFLAGS and LDFLAGS) set to 
the environment, whereas MSVCCompiler class does not. This change allows 
building CPython and any module that invokes distutils to utilize flags and 
executables that have been set to the environment. Inclusion of this change 
would ensure MSVCCompiler's behavior to be same as that of UnixCompiler and 
would also allow using a different set of compiler / linker / archiver, on 
Windows, without having the necessity for implementing separate compiler 
classes - using environment variables it should be possible to use a separate 
set of build executables - for example icl, clang, etc.

--
components: Distutils
files: msvc_respect_env_flags.patch
keywords: patch
messages: 264439
nosy: dstufft, eric.araujo, r.david.murray, rohitjamuar, zach.ware
priority: normal
severity: normal
status: open
title: Extend MSVCCompiler class to respect environment variables
type: enhancement
versions: Python 2.7, Python 3.5
Added file: http://bugs.python.org/file42642/msvc_respect_env_flags.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com