classification
Title: Fix undefined symbol errors on VS8.0 build
Type: resource usage Stage: resolved
Components: Extension Modules, Windows Versions: Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BreamoreBoy, amaury.forgeotdarc, brian.curtin, ezio.melotti, jaraco, loewis, python-dev, rhettinger, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: needs review, patch

Created on 2010-08-01 01:52 by brian.curtin, last changed 2015-03-25 06:36 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
undefined_symbols.diff brian.curtin, 2010-08-01 01:52 review
faster_stat.diff amaury.forgeotdarc, 2010-08-16 23:23 review
9445.patch steve.dower, 2015-03-21 04:52 review
Messages (14)
msg112252 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-08-01 01:52
Raymond informed me that #1578269 introduced breakage to compilation under Visual Studio 2005 due to three undefined symbols. I'm not currently setup to build under 2005, so I just offer this patch which defines the values as they are seen in VS 2008.
msg112386 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2010-08-01 21:11
Applied in r84356.  
Thank you.
msg114061 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-08-16 18:24
Can someone explain why among the 6 calls to Py_GetFinalPathNameByHandle, 5 of them use VOLUME_NAME_DOS and only one uses VOLUME_NAME_NT?
msg114064 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-08-16 18:58
I committed the code you speak of (posix__getfinalpathname in Modules/posixmodule.c), but I don't know if I have a great answer for that question. It looks like VOLUME_NAME_NT (path with volume device path) should just be changed to VOLUME_NAME_DOS (path with drive letter). In fact, I think it would be more "accurate" since NT yields with a larger path, thus we over-allocate up front before we go on and use DOS and then trim the target path down.

Jason (author of that function, added as nosy) might have some other input as to why he used VOLUME_NAME_NT there.
msg114070 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2010-08-16 19:46
In the case where I did use VOLUME_NAME_NT, I think I chose it because it returned a more robust result. That is, it's not clear what the result is if the result is not on a volume that is assigned a drive letter, but all files referenced must have a VOLUME_NAME_NT.

In other usage, I found that use of VOLUME_NAME_NT was unnatural, because it returned for the user a path that would be unfamiliar, rather than the more traditional VOLUME_NAME_DOS.

So, where the result is to be used by the interpreter and isn't exposed to the user, it seems prudent to use VOLUME_NAME_NT, and where the user will see the result, use VOLUME_NAME_DOS.

I admit, this is only a rule of thumb and may be subject to correction, but this was my motivation when making these selections.
msg114084 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-08-16 23:23
The calls to Py_GetFinalPathNameByHandle come in pairs: one to get the length, the other to retrieve the value.  They should at least be consistent.
There are two other issues:

- in all three places, it's possible for the function to return after malloc(), but before the call to free(); and PyMem_Malloc()/PyMem_Free() would be better.

- on my windows XP, os.stat() is slower than before, and is now significantly slower than os.lstat() (26.6 usec -> 32.8 usec); I found that check_GetFinalPathNameByHandle() repeatedly checks for the presence of the function, and always calls GetModuleHandle() and GetProcAddress(). The attached patch fixes this, can you test it on Vista?
msg114452 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-08-20 20:39
Your patch works for me on Win7.

I'll put together a patch for the malloc/free thing in your first bullet point.
msg181070 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2013-02-01 07:42
Is this issue still valid?
msg222669 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2014-07-10 14:30
I believe that this specific issue can be closed but is a follow up needed regarding problems mentioned in msg114084  ?
msg222678 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2014-07-10 17:29
The original issue seems to be fixed, but the other two related issues mentioned by Amaury still need to be addressed.  One has already a patch, the other doesn't.
msg222695 - (view) Author: Martin v. Löwis (loewis) * (Python committer) Date: 2014-07-10 20:04
I believe the patch is obsolete. Python does not support Windows XP anymore, so all supported versions provide GetFinalPathNameByHandle, and all the detection code can go.
msg238755 - (view) Author: Mark Lawrence (BreamoreBoy) * Date: 2015-03-21 03:41
I believe that this can be closed as fixed, would someone like to do the honours please.
msg238763 - (view) Author: Steve Dower (steve.dower) * (Python committer) Date: 2015-03-21 04:52
The patch is obsolete, but Martin is correct - we can drop the detection completely. Any concerns?

(Reading through the issue, there may be some value in a more general GetFinalPathNameByHandle wrapper that can get VOLUME_NAME_DOS if available and VOLUME_NAME_NT as a fallback, but presumably we'd have heard someone ask in the last five years if this was really needed.)
msg239223 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-03-25 06:36
New changeset d094eeeb1496 by Steve Dower in branch 'default':
Closes #9445: Removes detection of GetFinalPathNameByHandle
https://hg.python.org/cpython/rev/d094eeeb1496
History
Date User Action Args
2015-03-25 06:36:59python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg239223

stage: patch review -> resolved
2015-03-21 04:52:18steve.dowersetfiles: + 9445.patch

messages: + msg238763
2015-03-21 03:41:54BreamoreBoysetnosy: + tim.golden, zach.ware, steve.dower
messages: + msg238755
2014-07-10 20:04:12loewissetnosy: + loewis
messages: + msg222695
2014-07-10 17:29:57ezio.melottisettype: compile error -> resource usage
messages: + msg222678
components: + Extension Modules, - Build
versions: + Python 3.5, - Python 3.2
2014-07-10 14:30:21BreamoreBoysetnosy: + BreamoreBoy
messages: + msg222669
2013-02-01 07:42:34ezio.melottisetnosy: + ezio.melotti
messages: + msg181070
2010-08-20 20:39:17brian.curtinsetkeywords: patch, patch, needs review

messages: + msg114452
2010-08-16 23:23:59amaury.forgeotdarcsetkeywords: patch, patch, needs review
files: + faster_stat.diff
messages: + msg114084
2010-08-16 19:46:29jaracosetmessages: + msg114070
2010-08-16 18:58:21brian.curtinsetstatus: pending -> open

nosy: + jaraco
messages: + msg114064

assignee: rhettinger ->
keywords: patch, patch, needs review
2010-08-16 18:24:31amaury.forgeotdarcsetstatus: closed -> pending

nosy: + amaury.forgeotdarc
messages: + msg114061

keywords: patch, patch, needs review
2010-08-01 21:11:54rhettingersetstatus: open -> closed
keywords: patch, patch, needs review
resolution: fixed
messages: + msg112386
2010-08-01 01:52:53brian.curtincreate