Issue10027
Created on 2010-10-05 14:35 by brian.curtin, last changed 2022-04-11 14:57 by admin. This issue is now closed.
| Files | ||||
|---|---|---|---|---|
| File name | Uploaded | Description | Edit | |
| st_nlink.diff | brian.curtin, 2010-10-05 14:35 | review | ||
| py3k_posixpath_v1.patch | ocean-city, 2010-10-20 00:59 | replaced | review | |
| py3k_posixpath_traverse_via_DeviceIoControl.patch | ocean-city, 2010-10-22 11:12 | Same patch to r85789 | review | |
| Messages (19) | |||
|---|---|---|---|
| msg118012 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2010-10-05 14:35 | |
In msg117834 on #8879 it was noticed that os.lstat and os.stat don't set st_nlink on Windows, which is causing the patch on that issue to fail test_tarfile. Attached is a stripped down version of the patch Hirokazu Yamamoto proposed on #8879, containing just the os.*stat changes. As stated in that message, the patch is a quick hack to get test_os and test_tarfile working, so it could use review. |
|||
| msg118029 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2010-10-05 19:23 | |
Overall I think this looks like a reasonable restructuring, and it works in a few manual tests of existing hardlinks on my system. Until #8879 goes in, we can't really add tests for this. Hirokazu, do you want to commit this since you came up with it? |
|||
| msg118064 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-10-06 11:47 | |
There are some questions.
1. About my patch, I noticed it removed following code.
Isn't this needed? I like clean code, but I don't want to
break anything.
/* Get WIN32_FIND_DATA structure for the path to determine if
it is a symlink */
if(info.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) {
find_data_handle = FindFirstFileA(path, &find_data);
if(find_data_handle != INVALID_HANDLE_VALUE) {
if(find_data.dwReserved0 == IO_REPARSE_TAG_SYMLINK) {
/* first clear the S_IFMT bits */
result->st_mode ^= (result->st_mode & 0170000);
/* now set the bits that make this a symlink */
result->st_mode |= 0120000;
}
FindClose(find_data_handle);
}
}
/* Set S_IFEXEC if it is an .exe, .bat, ... */
dot = strrchr(path, '.');
if (dot) {
if (stricmp(dot, ".bat") == 0 || stricmp(dot, ".cmd") == 0 ||
stricmp(dot, ".exe") == 0 || stricmp(dot, ".com") == 0)
result->st_mode |= 0111;
}
2. About current behavior. when os.stat() is used for junction
(reparse point, but not simlink), returned information is
about junction on XP or earlier, but about target folder on
Vista or above. Is this intended bahavior?
Thank you.
|
|||
| msg118066 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-10-06 11:53 | |
P.S. Thank you for acceptance. I really wanted to commit that code! |
|||
| msg118720 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2010-10-14 21:48 | |
Jason, any idea on #2? |
|||
| msg118724 - (view) | Author: Jason R. Coombs (jaraco) * ![]() |
Date: 2010-10-14 22:15 | |
I'm unsure. I think when I addressed the issue, I was only concerned with symlinks. The Vista behavior sounds correct to me, so it may be that XP was left with the legacy behavior as it doesn't have native symlink support. It sounds as if the behavior on XP should be modified to follow the same technique as in Vista. I don't know when I'll have a moment to look at it in depth, but I'll try to in the next week or two. |
|||
| msg119108 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-10-19 02:21 | |
How about this patch? * st_nlink support for stat()/lstat(). * lstat() for junction read attributes of junction not target. * stat() for symlink of system locked file (ie: c:/pagefile.sys) should read attributes from target locked file not of symlink even via FindFirstFile. (I cannot test this, sorry) I hope other behavior was not changed by this patch. |
|||
| msg119110 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-10-19 02:24 | |
... I noticed I can test this via buildbot... Probably I'll try this. |
|||
| msg119111 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2010-10-19 02:39 | |
I'll also give it a run on my Windows machines but won't be able to until tomorrow morning (~1300 UTC). |
|||
| msg119130 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-10-19 10:33 | |
I noticed stat() won't set S_IEXEC in stat() with my patch (py3k_posixpath_v1.patch). :-( |
|||
| msg119182 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-10-20 01:03 | |
I replaced my patch (almost reverted stat() part) I think st_nlink will be set via stat()/lstat(), at least. |
|||
| msg119380 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-10-22 11:12 | |
I created test branch "branches/py3k-stat-on-windows" and committed the new patch in r85789. This achieves msg119108. I tested this on Windows7 buildbot where symlink support exists, it seems working correct. http://www.python.org/dev/buildbot/all/builders/x86%20Windows7%203.x/builds/1840 |
|||
| msg120521 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2010-11-05 18:26 | |
Works for me. I think it should be ok to commit. |
|||
| msg120675 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-11-07 12:38 | |
I found Win32 FileID API. With this library, it seems we can use GetFileInformationByHandleEx before Vista. We can get real file name and hard link count from handle. http://www.microsoft.com/downloads/en/details.aspx?FamilyID=1DECC547-AB00-4963-A360-E4130EC079B8&displaylang=en I don't have a patch, but maybe it is worth to implement it. |
|||
| msg121667 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2010-11-20 15:50 | |
I'm not sure how that would work in terms of redistributing, and how we'd handle it within our own build process. This close to the beta I'm -1 on adding that API. |
|||
| msg121977 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-11-21 20:21 | |
Thank you for reply. Could you commit my last patch instead of me? I cannot commit files for a while. |
|||
| msg122275 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2010-11-24 13:32 | |
Committed to py3k in r86727. I think this should be backported to the maintenance branches, but not until after the upcoming point releases. Although those branches won't have the ability to create hard links, they should have the ability to view information about existing links on the system (or if they create them via ctypes/pywin32). Since those branches won't be able to explicitly test this, I think it makes sense to let this bake for a while on py3k. |
|||
| msg122324 - (view) | Author: Hirokazu Yamamoto (ocean-city) * ![]() |
Date: 2010-11-25 02:01 | |
Thank you for commit! |
|||
| msg137624 - (view) | Author: Brian Curtin (brian.curtin) * ![]() |
Date: 2011-06-04 03:00 | |
This code has changed a lot since originally being committed, so I'll handle backporting in #12084 which has the latest changes. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2022-04-11 14:57:07 | admin | set | github: 54236 |
| 2011-06-04 03:00:56 | brian.curtin | set | status: open -> closed messages: + msg137624 |
| 2010-11-25 02:01:48 | ocean-city | set | messages: + msg122324 |
| 2010-11-24 20:26:23 | brian.curtin | unlink | issue8879 dependencies |
| 2010-11-24 13:32:28 | brian.curtin | set | assignee: ocean-city -> brian.curtin resolution: accepted -> fixed messages: + msg122275 stage: patch review -> resolved |
| 2010-11-21 20:21:07 | ocean-city | set | messages: + msg121977 |
| 2010-11-20 15:50:46 | brian.curtin | set | messages: + msg121667 |
| 2010-11-07 12:38:54 | ocean-city | set | messages: + msg120675 |
| 2010-11-05 18:26:48 | brian.curtin | set | assignee: ocean-city messages: + msg120521 |
| 2010-10-22 11:12:36 | ocean-city | set | files:
+ py3k_posixpath_traverse_via_DeviceIoControl.patch messages: + msg119380 |
| 2010-10-20 01:03:10 | ocean-city | set | messages: + msg119182 |
| 2010-10-20 00:59:47 | ocean-city | set | files: - py3k_posixpath_v1.patch |
| 2010-10-20 00:59:31 | ocean-city | set | files: + py3k_posixpath_v1.patch |
| 2010-10-19 10:33:11 | ocean-city | set | messages: + msg119130 |
| 2010-10-19 02:39:30 | brian.curtin | set | messages: + msg119111 |
| 2010-10-19 02:24:01 | ocean-city | set | messages: + msg119110 |
| 2010-10-19 02:21:20 | ocean-city | set | files:
+ py3k_posixpath_v1.patch messages: + msg119108 |
| 2010-10-14 22:15:35 | jaraco | set | messages: + msg118724 |
| 2010-10-14 21:48:48 | brian.curtin | set | nosy:
+ jaraco messages: + msg118720 |
| 2010-10-06 11:53:09 | ocean-city | set | messages: + msg118066 |
| 2010-10-06 11:47:50 | ocean-city | set | messages: + msg118064 |
| 2010-10-05 19:23:36 | brian.curtin | set | resolution: accepted messages: + msg118029 |
| 2010-10-05 14:39:44 | brian.curtin | link | issue8879 dependencies |
| 2010-10-05 14:35:25 | brian.curtin | create | |
