This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author jaraco
Recipients amaury.forgeotdarc, brian.curtin, jaraco, pitrou, python-dev, santoso.wijaya, tim.golden
Date 2013-02-26.14:57:17
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <7E79234E600438479EC119BD241B48D636A15C16@CH1PRD0611MB432.namprd06.prod.outlook.com>
In-reply-to <2131842814.65338392.1361887906997.JavaMail.root@zimbra10-e2.priv.proxad.net>
Content
Thanks for the suggestions.

> Antoine Pitrou added the comment:
>
> From an implementation standpoint, I would indeed prefer the path handling
> functions to be written in Python.

Is there an example of how a function like win32_symlink could call into ntpath.dirname()? I believe the reason we did not invoke ntpath was due to a possible race condition where the Python interpreter might not be fully started before os.symlink is invoked. Perhaps there are mitigating factors, but I'm not familiar enough with the Python internals to know what's safe and what is not (or even how to invoke that behavior).

> From a principle standpoint, I'm not sure it's a good idea for os.symlink()
> to be non-atomic (there's a small race condition between reading the target's
> attributes and creating the actual symlink).

I agree there exists a small possibility of a race condition here. I believe this race condition is of little concern to the vast majority of use-cases. For those use cases where a race condition might be a factor, the target_is_directory parameter may be supplied by that code to overcome the condition.

Furthermore, since the Windows APIs don't provide a way to match the symlink creation with its target, it's not possible to avoid this race condition.

> Also, since in general you always know whether you're making a link to a
> directory or a file, I'm not sure auto-detection is really a plus (except that it
> makes things more familiar for Unix developers).

For the vast majority of use cases, target detection will behave much like symlinks do in Unix, which allows os.symlink to be used portably without specifically accounting for Windows behavior. I'm not as concerned about Windows programmers using Python (who might not mind specifying the directory-ness of their targets), but Python programs written by Unix developers running on Windows. In the latter use-case, it's much harder to have those programs adopt an additional parameter to support Windows. I could imagine some library maintainers rejecting a patch that includes target_is_directory just because it's not elegant and seems unnecessary from a Unix programmer's perspective. For that use case and in order to support the general expectation that os.symlink should provide the most uniform behavior it can, the directory detection is highly valuable.

Indeed, both your initial usage of the function and Larry Hastings' initial reaction betray the expectation that os.symlink should create a directory-type symlink when the target is a directory. I don't see how it's not a huge plus to support a developer's expectations when possible.

Personally, I would prefer to not have the parameter at all. Unfortunately, to support cases where one might need to create directory symlinks to non-existent targets, the parameter is necessary. However, I'd like to limit its required usage to only the cases where it's necessary.

> I'm not against restoring detection, but it should be non-buggy and
> correctly unit-tested :)

I'd like to focus on restoring detection, which was vetted and approved in the original implementation. Obviously, I can't guarantee that it doesn't have additional bugs. Provably-correct code is expensive and way outside the scope of this effort. As you could see from your patch, there were tests capturing the desired behavior. That test has been restored. Additionally, this patch includes a new test to capture the case of a non-local link (a behavior that was never tested for Unix symlinks).
History
Date User Action Args
2013-02-26 14:57:19jaracosetrecipients: + jaraco, amaury.forgeotdarc, pitrou, tim.golden, brian.curtin, santoso.wijaya, python-dev
2013-02-26 14:57:19jaracolinkissue13772 messages
2013-02-26 14:57:17jaracocreate