Jason R. Coombs added the comment:

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).

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue13772>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to