classification
Title: os.symlink: FileExistsError shows wrong message
Type: behavior Stage:
Components: Extension Modules, Interpreter Core Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Yonatan Goldschmidt, andrewnester, eryksun, kamilturek, larry, miserlou, sayanchowdhury, serhiy.storchaka, wrohdewald, xiang.zhang
Priority: normal Keywords:

Created on 2017-02-26 08:52 by wrohdewald, last changed 2021-05-01 14:08 by Yonatan Goldschmidt.

Files
File name Uploaded Description Edit
x.py wrohdewald, 2017-02-26 08:52
Messages (12)
msg288591 - (view) Author: Wolfgang Rohdewald (wrohdewald) Date: 2017-02-26 08:52
execute the attached script. It should return

FileExistsError: [Errno 17] File exists: 'a_link' -> 'a'

but it returns

FileExistsError: [Errno 17] File exists: 'a' -> 'a_link'
msg288592 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-26 10:41
I concur the current message is misleading.

OSError makes the string "file1 -> file2". This also affects other methods calling `path_error2()` such as os.link().
msg288906 - (view) Author: Andrew Nester (andrewnester) * Date: 2017-03-03 17:46
I've been investigating this issue and did not come up with some easy solution.

So the problem is:
os_symlink uses `path_error2` to throw exception.
the order of file arguments now is src then dest. For provided example src is `a` and dest is `sym_link`. As a result `src` -> `dest` is generated here https://github.com/python/cpython/blob/master/Objects/exceptions.c#L1059

If we change order of arguments passed to `path_error2`, error message will be generated properly but OSError.filename will be incorrect (a_link instead of a) and following test will fail for `link`/`symlink`

https://github.com/python/cpython/blob/master/Lib/test/test_os.py#L2901

Not sure if it's OK or not, so it definitely needs some input from Python core developers.
msg288936 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-03 21:45
The current error message looks good to me. What is wrong with it?
msg288947 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-03-04 03:21
A symbolic link is typically represented the other way around, from the point of view of the link pointing at the target. However, Python conceptualizes linking as something like a copy or rename operation, with source and destination filenames, and the current error message represents this point of view.
msg288963 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-04 06:03
This is not about how a symbolic link is represented. This is about how the operation of creating a symbolic link is represented. The first filename is the first argument of os.symlink(), the second filename is the second argument. Try to run

    os.symlink('a', 'a_link')
    os.symlink('b', 'a_link')

You should get an error:

    FileExistsError: [Errno 17] File exists: 'b' -> 'a_link'
msg288971 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2017-03-04 07:34
To me the error message is in the model of a source -> destination operation, in which the arrow indicates the operation's information flow (e.g. of the target path or inode number) from the source file to the destination file. I've never viewed it superficially as just an ordering of parameter1 -> parameter2.
msg289064 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-03-06 03:36
I agree with Eryk. The error message is misleading. When I see it, I take it as source -> destination. I think we should make the error message clearer, or document it in the OSError documentation.
msg296270 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-18 12:39
See issue20517 for the discussion about current implementation. Are there any ideas about clearer error messages?

Added Larry as the author of the original idea and implementation.
msg296273 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2017-06-18 14:43
"Special cases aren't special enough to break the rules."  I want the error message to mirror the API, which it currently does.  If we swapped them, the error message would now contradict the API.  So no, I don't support swapping "src" and "dst" in the error message only when the error pertains to os.symlink.

If literally every time the two-filename version of OSError is used inside Python, the two filenames are "src" and "dst", then we could consider making it slightly more explicit, e.g.

    FileExistsError: [Errno 17] File exists: src='a', dst='a_link'

I think I'd want the source code to reflect this (e.g. thinking about "src" and "dst" rather than "filename" and "filename2").

Would OP et al consider this change to the error message an improvement, or is it not interesting?
msg330827 - (view) Author: Rich Jones (miserlou) Date: 2018-11-30 21:01
@Larry - that would be an acceptable solution!

I'm here because I encountered this error independently. I explain why the  arrow is a problem here: https://bugs.python.org/issue35367

The issue is that the '->' notation is already used by the standard operating system utilities in this context, so for Python overload this semantically in this case is the source of all the confusion.

It would avoid the scare that we've all encountered if it just said 'src'/'dst' rather than '->'.

Thanks!
R
msg392598 - (view) Author: Yonatan Goldschmidt (Yonatan Goldschmidt) * Date: 2021-05-01 14:08
Just reached this issue independently (spent a few minutes debugging an error message like "FileExistsError: [Errno 17] File exists: 'a' -> 'b'", where 'a' didn't exist...)

I agree with Rich on this - for me, the source of confusion was that the way Python uses the arrow notation is just swapped from how ls(1) uses it.

Stage is "patch review" but I couldn't find any PR for it; if we're good with Larry's solution then I'm happy to post a PR implementing it.
History
Date User Action Args
2021-05-01 14:08:17Yonatan Goldschmidtsetnosy: + Yonatan Goldschmidt
messages: + msg392598
2021-03-14 10:02:12kamiltureksetnosy: + kamilturek
2021-03-12 21:24:09eryksunsetstage: patch review ->
components: + Extension Modules, Interpreter Core, - Library (Lib)
versions: + Python 3.8, Python 3.9, Python 3.10, - Python 3.5, Python 3.6, Python 3.7
2018-11-30 21:01:43miserlousetnosy: + miserlou
messages: + msg330827
2018-11-30 20:56:11eryksunlinkissue35367 superseder
2017-06-18 14:43:25larrysetmessages: + msg296273
2017-06-18 12:39:41serhiy.storchakasetnosy: + larry
messages: + msg296270
2017-03-06 03:36:55xiang.zhangsetmessages: + msg289064
2017-03-04 07:34:29eryksunsetmessages: + msg288971
2017-03-04 06:03:52serhiy.storchakasetmessages: + msg288963
2017-03-04 03:21:11eryksunsetnosy: + eryksun
messages: + msg288947
2017-03-03 21:45:10serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg288936
2017-03-03 21:27:32terry.reedysettype: behavior
stage: patch review
2017-03-03 17:46:02andrewnestersetnosy: + andrewnester
messages: + msg288906
2017-02-26 13:26:00sayanchowdhurysetnosy: + sayanchowdhury
2017-02-26 10:41:34xiang.zhangsetnosy: + xiang.zhang

messages: + msg288592
versions: + Python 3.6, Python 3.7
2017-02-26 08:52:24wrohdewaldcreate