Stylistic comments
Hi all (especially Robin and Thomas!) -- I was just looking through the new registry-grovelling code in msvccompiler.py, making some stylistic changes, when I noticed the following in '_find_exe()' (comments added by me): try: for p in string.split(os.environ['Path'],';'): fn=os.path.join(os.path.abspath(p),exe) if os.path.isfile(fn): return fn # XXX BAD BAD BAD BAD BAD BAD BAD BAD BAD BAD BAD BAD !!!!!!!!!!!!!!!! except: # XXX WHAT'S BEING CAUGHT HERE?!?!? pass What is this 'except' clause supposed to catch? Other changes: I have renamed '_devstudio_versions()' to 'get_devstudio_versions()' because it's more descriptive and I don't see a big need for the leading underscore; likewise '_msvc_get_paths()' is now 'get_msvc_paths()'. I've changed the 'get_devstudio_versions()' docstring to: """Get list of devstudio versions from the Windows registry. Return a list of strings (???) containing version numbers; the list will be empty if we were unable to access the registry (eg. couldn't import a registry-access module) or the appropriate registry keys weren't found. (XXX is this correct???)""" Input, please? I'd like a similarly explicit and useful docstring for 'get_msvc_paths()', but don't understand it well enough to write one: could someone supply me one please? Oh yeah, I'm mystified by the purpose for '_find_SET()' and '_do_SET()'. Could I get docstrings for those please? Better parameter names (full words, not single characters!) might help. Also, the real reason I went off on this tangent was because we get an even *worse* traceback than before now on machines without MSVC++ (eg. my Linux box being told to use an MSVCCompiler object, which used to work just fine in dry-run mode). In particular, it blows up because 'get_devstudio_versions()' returns None, but '_find_exe()' expects a sequence. My inclination is to fix 'get_devstudio_versions()' to always return a list, empty if the imports failed. Ditto for 'get_msvc_paths()' for consistency. Anyone see a problem with this? (I.e., are we ever going to need to distinguish "couldn't find anything in the registry" from "couldn't access the registry at all"?) Thanks -- Greg -- Greg Ward - Linux geek gward@python.net http://starship.python.net/~gward/ "He's dead, Jim. You get his tricorder and I'll grab his wallet."
In article <20000209215010.A611@beelzebub>, Greg Ward
Hi all (especially Robin and Thomas!) --
I was just looking through the new registry-grovelling code in msvccompiler.py, making some stylistic changes, when I noticed the following in '_find_exe()' (comments added by me):
try: for p in string.split(os.environ['Path'],';'): fn=os.path.join(os.path.abspath(p),exe) if os.path.isfile(fn): return fn # XXX BAD BAD BAD BAD BAD BAD BAD BAD BAD BAD BAD BAD !!!!!!!!!!!!!!!! except: # XXX WHAT'S BEING CAUGHT HERE?!?!? pass
What is this 'except' clause supposed to catch?
well os.environ['Path'] may not be there and then it would be a keyError
Other changes: I have renamed '_devstudio_versions()' to 'get_devstudio_versions()' because it's more descriptive and I don't see a big need for the leading underscore; likewise '_msvc_get_paths()' is now 'get_msvc_paths()'.
I've changed the 'get_devstudio_versions()' docstring to:
"""Get list of devstudio versions from the Windows registry. Return a list of strings (???) containing version numbers; the list will be empty if we were unable to access the registry (eg. couldn't import a registry-access module) or the appropriate registry keys weren't found. (XXX is this correct???)"""
Input, please?
Correct on the doc. I put underscore to stop these being implicitly available outside of the module.
I'd like a similarly explicit and useful docstring for 'get_msvc_paths()', but don't understand it well enough to write one: could someone supply me one please?
Oh yeah, I'm mystified by the purpose for '_find_SET()' and '_do_SET()'. Could I get docstrings for those please? Better parameter names (full words, not single characters!) might help.
_find_SET(name,version_number) '''' looks up in the registry and returns a list of values suitable for use in a SET command eg SET name=value. Normally the value will be a ';' separated list similar to a path list. name is the name of an MSVC path and version_number is a version_number of an MSVC installation. '''' _do_SET(name,version_number) '''' sets os.environ[name] to an MSVC path type value obtained from _find_SET. This is equivalent to a SET command prior to exceution of spawned commands. ''''
Also, the real reason I went off on this tangent was because we get an even *worse* traceback than before now on machines without MSVC++ (eg. my Linux box being told to use an MSVCCompiler object, which used to work just fine in dry-run mode). In particular, it blows up because 'get_devstudio_versions()' returns None, but '_find_exe()' expects a sequence. My inclination is to fix 'get_devstudio_versions()' to always return a list, empty if the imports failed. Ditto for
I thought we had that None fixed ages ago. They should return []. What to do if they do is open.
'get_msvc_paths()' for consistency. Anyone see a problem with this? (I.e., are we ever going to need to distinguish "couldn't find anything in the registry" from "couldn't access the registry at all"?)
Thanks --
Greg
-- Robin Becker
From: Greg Ward
Hi all (especially Robin and Thomas!) --
Also, the real reason I went off on this tangent was because we get an even *worse* traceback than before now on machines without MSVC++ (eg. my Linux box being told to use an MSVCCompiler object, which used to work just fine in dry-run mode). In particular, it blows up because 'get_devstudio_versions()' returns None, but '_find_exe()' expects a sequence. My inclination is to fix 'get_devstudio_versions()' to always return a list, empty if the imports failed. Ditto for 'get_msvc_paths()' for consistency. Anyone see a problem with this?
As Robin already said, this had been fixed some time ago, but it somehow made it back in my version. I have to admit that the patch I sent last night was not the latest one Robin and I had agreed on. It was the newest one I had on my harddrive though... Attached is another msvccompiler.py (zipped), and hopefully Robin can agree on this one. I've already included the doc-strings Robin has supplied. BTW: What is the preferred format posting sourcecode to the list? AFAIK, zipped attachments are not so nice in the archive, unpacked source-code has the white-space problem in Outlook Express, which I'm using here... As soon as you extend the distutils framework to use temp-directories for building, I will submit a patch for msvc again. Thomas
On 10 February 2000, Thomas Heller said:
As Robin already said, this had been fixed some time ago, but it somehow made it back in my version. I have to admit that the patch I sent last night was not the latest one Robin and I had agreed on. It was the newest one I had on my harddrive though...
Oops! Oh well, it happens to the best of us.
Attached is another msvccompiler.py (zipped), and hopefully Robin can agree on this one. I've already included the doc-strings Robin has supplied.
Merging in the changes now. (And let me just say that ediff -- a super-duper-ultra-mega-diff with file-merge capabilities for [X]Emacs -- is *way* cool.) One question: you took away the bare 'except' in '_find_exe()', but now don't catch KeyError from the 'os.environ' lookup at all. Bad thing? OK, I've merged in the changes. Actually, it turns out that I accepted all the diffs, so I am just using the version of msvccompiler.py that you sent me -- good. I made one additional change: added a docstring for 'find_exe()' (renamed from '_find_exe()' because I don't buy the need for underscores -- nobody is doing "from msvccompiler import *"): def find_exe (exe, version_number): """Try to find an MSVC executable program 'exe' (from version 'version_number' of MSVC) in several places: first, one of the MSVC program search paths from the registry; next, the directories in the PATH environment variable. If any of those work, return an absolute path that is known to exist. If none of them work, just return the original program name, 'exe'.""" Correct? There, I've checked in those changes -- rev. 1.13 of msvccompiler.py. And finally, I can see no reason for the '_find_SET()' function to exist at all -- both calls to it can be changed to 'get_msvc_paths()'. Anyone have anything to see in defence of '_find_SET()'? ...OK, I've removed '_find_SET()' and made some other stylistic surgery, now checked in as rev. 1.14. This is *completely* untested, as usual -- please try it out and make sure I didn't screw anything up. ;-)
BTW: What is the preferred format posting sourcecode to the list? AFAIK, zipped attachments are not so nice in the archive, unpacked source-code has the white-space problem in Outlook Express, which I'm using here...
For cases like this -- well-understood, thoroughly-discussed changes -- go ahead and mail me the code directly (gward@python.net). The easiest way for me to deal with it is if the code is directly attached, not in a zip file or what-have-you. If you're using a mail client that is incapable of dealing with "text/plain" (and this wouldn't surprise me coming from Redmond), I don't know what to tell you. Attaching a zip file works, but it's a bit cumbersome on both ends.
As soon as you extend the distutils framework to use temp-directories for building, I will submit a patch for msvc again.
That'll be a little while -- it's lower priority than fixing the "dist" and "install" commands, and definitely lower priority than writing the "bdist" command. As long as building on Windows is *working*, I'm not going to get *too* uptight about compiler turds being left in the wrong place (although it is something I want to fix). Happy hacking all -- pray for snow (at least in West Virginia!) -- Greg -- Greg Ward - just another /P(erl|ython)/ hacker gward@python.net http://starship.python.net/~gward/ Eschew obfuscation!
participants (3)
-
Greg Ward -
Robin Becker -
Thomas Heller