| msg51978 - (view) |
Author: Jon Foster (jongfoster) * |
Date: 2007-02-26 23:07 |
| Hi, I consider this to be a bug in os.path.isabs(): PythonWin 2.5 (r25:51908, Sep 19 2006, 09:52:17) [MSC v.1310 32 bit (Intel)] on win32. >>> import os.path >>> s = '\\Python25' >>> os.path.isabs(s) True >>> os.path.abspath(s) 'C:\\Python25' >>> os.chdir('d:') >>> os.path.abspath(s) 'D:\\Python25' >>> If s is really an absolute path as isabs() claims, then why does abspath() return a different path (i.e. not s)? And worse, note that a call to os.chdir() changes the meaning of s! So s is clearly not an absolute path, and isabs() is wrong. It turns out that on Windows there are really 4 different kinds of paths: 1) Completely relative, e.g. foo\bar 2) Completely absolute, e.g. c:\foo\bar or \\server\share 3) Halfbreeds with no drive, e.g. \foo\bar 4) Halfbreeds relative to the current working directory on a specific drive, e.g. c:foo\bar Python 2.5's os.path.isabs() method considers both (2) and (3) to be absolute; I agree with the classification of (2) but strongly disagree about case (3). Oh, and os.path.join is also broken, e.g. os.path.join('foo', 'a:bar') gives 'foo\\a:bar', which is an invalid path. Another consequence of this is that os.path.isabs() is not enough to classify paths. Sometimes you really need a relative path, so we really need (at least) a new os.path.isrelative(), which can return "not isabs(s)" on POSIX platforms, and do the right thing on Win32. The attached patch: - Changes the behaviour of os.path.isabs() and os.path.join() on Win32, to classify pathnames as described above (including adding UNC support) - Adds os.path.isrelative() on all platforms - Changes a couple of Win32 os.path tests where I have deliberately broken backward compatibility - Adds lots of new tests for these 3 functions on Win32 This does change the behaviour of isabs(), and it is possible that existing applications might be depending on the current behaviour. Silently changing the behaviour might break those applications. I'm not sure what the best course of action is - applying this for 2.6, putting a new API in (os.path.isreallyabs()?), or waiting for Python 3000...? Kind regards, Jon Foster |
|
|
| msg51979 - (view) |
Author: Jon Foster (jongfoster) * |
Date: 2007-02-26 23:15 |
| P.S. The patch is against Python SVN trunk |
|
|
| msg51980 - (view) |
Author: Jason Orendorff (jorend) |
Date: 2007-03-07 04:01 |
| Thanks for submitting this. Nice summary of the different cases. I think very few people have given this that much thought. :) You need a patch for the docs if you want this to even get considered. Then the main obstacle is convincing the team that the nice new logical behavior is worth the breakage. I would be surprised if it got through. And Python 3000 is not going to be the "subtle breakage" release. I would give you better odds if your patch created new APIs instead, and *documented* exactly what's useful about the model you're suggesting. |
|
|
| msg51981 - (view) |
Author: Jason Orendorff (jorend) |
Date: 2007-03-07 10:32 |
| There is an interesting thread on python-dev right now about possibly tweaking the behavior of splitext() in 2.6. So maybe there is hope after all. |
|
|
| msg51982 - (view) |
Author: Jon Foster (jongfoster) * |
Date: 2007-03-07 23:09 |
| Thanks for the comments. Updated patch attached, which includes the doc changes. File Added: win32_absolute_paths_2.patch |
|
|
| msg51983 - (view) |
Author: Mark Hammond (mhammond) *  |
Date: 2007-03-08 00:36 |
| I agree the current behaviour is not ideal - but the right way forward isn't clear. Note the existing (strange) behaviour of os.path.join: >>> os.path.join("c:\\", "\\a") 'c:\\a' >>> os.path.join("c:\\foo", "\\a") '\\a' if isabs('\\a') returns False, there is almost an expectation that the last example should return 'c:\\foo\\a' - but that would almost never give what was expected. 'c:\\a' seems the correct result, but my quick scan of the test code in the patch shows this isn't exercised. FWIW, about the only common use of isabs() in my code is something like: log_directory = get_option_from_config_file(...) if not os.path.isabs(log_directory): # make log_directory absolute based on app directory. log_directory = os.path.join(app_directory, log_directory) Best I can tell, the above would not break if isabs('/foo') started returning False, so long as os.path.join('c:\\foo', '\\bar') resulted in 'c:\\bar'. Off the top-of-my-head, another strategy may be to introduce isrelative() in 2.6, and in 2.7 introduce a warning when isabs() is passed one of the halfbreeds, warning that 2.8 will return a different value, and pointing to the new isrelative(). 2.8 then gets the full, 'correct' implementation as described here. |
|
|
| msg51984 - (view) |
Author: Jon Foster (jongfoster) * |
Date: 2007-03-08 20:53 |
| Hi Mark, Thanks for reviewing this. You wrote: > os.path.join("c:\\foo", "\\a") ... > 'c:\\a' seems the correct result Yes indeed, and this patch makes it do that. Hiding in the patch is this changed test case: -tester("ntpath.join('c:/a', '/b')", '/b') +tester("ntpath.join('c:/a', '/b')", 'c:/b') # CHANGED: Was '/b') and these new ones: +tester('ntpath.join("c:\\a", "\\b")', "c:\\b") +tester('ntpath.join("c:/a", "/b")', "c:/b") (Hmm looks like one of those tests is a duplicate... not a big deal I guess). I guess another way to handle these changes is to introduce new isabsolute(), isrelative(), and joinpath() functions, and deprecate isabs() and join(). (By "deprecate" I mean at least put a warning in the docs that it shouldn't be used; whether or not to make the code raise a DeprecationWarning is a separate decision). But that's kinda ugly from an API design point of view. I'm happy to roll a patch that does that, if we decide that fixing isabs() isn't possible. It's a pity there isn't a way to do something like "from __future__ import fixed_win_paths". But given how dynamic Python is, I guess that would be hard. Kind regards, Jon |
|
|
| msg58743 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2007-12-18 12:04 |
| The problem should be addressed and fixed before the next alpha release. |
|
|
| msg110297 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-07-14 16:23 |
| Nosy list changed as both have an interest in Windows issues. |
|
|
| msg117582 - (view) |
Author: Ulrich Eckhardt (eckhardt) |
Date: 2010-09-29 08:46 |
| I just stumbled across the issue with isabs(). I'd also say that Mark Hammond already provided the typical use case for this, i.e. that you allow relative paths for convenience when storing them or putting them on the commandline, but for actual use you first convert them so that their meaning doesn't change under your feet, e.g. because they depend on volatile things like the current working directory or drive. If you assume that is the intention or typical use case, then calling isabs() to determine if the path is stable doesn't work. Of course, the wording of the documentation then needs to change, too, as it explicitly says "after chopping off a potential drive letter". Concerning the behaviour of path.join() and support for "\\server\share" paths, I'm not sure, but I think that these are issues that can be discussed/changed separately. |
|
|
| msg138215 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2011-06-12 19:41 |
| A bug for tracker purposes is a discrepancy between doc and code. That can be fixed in current versions. A design change is a feature request and can only go in future versions. A deprecation warning for one cycle is desirable when appropriate. "os.path.isabs(path) Return True if path is an absolute pathname. On Unix, that means it begins with a slash, on Windows that it begins with a (back)slash after chopping off a potential drive letter." From what you report, the code does just that, so there is no bug. Breaking code just because you do not like the current definition is very unlikely to be accepted. Adding a .isrelative(path) that seems to be equivalent to 'not .ispath' is unlikely to accepted. People who care about the presence of a drive letter can write "path[1]==':'" "os.path.join(path1[, path2[, ...]]) Join one or more path components intelligently. If any component is an absolute path, all previous components (on Windows, including the previous drive letter, if there was one) are thrown away, and joining continues." >>> import os >>> os.path.join('foo', 'a:bar') 'foo\\a:bar' Producing a non-path does not seem intelligent, so I agree that this a bug. I am not sure what this should produce. 'foo/bar', 'a:foo/bar', and raising an exception would be candidates. All would be better than the current useless return. In any case, a bug fix patch can only fix bugs and should not be intermixed with feature patches. As to the second sentence: your proposal to change documented and inplemented semantics is, again, a feature request, not a bug fix. If you want to continue with your feature requests, you can if you wish reclassify this issue, retitle again, and open a new behavior issue for the path bug and upload a bugfix-only patch. Before or along with doing so, I would suggest discussion on the python-ideas list. |
|
|
| msg166066 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2012-07-21 20:45 |
| I've opened a issue 15414 for fixing the documentation for os.path.join on Windows, and possibly existing bugs. So I'm reclassifying this as a feature request, since that is most of what the discussion is about. |
|
|
| msg218560 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-05-14 18:21 |
| ntpath.join() was fixed in . |
|
|
| msg222855 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2014-07-12 16:53 |
| I've asked for a commit review on issue 15414 so can we close this? |
|
|
| msg229053 - (view) |
Author: Zachary Ware (zach.ware) * |
Date: 2014-10-11 00:47 |
| To kick this along a bit, do the following testcases seem like the right behavior to others, based on Mark Hammond's roadmap in ? If there's some agreement, I'll work on getting a modernized patch put together. # currently (3.4) assertTrue(ntpath.isabs(r"\driveless\halfbreed")) assertFalse(ntpath.isabs(r"D:rived\halfbreed")) assertTrue(ntpath.isabs(r"\\any\UNC\path")) assertTrue(ntpath.isabs(r"O:\bviously\absolute")) assertFalse(ntpath.isabs(r"obviously\relative")) # 3.5 # Halfbreeds are not relative, keep same isabs behavior but warn with assertWarnsRegex(FutureWarning, "will return False in 3.6"): assertTrue(ntpath.isabs(r"\driveless\halfbreed")) # same behavior assertFalse(ntpath.isabs(r"D:rived\halfbreed")) assertTrue(ntpath.isabs(r"\\any\UNC\path")) assertTrue(ntpath.isabs(r"O:\bviously\absolute")) assertFalse(ntpath.isabs(r"obviously\relative")) # new functions assertFalse(ntpath.isrelative(r"\driveless\halfbreed")) assertFalse(ntpath.isrelative(r"D:rived\halfbreed")) assertFalse(ntpath.isrelative(r"\\any\UNC\path")) assertFalse(ntpath.isrelative(r"O:\bviously\absolute")) assertTrue(ntpath.isrelative(r"obviously\relative")) assertTrue(posixpath.isrelative("foo/bar")) assertTrue(macpath.isrelative(":foo:bar")) assertTrue(macpath.isrelative("foobar")) # 3.6 assertFalse(ntpath.isabs(r"\driveless\halfbreed")) # all else the same I'll note that this is a bit extra complicated by the fact that MS calls r"\driveless\halfbreed" an "absolute path", see http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247%28v=vs.100%29.aspx#fully_qualified_vs._relative_paths That gives me the impression that Windows has notions of "fully qualified" paths (r"C:\foo", "C:\\", r"\\any\UNC\path"), absolute paths (r"\foo", etc.), and relative paths (r"foo\bar", "C:foo", and annoyingly, "C:\foo\..\bar"). My opinion is that we should just declare "we don't quite agree with MS on this one" and go with the semantics outlined above, though we're currently mostly in agreement with them. |
|
|