[Patches] [ python-Patches-462296 ] Add attributes to os.stat results

noreply@sourceforge.net noreply@sourceforge.net
Fri, 28 Sep 2001 07:23:48 -0700


Patches item #462296, was opened at 2001-09-17 10:57
You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=462296&group_id=5470

Category: Library (Lib)
Group: None
Status: Open
Resolution: None
Priority: 5
Submitted By: Nick Mathewson (nickm)
Assigned to: Nobody/Anonymous (nobody)
Summary: Add attributes to os.stat results

Initial Comment:
See bug #111481, and PEP 0042.  Both suggest that the
return values for os.{stat,lstat,statvfs,fstatvfs}
ought to be struct-like objects rather than simple tuples. 

With this patch, the os module will modify the
aformentioned functions so that their results still
obey the previous tuple protocol, but now have
read-only attributes as well.  In other words,
"os.stat('filename')[0]" is now synonymous with
"os.stat('filename').st_mode.

The patch also modifies test_os.py to test the new
behavior.

In order to prevent old code from breaking, these new
return types extend tuple.  They also use the new
attribute descriptor interface. (Thanks for
PEP-025[23], Guido!)

Backward compatibility:  Code will only break if it
assumes that type(os.stat(...)) == TupleType, or if it
assumes that os.stat(...) has no attributes beyond
those defined in tuple.

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

>Comment By: Guido van Rossum (gvanrossum)
Date: 2001-09-28 07:23

Message:
Logged In: YES 
user_id=6380

Another comment: we should move this to its own file so that
other os.stat() implementations (esp. MacOS, maybe RiscOS)
that aren't in posixmodule.c can also use it, rather than
having to maintain three separate versions of the code.

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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-09-28 07:18

Message:
Logged In: YES 
user_id=6380

One comment on the patch: beautiful use of the new type
stuff, but there's something funky with the constructors
going on. It seems that the built-in __new__ (inherited from
the tuple class) requires exactly one argument -- a sequence
to be tuplified -- but your __init__ requires 13 arguments.
So construction by using posix.stat_result(...) always
fails. It makes more sense to fix the init routine to
require a 13-tuple as argument. I would also recommend
overriding the tp_new slot to require a 13-tuple: right now,
I can cause an easy core dump as follows:

>>> import os
>>> a = os.stat_result.__new__(os.stat_result, ())
>>> a.st_ctime
Segmentation fault (core dumped)
$ 



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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-27 21:20

Message:
Logged In: YES 
user_id=499

I've fixed it with the suggestions you made, and also 
   1) Added docstrings
   2) Fixed a nasty segfault bug that would be triggered by
        os.stat("/foo").__class__((10,)).st_size
      and added tests to keep it from reappearing.

I'm not sure I know how to cover Mac and RISCOS properly:
riscos.stat returns a 13-element tuple, and is hence already
incompatible with posix.stat; whereas mac.{stat|xstat}
return differing types. 

If somebody with experience with these modules could let
give me guidance as to the Right Thing, I'll be happy to
give it a shot... but my shot isn't likely to be half as
good as somebody who knew the modules better.  (For example,
I don't have the facilities to compile macmodule or
riscmodule at all, much less test them.)

I'd also be glad to make any changes that would help
maintainers of those modules.

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-09-24 01:44

Message:
Logged In: YES 
user_id=21627

The patch looks good to me. Are you willing to revise it one
more time to cover all the stat implementations?
A few comments on the implementation:
- Why do you try to have your type participate in GC? they
will never be part of a cycle. If that ever becomes an
issue, you probably need to implement a traversal function
as well.
- I'd avoid declaring PosixStatResult, since the field
declarations are misleading. Instead, you should just add
the right number of additional in the type declaration. 


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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-21 13:07

Message:
Logged In: YES 
user_id=499

And here's an even better all-C version.  (This one doesn't
use a dictionary to store optional attributes.)

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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-21 11:01

Message:
Logged In: YES 
user_id=499

Well, here's a posixmodule-only, all-C version.  If this
seems like a good approach, I'll add some better docstrings,
move it into whichever module you like, and make
riscosmodule.c and macmodule.c use it too.


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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-09-19 21:35

Message:
Logged In: YES 
user_id=6380

Or you could put it in modsupport.c, which is already a
grab-bag of handy stuff.

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-09-19 11:36

Message:
Logged In: YES 
user_id=21627

There aren't actually so many copies of the module, since 
posixmodule implements "posix","nt", and "os2". I found 
alternative implementations in riscosmodule and macmodule.

Still, putting the support type into a shared C file is 
appropriate. I can think of two candidate places: 
tupleobject.c and fileobject.c. It may be actually 
worthwhile attempting to share the stat() implementations 
as well, but that could be an add-on.


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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-19 11:10

Message:
Logged In: YES 
user_id=499

I'm becoming more and more convinced that doing it in C is
the right thing, but I have issue with doing it in the posix
module.  The stat function is provided on (nearly?) all
platforms, and doing it in C will require minor changes to
all of these modules.  We can probably live with this, but I
don't think we should duplicate code between all of the os
modules.  

Is there some other appropriate place to put it in C?

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-09-18 23:52

Message:
Logged In: YES 
user_id=21627

Using posix.stat is common, see

http://groups.yahoo.com/group/python-list/message/4349
http://www.washington.edu/computing/training/125/mkdoc.html
http://groups.google.com/groups?th=7d7d118fed161e0&seekm=5qdjch%24dci%40nntp6.u.washington.edu

for examples. None of these would break with your change,
though, since they don't rely on the lenght of the tuple.

If you are going to implement the type in C, I'd put it in
the posix module. If you are going to implement it in Python
(and only use it from the Posix module), making it
general-purpose may be desirable. However, a number of
things would need to be considered, so a PEP might be
appropriate. If that is done, I'd propose an interface like

tuple_with_attrs((value-tuple), (tuple-of-field-names),
exposed-length-of-tuple))

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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-18 14:11

Message:
Logged In: YES 
user_id=499

Ah!  Now I see.  I hadn't realized that anybody used the
posix module directly.  (People really do this?)

I'll try to write up a patch in C tonight or tomorrow
morning.  A couple of questions on which I could use advice: 
(1) Where is the proper place to put this kind of
tuple-with-fields hybrid? Modules? Objects?  In a new file
or an existing one?
(2) Should I try to make it general enough for non-stat use?

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-09-18 00:54

Message:
Logged In: YES 
user_id=21627

The problem with your second and third patch is that it
includes an incompatibility for users of posix.stat (and
friends), since it changes the siye of the tuple. If you
want to continue to return a tuple (as the top-level data
structure), you'll break compatibility for applications
using the C module directly. An example of code that would
be broken is

mode, ino, dev, nlink, uid, gid, size, a, c, m =
posix.stat(filename)

To pass the additional fields, you already need your class
_StatResult available in C.
You may find a way to define it in Python and use it in C,
but that has proven to be very fragile in the past.

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

Comment By: Guido van Rossum (gvanrossum)
Date: 2001-09-17 18:54

Message:
Logged In: YES 
user_id=6380

Haven't had time to review the patch yet, but the idea of
providing a structure with fields that doubles as a tuple is
a good one. It's been tried before and can be done in pure
Python as well.

Regarding the field names: I think the field names should
keep their st_ prefix -- IMO this makes the code more
recognizable and hence readable.

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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-17 17:32

Message:
Logged In: YES 
user_id=499

Here's the revised (*example only*) patch that takes the
more portable approach I mention below.

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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-17 16:10

Message:
Logged In: YES 
user_id=499

On further consideration, the approach taken in the second
(*example only*) patch is indeed too fragile.  The C code
should not lengthen the tuple arbitrarily and depend on the
Python code to decode it; instead, it should return a
dictionary of extra fields.  I think that this approach uses
a minimum of C, is easily maintainable, and very extensible.

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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-17 15:53

Message:
Logged In: YES 
user_id=499

Martin: I'm not entirely sure what you mean here; while my
patch for extra fields requires a minor chunk of C (to
access the struct fields), the rest still works in pure
python.  I'm attaching this second version for reference.

I'm not sure it makes much sense to do this with pure C; it
would certainly take a lot more code, with little benefit I
can descern.  But you're more experienced than I; what am I
missing?

I agree that the field naming is suboptimal; I was taking my
lead from the stat and statvfs modules.  If people prefer,
we can name the fields whatever we like.

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

Comment By: Martin v. Löwis (loewis)
Date: 2001-09-17 15:24

Message:
Logged In: YES 
user_id=21627

I second the request for supporting additional fields 
where available. At the same time, it appears 
unimplementable using pure Python.

Consequently, I'd like to see this patch redone in C. The 
implementation strategy could probably remain the same, 
i.e. inherit from tuple for best compatibility; add the 
remaining fields as slots. It may be reasonable to 
implement attribute access using a custom getattr 
function, though. 

I have also my doubts about the naming of the fields. The 
st_ prefix originates from the time where struct fields 
were living in the global namespace (i.e. across different 
structures), so prefixing them for uniqueness was 
essential. I'm not sure whether we should inherit this 
into Python...


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

Comment By: Nick Mathewson (nickm)
Date: 2001-09-17 13:58

Message:
Logged In: YES 
user_id=499

BTW, if this gets in, I have another patch that adds support
for st_blksize, st_blocks, and st_rdev on platforms that
support them.  It don't expose these new fields in the
tuple, as that would break all the old code that tries to
unpack all the fields of the tuple.  Instead, these fields
are only accessible as attributes. 

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

You can respond by visiting: 
http://sourceforge.net/tracker/?func=detail&atid=305470&aid=462296&group_id=5470