classification
Title: Fail when vsvarsall.bat produces stderr
Type: enhancement Stage: patch review
Components: Distutils Versions: Python 3.4, Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: tarek Nosy List: amaury.forgeotdarc, dstufft, eric.araujo, flub, lemburg, tarek
Priority: normal Keywords: patch

Created on 2010-09-21 15:24 by flub, last changed 2019-04-26 20:37 by BreamoreBoy.

Files
File name Uploaded Description Edit
msvc9.diff flub, 2010-09-21 15:24 review
msvc9_log.diff flub, 2010-09-21 22:18 Log stderr rather then fail review
Messages (10)
msg117067 - (view) Author: Floris Bruynooghe (flub) Date: 2010-09-21 15:24
It would have saved me a lot of time if msvc9compiler would fail if executing the vsvarsall.bat file produced any output.  The attached patch does this and fails when I try to compile from within a cygwin environment.  I've also tested this from the normal windows command prompt and there buiding does succeed with this patch applied.
msg117082 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-09-21 16:49
A non-empty stderr does not mean that a command failed.
For example, the Microsoft compiler "cl.exe" prints the version string to stderr.
msg117112 - (view) Author: Floris Bruynooghe (flub) Date: 2010-09-21 21:58
I'm aware of that but my limited testing showed that in this case that doesn't happen.

However if this is considered too brittle to just plain fail as soon as there's stderr, how about using distutils' log facility to log the stderr at a reasonable level (warning?)?  That way at least you'll be able to see something useful when you get a failure at a strange looking and far less meaningful traceback a few lines lower.
msg117115 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-09-21 22:01
yes, displaying the stderr content is a good idea.
msg117116 - (view) Author: Floris Bruynooghe (flub) Date: 2010-09-21 22:18
msvc9_log.diff does log stderr at warning level when it occurs.
msg119384 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-10-22 14:26
I’d replace “created stderr” with “printed on stderr”, but otherwise msvc9_log.diff looks good.

distutils is feature-frozen, but I think any changes that help debugging are good.  Tarek, do you agree?
msg119385 - (view) Author: Marc-Andre Lemburg (lemburg) * (Python committer) Date: 2010-10-22 14:31
Éric Araujo wrote:
> 
> Éric Araujo <merwok@netwok.org> added the comment:
> 
> I’d replace “created stderr” with “printed on stderr”, but otherwise msvc9_log.diff looks good.
> 
> distutils is feature-frozen, but I think any changes that help debugging are good.  Tarek, do you agree?

I hear you saying that a lot. Could you provide a reference to where
this was decided ?
msg119386 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2010-10-22 14:35
Sure: http://tarekziade.wordpress.com/2010/03/03/the-fate-of-distutils-pycon-summit-packaging-sprint-detailed-report/

This resulted in the revert of pervasive changes in 3.2: #9088
msg221638 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-06-26 19:47
The patch is two extra lines that look fine to me, can somebody do a commit review please?
msg227790 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-09-29 10:11
Ping.
History
Date User Action Args
2019-04-26 20:37:45BreamoreBoysetnosy: - BreamoreBoy
2014-09-29 10:11:42BreamoreBoysetmessages: + msg227790
2014-06-26 19:47:12BreamoreBoysetversions: + Python 3.4, Python 3.5, - 3rd party, Python 3.2
nosy: + dstufft, BreamoreBoy

messages: + msg221638

components: - Distutils2
2010-10-22 14:35:26eric.araujosetmessages: + msg119386
2010-10-22 14:31:43lemburgsetnosy: + lemburg
messages: + msg119385
2010-10-22 14:27:09eric.araujosetnosy: amaury.forgeotdarc, flub, tarek, eric.araujo
stage: needs patch -> patch review
components: + Distutils2
versions: + 3rd party, - Python 3.3
2010-10-22 14:26:26eric.araujosetmessages: + msg119384
2010-09-21 22:18:17flubsetfiles: + msvc9_log.diff

messages: + msg117116
2010-09-21 22:01:43amaury.forgeotdarcsetmessages: + msg117115
stage: needs patch
2010-09-21 21:58:58flubsetmessages: + msg117112
2010-09-21 16:49:31amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg117082
2010-09-21 15:24:35flubcreate