[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: Thanks for having a look. Checked in with the suggested changes to r87539. -- resolution: - fixed stage: commit review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: Checked in a small doc update in r87547. Removes the part about os.symlink not being available, and mentions the OSError. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Amaury Forgeot d'Arc amaur...@gmail.com added the comment: Tests now fail on windows XP: http://www.python.org/dev/buildbot/all/builders/x86%20XP-4%203.x/builds/3874/steps/test/logs/stdio os.symlink() may raise NotImplementedError, and test.support.can_symlink() should catch it. -- stage: committed/rejected - commit review status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: Oops, sorry. Fixed in r87561. -- stage: commit review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: Here's a patch. I think this works more like what you guys are looking for. Tests pass on Windows 7 and I checked it on a Mac to be sure, and it's good there too. -- Added file: http://bugs.python.org/file20178/issue9333_v3.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: (hit enter too soon, sorry) The patch makes os.symlink always available on Windows machines, but it will only have an effect when privileged. Windows XP and Windows 2003 will still receive NotImplementedError, as the underlying calls aren't available there. On Vista and 7 for non-privileged users, OSError will be raised. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Amaury Forgeot d'Arc amaur...@gmail.com added the comment: The patch looks good, I only have stylistic remarks: - We normally don't use windows-specific types in CPython code. Please use int instead of BOOL. And C variables are usually lowercase, even module globals. I suggest something like static int win32_can_symlink; - the enable_symlink() function should be static as well. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: I'll come up with a patch to make the attribute always available, but raise OSError when the privilege is not held. -- resolution: fixed - ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Jason R. Coombs jar...@jaraco.com added the comment: Thanks and good work, Brian. I think ,though, I'm leaning toward agreeing with Amaury on the presence of the symlink attribute in os. I can easily see the justification for hiding it in legacy environments (Windows XP Server 2003), where the relevance diminishes over time, but since we're talking about Python 3, with limited adoption, I'm inclined to suggest it's better to err on the side of breaking existing code and getting the code right, rather than backward compatibility. To me, the test `hasattr(os, 'symlink')` does not effectively communicate the nuances of the underlying functionality. It doesn't provide a run-time environment any data on why it may or may not be present. While it does provide excellent backward compatibility (based on a survey of use-cases), I worry it's not the best solution, and might be undesirable in the long run. Would it be possible to provide a `can_symlink` or `user_can_symlink` function which would be recommended to replace the `hasattr` test? Perhaps we consider keeping the current implementation, deprecate the use of the hasattr test, and prepare for a change in 3.3 or 3.4 where the symlink method is always present on Windows systems = 6.0. I defer to Brian's opinion on this, but did want to share my mild discomfort with the current implementation. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: Fixed in r86935. Tests pass on the following setups: - Windows 7 (regular user - no symlink privilege) - Windows 7 (administrator + symlink privilege) - Windows Server 2003 (no symlink abilities) - Arch Linux (just a sanity check) I'm going to create a follow-up issue to explain how this works for the Windows part of the FAQ. There is documentation, but beginners could probably use some tips and explanation. -- resolution: - fixed stage: patch review - committed/rejected status: open - closed ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Amaury Forgeot d'Arc amaur...@gmail.com added the comment: So the presence of os.symlink depends on some dynamic privilege? It seems to me that it's the first time in Python. For example, os.chroot() is always available on Unix, even when the user is not root. Of course the call will fail at runtime. Why not simply raise an exception when the user has not enough privileges? (I mean OSError or WindowsError of course, not AttributeError) -- nosy: +amaury.forgeotdarc ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: So the presence of os.symlink depends on some dynamic privilege? Yes. Why not simply raise an exception when the user has not enough privileges? (I mean OSError or WindowsError of course, not AttributeError) My thinking was that anyone writing cross-platform code which uses symlink in any way is already doing hasattr(os, symlink), and if they get a symlink attribute, it should work. With an exception they would have to add an additional try/except for the common case that os.symlink would fail due to lack of privilege on Windows. I suspect that most people are not running with the required privilege, as evidenced by a look around the web at how others have written and tested this area of code in their applications. Even if someone has an account with administrator-level access, the command prompt starts up with regular privileges, so even those users (e.g., myself) would experience os.symlink raising an exception. Until the application is started explicitly with administrator privileges by an account blessed with access to the symlink privilege does the os.symlink even provide value. This was noticed in virtualenv3 right off the bat when the first os.symlink checkin happened (see msg112322). They do the hasattr check and go ahead expecting it to work, and it would not work in their case no matter what checks they would do. I've seen other applications setup to do the same thing. In the end, I'd rather not make people do this: if hasattr(os, symlink): if not os.path.exists(dest): try: os.symlink(src, dest) except OSError: print(no privilege) but instead allow them to do what they are likely already be doing: if hasattr(os, symlink): if not os.path.exists(dest): os.symlink(src, dest) For new uses of os.symlink on Windows versions which support it, it may appear a bit unorthodox. I accept that, and I wish it could just work, but we're given a complex set of hoops to jump through in order to make this usable in any case. I made my decision based on the small percentage of times where this functionality is even available, coupled with existing usage of the function. -- stage: committed/rejected - commit review status: closed - open ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: I've tried changing privileges for a user and I'm not seeing that they get reflected in real-time while an application is running. Maybe I'm not doing it right, but I'm not seeing it. I'm also not able to find anything about that being supported or anyone else trying this...actually, there's very little privilege related info anywhere outside of these are the available privileges. I'll spend a little more time and see if I can channel any sysadmin types to see if they know anything about this. It's looking like we'll just have to try enabling on module loading and if it works, great, otherwise there's no os.symlink for the lifetime of the interpreter. Attached is a patch that works safely on XP/Server 2003 where there is no symlink possibility. It still has that test_tarfile error which I'll get to next. -- components: +Windows keywords: +needs review Added file: http://bugs.python.org/file18503/issue9333_v2.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Changes by Brian Curtin cur...@acm.org: Removed file: http://bugs.python.org/file18132/enable_symlink.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: Here's a patch implementing a similar idea to what Jason mentioned, complete with the test updates and code removals. It initially adds win_symlink as _symlink, and on module initialization it will be renamed to symlink if the privilege is available. There are two test failures in test_tarfile that I'll have to work out. Other than that, how does this look? -- stage: - patch review Added file: http://bugs.python.org/file18330/issue9333.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: Forgot to mention: I've only run this on Win7 at the moment. I'll need to take a look at how this works on older Windows. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Jason R. Coombs jar...@jaraco.com added the comment: I love it -- especially all of the '-' lines in the patch! Good work. Is it conceivable that a Windows XP user would have that privilege (and thus would have access to a non-functional os.symlink function)? One thing I particularly like is this provides an interface (os._symlink) that developers can use if they expect privileges to change at runtime, but for the majority of use cases, the os.symlink interface is otherwise largely consistent. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: I'll have to investigate the possibility of the privilege occurring on XP -- I'm doubtful that it exists there, but I'll confirm. Currently os._symlink is not exposed -- it gets swallowed up in Lib/os.py in the nt section starting on line 55 (it is available as nt._symlink, though). This is another point I need to confirm, but I don't think a process' available privileges can change during runtime, or at least I'm not familiar with that. For that reason, I just do the enable_symlink() on init and what happens there is what stays for the lifetime of the interpreter. If available privileges can in fact change - and I'm not sure how we'd test that - enable_symlink() would have to be exposed. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Jason R. Coombs jar...@jaraco.com added the comment: I'm attaching this patch for discussion: Here's what I had in mind for addressing the NotImplementedError when calling os.symlink on Windows XP. During the posixmodule initialization, if the system does not have the CreateSymbolicLink API call, the function is never added to the module. This will greatly simplify the tests and will limit the impact of this new capability on Windows XP. This function could be extended to do the same for privilege testing. Caution - I have not tested the patch, only written it for discussion purposes. What do you think of this approach to hide the NotImplementedError? If there's no objection, I'll flesh out the complete implementation, update the tests and documentation, and resubmit another patch. How about using this technique to hide the function when the privilege isn't present? The one objection I could see w.r.t. privileges is the privilege could be granted after the module is initialized - meaning that a given process would need to have the privilege granted before importing the os module. -- Added file: http://bugs.python.org/file18310/self-omitting-symlink.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
New submission from Brian Curtin cur...@acm.org: As it currently stands, the possibility exists that some users might not have the SeCreateSymbolicLinkPrivilege privilege enabled (depending on security settings, corporate policy, etc). There should be some method of enabling that privilege outside of the way we do it in the tests (using ctypes). Attached is a quick prototype...no docs or tests yet. If you don't have the privilege enabled, os.enable_symlink() will attempt to enable it for you (True if successful, False if not). For the security conscious: The AdjustTokenPrivileges function cannot add new privileges to the access token. It can only enable or disable the token's existing privileges -- assignee: brian.curtin components: Extension Modules files: enable_symlink.diff keywords: patch messages: 111213 nosy: brian.curtin, eric.smith, jaraco priority: normal severity: normal status: open title: Expose a way to enable os.symlink on Windows type: behavior versions: Python 3.2 Added file: http://bugs.python.org/file18132/enable_symlink.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Jason R. Coombs jar...@jaraco.com added the comment: I think we should consider simply calling this function before running os.symlink. It would be nice if the API were as compatible as possible on both unix and Windows. My worry is that where code that works on unix systems is simply: os.symlink(...) But now to support symlinks on Windows, one must write: if hasattr(os, 'enable_symlink') and not os.enable_symlink(): raise WindowsError(...) os.symlink(...) Maybe instead of adding os.enable_symlink, Python should include the above logic before attempting to create a symlink and raise an exception if it fails? This would provide a consistent API across platforms for the most common use-case. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Brian Curtin cur...@acm.org added the comment: That's a way better idea. It would also cut down some of the code in Lib/test/symlink_support.py. I'll take a whack at that and see how it looks. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue9333] Expose a way to enable os.symlink on Windows
Eric Smith e...@trueblade.com added the comment: Wouldn't you have to set this, then restore it? This would then open a non thread-safe race condition, assuming this is a per-process setting, not a thread-local setting. Not that I'm necessarily opposed, but it's an issue. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue9333 ___ ___ Python-bugs-list mailing list Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
