Patches item #1669539, was opened at 2007-02-26 23:07
Message generated for change (Comment added) made by jongfoster
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1669539&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.6
Status: Open
Resolution: None
Priority: 5
Private: No
Submitted By: Jon Foster (jongfoster)
Assigned to: Nobody/Anonymous (nobody)
Summary: Change (fix!) os.path.isabs() semantics on Win32

Initial Comment:
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


----------------------------------------------------------------------

>Comment By: Jon Foster (jongfoster)
Date: 2007-03-08 20:53

Message:
Logged In: YES 
user_id=199289
Originator: YES

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


----------------------------------------------------------------------

Comment By: Mark Hammond (mhammond)
Date: 2007-03-08 00:36

Message:
Logged In: YES 
user_id=14198
Originator: NO

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.

----------------------------------------------------------------------

Comment By: Jon Foster (jongfoster)
Date: 2007-03-07 23:09

Message:
Logged In: YES 
user_id=199289
Originator: YES

Thanks for the comments.  Updated patch attached, which includes the doc
changes.
File Added: win32_absolute_paths_2.patch

----------------------------------------------------------------------

Comment By: Jason Orendorff (jorend)
Date: 2007-03-07 10:32

Message:
Logged In: YES 
user_id=18139
Originator: NO

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.

----------------------------------------------------------------------

Comment By: Jason Orendorff (jorend)
Date: 2007-03-07 04:01

Message:
Logged In: YES 
user_id=18139
Originator: NO

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.



----------------------------------------------------------------------

Comment By: Jon Foster (jongfoster)
Date: 2007-02-26 23:15

Message:
Logged In: YES 
user_id=199289
Originator: YES

P.S. The patch is against Python SVN trunk

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1669539&group_id=5470
_______________________________________________
Patches mailing list
[email protected]
http://mail.python.org/mailman/listinfo/patches

Reply via email to