msg154316 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-02-26 09:23 |
Following on from Guido's rejection of PEP 410:
http://mail.python.org/pipermail/python-dev/2012-February/116837.html
This bug is the proposed hammering-out space for how to preserve exact metadata for st_atime / st_mtime between os.stat and os.utime.
(Yes, there's already #11457 -- but that went pretty far down the rabbit hole of Decimal. I thought maybe a fresh start would be best.)
|
msg154319 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-02-26 09:42 |
Guido proposed st_atime_ns et al. I'll make an alternate proposal.
I'd like to avoid tying ourselves to ns resolution. As MvL said: "I don't want to deal with this issue *again* in my lifetime".
If all we want is to facilitate copying the exact metadata from os.stat to os.utime, then we don't really care about conveniently modifying the timestamp. So I propose we use MvL's suggestion of a tuple of ints. Either (numerator, denominator) or (seconds, fractional_numerator, fractional_denominator). (These are mentioned in PEP 410 under the heading "Tuple of ints" as options A and B respectively.) Name the fields with the suffix "_exact" (e.g. st_mtime_exact). os.stat and its ilk produce it; os.utime and its ilk consume it. If people want to monkey with the values and do math, let 'em--consenting adults.
|
msg154334 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-02-26 12:05 |
OTOH, it seems that Guido is very much in favor of hard-coding ns resolution in the API, claiming that expectations of even finer resolution are academic. While I still disagree (the *very* same argument was brought up for ms resolution ten years ago), I think that practicality beats purity here: people apparently can deal with an fixed scale much better than with a floating one. So it may be better to meet the apparent intuition of the majority than follow the purity route.
|
msg154355 - (view) |
Author: R. David Murray (r.david.murray) *  |
Date: 2012-02-26 14:38 |
Is it totally insane to think about using a float subclass with an additional attribute instead of a tuple?
|
msg154362 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-02-26 15:46 |
@RDM: yes, that's insane for this purpose. The size of the solution doesn't match the size of the problem (the problem is non-existent for most people).
@Larry: That seems unnecessarily general. I take full responsibility for fixing the precision at ns. And note that I *am* giving you a way out -- I'm establishing a clear pattern, and if someone really, desperately wants ps or fs, it's completely obvious how to change the API again. But I bet you a case of beer that won't happen in our lifetimes (or in Benjamin's). So the dynamic precision is just unnneeded complexity.
|
msg154363 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-02-26 15:56 |
I suggest that publishing nanoseconds as a plain int would be a nasty API. Consider what it would do to os.utime:
if isinstance(mtime, int):
# must be st_mtime_ns, it's in nanoseconds, use as-is
value = mtime
else:
assert isinstance(mtime, float)
# must be st_mtime, it's in seconds, multiply by a billion
value = mtime * 1000000000
Have we ever published an API that treated a parameter as two wildly different numbers based solely on whether the parameter was an int or a float?
|
msg154365 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-02-26 15:59 |
(D'oh. My pseudo-code should have said
value = int(mtime * 1000000000)
for that second assignment. Hopefully my point was still clear.)
|
msg154366 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-02-26 16:05 |
How about a separate float attribute for the fractional part (in seconds)? This gives a femtosecond precision (assuming a 50 bits mantissa). The utime() could accept a (integral part, fractional part) tuple to set a timestamp without losing bits.
(PHP does something similar, but badly, with its microtime() function:
http://www.php.net/manual/en/function.microtime.php)
|
msg154367 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-02-26 16:07 |
@pitrou: What would that look like when passed in to os.utime?
|
msg154368 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-02-26 16:18 |
>>> st = os.stat('LICENSE')
>>> st.st_mtime
1330108216.7984242
>>> st.st_mtime_frac
0.798424152
>>> tup = st.st_mtime, st.st_mtime_frac
>>> os.utime('LICENSE', (tup, tup))
Of course, the fact that utime takes a (atime, mtime) tuple makes this a bit cumbersome.
When passed a tuple, utime would ignore the fractional part of the first element.
|
msg154377 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-02-26 18:46 |
> I suggest that publishing nanoseconds as a plain int would be a
> nasty API. Consider what it would do to os.utime:
No, it wouldn't. Please re-read Guido's proposal. If you want to
specify nanoseconds, you have to pass the ns= parameter. My only
quibble with the specific spelling is that it invokes Godwin's law
(but I can live that that as a theoretical concern, also).
> Have we ever published an API that treated a parameter as two wildly
> different numbers based solely on whether the parameter was an int
> or a float?
No, and Guido is on the record for objecting such APIs. Hence the
keyword parameter.
|
msg154378 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-02-26 19:00 |
Thanks Martin. I'd be happy with nsec instead of ns.
|
msg154383 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-02-26 20:38 |
My mistake! That ought to teach me to bound out of bed Sunday morning with my "brilliant realization". (But it probably won't.)
I volunteer to implement this, with the new custom class for the os.stat object. I'll have it done no later than the PyCon sprints next month.
For the record, I actually prefer "ns". "s" is the SI standard abbreviation for second (as is "n" for nano-):
http://en.wikipedia.org/wiki/International_System_of_Units#Units_and_prefixes
If we're going to abbreviate, I think it's reasonable to go with their standard. But I'm willing to be overridden.
|
msg154389 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-02-26 21:05 |
Let's go with ns= them. It's also what POSIX uses (although I initially had problems parsing "futimens", reading it as foo-timen-z, when it really is eff-youtime-en-es)
|
msg154863 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-03-04 00:19 |
Because the use case is to copy the access and modification time from a file to another, I would prefer to use the timespec structure: (sec: int, nsec: int). st_atime_ns and st_mtime_ns fields would be added to os.stat() structure: int in range [0; 999999999].
Copy atime and mtime from src to dst:
st = os.stat(src)
atime = (math.floor(st.st_atime), st.st_atime_ns)
mtime = (math.floor(st.st_mtime), st.st_mtime_ns)
os.utime(dst, (atime, mtime))
os.utime() would accept int, float or (sec: int, nsec: int) for atime and mtime. Examples:
os.utime(name, (1, 1))
os.utime(name, (1.0, 1.0))
os.utime(name, ((1, 0), (1, 0)))
|
msg154864 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-04 00:21 |
> os.utime() would accept int, float or (sec: int, nsec: int) for atime and mtime.
That's not future-proof for when we have better-than-nanosecond
timestamps. See my suggestion above.
|
msg154865 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-04 00:24 |
Actually, I'm hoping that by the time we get better than nanosecond resolution, we can also switch to 128-bit floats, and then the existing st_[acm]time field will become the preferred representation once more. What goes around comes around!
|
msg154866 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-04 00:37 |
> Actually, I'm hoping that by the time we get better than nanosecond
> resolution, we can also switch to 128-bit floats, and then the
> existing st_[acm]time field will become the preferred representation
> once more.
What if your hope isn't fulfilled? That doesn't sound like a reasonable
decision-making strategy.
|
msg154868 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-04 00:40 |
Well, Guido has already nixed future-proof formats, see his comments above:
"I take full responsibility for fixing the precision at ns."
So hope is all I have left.
|
msg154869 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-04 00:43 |
> Well, Guido has already nixed future-proof formats, see his comments
> above:
I don't think Guido is *against* future-proof formats per se, he's
against them when they have a cost compared to non future-proof ones.
The proposal I made (a (integral part, float fractional part) tuple)
doesn't have a cost compared to the plain (int seconds, int nanoseconds)
tuple proposal.
(of course, you can also have nanoseconds as a float, but that starts
being weird)
|
msg154870 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-04 00:48 |
I grant you that (int, float) is probably, theoretically better than (int, int). But it's academic as Guido has ruled against anything but a straight int representing nanoseconds, and I doubt he's gonna change his mind.
|
msg154871 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-03-04 00:50 |
Any solution involving tuple is too ugly. Please stick with the plan.
|
msg154872 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-04 00:50 |
> I grant you that (int, float) is probably, theoretically better than
> (int, int). But it's academic as Guido has ruled against anything but
> a straight int representing nanoseconds, and I doubt he's gonna change
> his mind.
Why not let Guido speak out?
|
msg154873 - (view) |
Author: Arfrever Frehtes Taifersar Arahesis (Arfrever) *  |
Date: 2012-03-04 00:52 |
The following solution might be compatible with Guido's suggestion:
os.stat(path).st_atime_ns -> nanoseconds_since_epoch_as_int
os.stat(path).st_ctime_ns -> nanoseconds_since_epoch_as_int
os.stat(path).st_mtime_ns -> nanoseconds_since_epoch_as_int
atime = os.stat(path).st_atime_ns
mtime = os.stat(path).st_mtime_ns
os.utime(path, (atime, mtime), resolution="ns")
|
msg154874 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-03-04 00:57 |
I don't see how that's better than
os.utime(path, ns=(atime, mtime))
If you think that in the future you could add resolution="fs", well, you could just as easily add fs=(atime, mtime).
|
msg155138 - (view) |
Author: Alyssa Coghlan (ncoghlan) *  |
Date: 2012-03-08 00:21 |
FWIW, +1 on using the *name* of the keyword arg to define the format/resolution of the argument. It's simple and clear right now, and is easily updated to handle higher resolutions in the future.
|
msg155605 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-03-13 13:05 |
I'm lost in all issues related to os.stat/utime and nanosecond, here is a list:
- #10148: duplicate
- #11457: closed (was related the the rejected PEP 410)
- #12904: closed, it was the first step to fix os.stat/os.utime
- #13882: closed (implementation of the PEP 410)
- #13964: duplicate
- #14127: this issue, open :-)
|
msg155608 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-03-13 13:26 |
The following changes has to be done to fix this issue:
- add st_atime_ns, st_mtime_ns and st_ctime_ns fileds to os.stat() result: number of nanoseconds since Epoch (1970.1.1), an integer
- change os.*utime*() functions (see below)
- shutil.copystat() should use os.utime(ns=...) and os.ltime(ns=...)
List of the os.*utime*() functions:
- os.futimes(): use futimens() or futimes()
- os.futimens(): use futimens(); UTIME_NOW and UTIME_OMIT flags
- os.futimesat(): use utimensat() or futimesat()
- os.lutimes(): use futimesat(AT_SYMLINK_NOFOLLOW) or lutimes()
- os.utime(): use SetFileTime() (Windows), utimensat(), utimes() or utime()
- os.utimensat(): use utimensat(); UTIME_NOW and UTIME_OMIT flags
Changes on the os.*utime*() functions:
- add ns keyword to:
* os.futimes()
* os.futimesat()
* os.lutimes()
* os.utime()
- except a number of nanoseconds instead of a number of seconds:
* os.futimens()
* os.utimensat()
The ns keyword is an exclusive parameter with existing times parameter. Examples:
* seconds: os.utime(name, (1, 2))
* seconds: os.utime(name, times=(1, 2))
* nanoseconds: os.utime(name, ns=(1, 2))
* INVALID! os.utime(name, (1, 2), ns=(1, 2))
* INVALID! os.utime(name, times=(1, 2), ns=(1, 2))
I don't want to remove os.futimens() and os.utimensat() because they add a feature: UTIME_NOW and UTIME_OMIT flags.
|
msg155620 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-03-13 14:57 |
Larry, are you sprinting on this? I'd love to help.
|
msg155632 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-03-13 16:44 |
> I don't want to remove os.futimens() and os.utimensat() because they add a feature: UTIME_NOW and UTIME_OMIT flags.
I'm not sure how this could work: UTIME_NOW and UTIME_OMIT have
typically values such as ((1l << 30) - 2l) which could be mistaken
as a time stamp if there is a flat nanosecond value.
There would be ways to solve this, of course: not passing the
value should be considered as UTIME_OMIT, and passing -1 may
be treated as UTIME_NOW.
|
msg155635 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-13 17:29 |
> Larry, are you sprinting on this? I'd love to help.
I am. Come on by--mi CPU es tu CPU!
|
msg155724 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-14 06:18 |
Guido suggested I break the work up into small patches. So here's the first patch: adds st_?time_ns fields to os.stat() result. Includes doc changes and a reasonable smoke-test in the regrtest suite; suite passes all expected tests. FYI I wasn't sure what to call the currently-slightly-uncomfortably-named new function _PyTime_LongFromTime_t().
|
msg155740 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-14 10:04 |
Posted a review on Rietveld.
|
msg155747 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-03-14 12:14 |
> FYI I wasn't sure what to call the currently-slightly-uncomfortably-named new function _PyTime_LongFromTime_t().
There is already a private _PyLong_FromTime_t() function in
_testcapi.c: just move the function to pytime.c (but keep the _Py
prefix).
|
msg155773 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-03-14 17:19 |
I added a review as well. Not sure if that sends email by itself.
|
msg155774 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-14 17:21 |
> I added a review as well. Not sure if that sends email by itself.
It's supposed to send e-mail, but sometimes not all recipients are spelt
right...
|
msg155784 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-14 19:47 |
I was notified by email of both Antoine's and Guido's reviews.
|
msg155868 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-15 07:11 |
My revised patch, incorporating changes suggested by Antoine and Guido.
|
msg155919 - (view) |
Author: Guido van Rossum (gvanrossum) *  |
Date: 2012-03-15 17:53 |
Somehow patch #2 doesn't show up in Rietveld. :-(
Anyway, I'll leave completion of the review to Antoine, who's more competent in this area. :-)
|
msg155938 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-15 19:12 |
Attached is a second patch that adds keyword-only argument support to PyArg_ParseTupleAndKeywords. This was developed in isolation.
The magic character is '$'; I would have used '*', but we already use '*' in the format string as a modifier to 'U' and 's' etc. If someone has a better idea for the format I'd be interested. Martin suggested using '|' a second time but I am unsure.
(Re: no reitveld link: my repo must have drifted too far and the patch didn't align cleanly. I've freshened and am rerunning the regression test; once that runs clean I'll upload a fresh patch.)
|
msg155940 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-15 19:15 |
> Attached is a second patch that adds keyword-only argument support to
> PyArg_ParseTupleAndKeywords. This was developed in isolation.
Please post that on a dedicated issue :)
|
msg155941 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-15 19:19 |
Do you insist? It's a small patch, and I need it as a precondition to posting the... *third* patch in this series, which adds the ns= parameter to utime and its brothers. (Most of the patch is test and doc; it really only adds about twenty lines of C.)
|
msg155942 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-15 19:20 |
A fresh patch for adding st_mtime_ns etc to os.stat output. This is the same as last night's patch but with fresh line numbers. (Generated against 44eba26951f6.)
|
msg155944 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-15 19:26 |
> Do you insist? It's a small patch, and I need it as a precondition to
> posting the...
Yes, I do. It might a small patch, but it changes a core API and should
be discussed on its own.
|
msg155946 - (view) |
Author: Antoine Pitrou (pitrou) *  |
Date: 2012-03-15 19:40 |
Posted a last review on Rietveld.
|
msg155962 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-15 21:50 |
I've created a separate issue for adding keyword-only parameter support to PyArg_ParseTupleAndKeywords, #14328. I've also posted a new patch to that issue incorporating Greg's suggestions. Please file all further comments regarding this topic on that issue.
|
msg156000 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-16 07:01 |
Updated patch incorporating only one of Antoine's latest suggestions. Please see my reply on Reitveld for r3 as to why I skipped two of your three suggestions.
|
msg156181 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-17 17:33 |
Incorporated Greg's (hopefully) parting shot. Also put the "as an integer" back for the docs. Finally, munged the note about float seconds vs integer nanoseconds a bit; I think it's an improvement.
Can I get the go ahead now, pretty please?
|
msg156364 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-19 22:03 |
Revision 7 of my patch. Incorporates all suggestions from Georg. Added "as an integer" in the docs to make Antoine happy.
If I fail to create the "billion" int in INITFUNC, I return NULL and leak references. I figure this is fine, as a) the other such tests in this function also leak, and b) if you can't initialize the os module, you're not going to have a very productive session.
Dare I dream that it's ready for checkin?
|
msg156366 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-03-19 22:15 |
> Revision 7 of my patch.
- assert(sizeof(time_t) <= sizeof(long));
You should keep this assertion in _PyLong_FromTime_t().
|
msg156368 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-19 23:18 |
Incorporated Victor's reinstated assert suggestion. Fingers crossed.
|
msg156369 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-03-19 23:41 |
The patch looks good to me. I was going to commit it but then I realized that Larry is a core developer. Last nits: you should add a newline after } in "} else {", and it would be better to move billion variable into fill_time(). Something like:
static void
fill_time(PyObject *v, int index, time_t sec, unsigned long nsec)
{
...
static PyObject *billion = NULL;
if (billion == NULL) {
billion = PyLong_FromLong(1000000000);
if (!billion)
return;
}
...
}
|
msg156375 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-03-20 01:27 |
> The patch looks good to me. I was going to commit it
> but then I realized that Larry is a core developer.
Indeed; please permit me the pleasure of checking it in myself. That is, assuming I actually get the go-ahead someday. (Probably in the far-flung future. I'm imagining my house CPU telling me the good news via my cranial implant. Perhaps while I'm landing my jet car!)
> Last nits: you should add a newline after } in "} else {",
1/3 of the "else {" statements in the Python tree are "} else {". And that's not a new line with me; I merely renamed the variable.
But I just checked, and PEP 7 says so, and I figure it's harmless enough, so I have made that change.
> and it would be better to move billion variable into fill_time().
That's how it was until patch 5 or 6 or so. Greg P. Smith suggested moving into INITFUNC and I agreed. I think it should stay where it is.
Attached is patch #9, adding a newline between "}" and "else" as Victor suggests.
|
msg157423 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-04-03 14:19 |
Sorry to let this slide but I just got back from vacation. Unless anybody has any more feedback I'm checking this in tomorrow (Wednesday April 4). Phew!
|
msg158764 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-04-19 22:08 |
New changeset f554043badec by Larry Hastings in branch 'default':
Issue #14127: Add st_{cma}time_ns fields to os.stat() result object.
http://hg.python.org/cpython/rev/f554043badec
|
msg158771 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-04-19 22:58 |
Sorry about the delay; laptop died, finally dealt with reviving the data off it. Also: fixed spelling error in title.
|
msg159219 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-04-24 22:05 |
Attached is round 1 of my patch adding the ns= parameter to utime, futimes, and lutimes.
Some notes:
* I admit the "see utime for use of times and ns" documentation dodge
(for both Doc and docstring) is lazy. Yet I'm still hoping to get
away with it.
* I removed futimens because it's now extraneous.
* I didn't add ns= to utimensat or futimesat. That's because I'm hoping to get rid of them via #14626. (And lutimes too, come to think of it!) I hereby promise that if they survive to June 15th I'll add ns= support
before 3.3 hits beta.
* At last, shutil.copystat (and therefore shutil.copy2) now *exactly*
preserves timestamps. :D
|
msg159221 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-04-24 22:10 |
> I removed futimens because it's now extraneous.
futimens() has nice feature: it is possible to only update atime only update mtime, or use "now" as the new atime and/or mtime.
3882 If *_nsec is specified as UTIME_NOW, the timestamp is updated to the\n\
3883 current time.\n\
3884 If *_nsec is specified as UTIME_OMIT, the timestamp is not updated.
os.utimensat() has the same feature. Is it possible that os.utimensat() is not available whereas os.futimens() is availalble?
|
msg159222 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-04-24 22:15 |
> futimens() has nice feature: it is possible to only update atime
> only update mtime, or use "now" as the new atime and/or mtime.
YAGNI. Worst case, you can use call futimes twice, once with no args, then fstat() it to get the current-ish time and rewrite the fields selectively.
Do you have code where you selectively use UTIME_NOW for only one of the two fields?
|
msg159239 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-04-25 00:10 |
Second round of my patch, incorporating nearly all of Victor's suggestions. Thanks, Victor!
|
msg159361 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-04-26 09:48 |
Anybody else? I guess I'm gonna juuuust miss Alpha 3, but if nobody has any other objections I'll check this in today (Thursday).
If you want me to hold off just let me know.
|
msg159410 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-04-26 18:40 |
> Somehow patch #2 doesn't show up in Rietveld. :-(
It's because it's a git-style diff, so it includes no base revision,
and it didn't apply cleanly to the default head (which is tried as
a fall-back in case of a missing base revision).
|
msg159416 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-04-26 19:30 |
FYI, Martin was replying to Guido's comment from more than a month ago, when I posted revision #2 of my first patch series. By sheer coincidence I posted revision #2 of a new patch series yesterday. But Reitveld worked fine for that!
Anyway--no comments? Normally I find the patch review process akin to getting pecked to death by ducks. It's hard to believe this one might go in after only one revision. Somebody pinch me!
|
msg159637 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-04-29 17:57 |
Failure on x86 OpenIndiana 3.x:
FAIL: test_stat_attributes (test.test_os.StatAttributeTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/test/test_os.py", line 240, in test_stat_attributes
self.check_stat_attributes(self.fname)
File "/export/home/buildbot/32bits/3.x.cea-indiana-x86/build/Lib/test/test_os.py", line 199, in check_stat_attributes
self.assertEqual(floaty, nanosecondy)
AssertionError: 133572199178458 != 133572199178457
http://www.python.org/dev/buildbot/all/builders/x86%20OpenIndiana%203.x/builds/3494/steps/test/logs/stdio
|
msg159836 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-05-03 07:31 |
New changeset bba131e48852 by Larry Hastings in branch 'default':
Issue #14127: Add ns= parameter to utime, futimes, and lutimes.
http://hg.python.org/cpython/rev/bba131e48852
|
msg159837 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-05-03 07:37 |
@haypo: Thanks for pointing that out buildbot failure! The OpenIndiana buildbot was bit by a rounding error. I fixed it by adding in a fudge factor it--I snuck in one last change just now. I weakened the unit test as follows:
- self.assertEqual(floaty, nanosecondy)
+ self.assertAlmostEqual(floaty, nanosecondy, delta=2)
Also: I'm leaving this issue open for now, to remind myself to add ns= to utimensat and futimesat if they're still alive on June 15th.
|
msg159840 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-05-03 08:18 |
> The OpenIndiana buildbot was bit by a rounding error.
How do we have rounding issue with only integers?
|
msg159841 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-05-03 08:24 |
We don't!
The test that failed compares the ns_[amc]time and ns_[amc]time_ns fields to ensure they're roughly equivalent. Note that the former fields are floats, which by now ought not to be news to you.
|
msg159857 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2012-05-03 12:57 |
http://www.python.org/dev/buildbot/all/builders/AMD64%20OpenIndiana%203.x/builds/3388/steps/test/logs/stdio
ERROR: test_copy2_symlinks (test.test_shutil.TestShutil)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/export/home/buildbot/64bits/3.x.cea-indiana-amd64/build/Lib/test/test_shutil.py",
line 328, in test_copy2_symlinks
shutil.copy2(src_link, dst, symlinks=True)
File "/export/home/buildbot/64bits/3.x.cea-indiana-amd64/build/Lib/shutil.py",
line 193, in copy2
copystat(src, dst, symlinks=symlinks)
File "/export/home/buildbot/64bits/3.x.cea-indiana-amd64/build/Lib/shutil.py",
line 157, in copystat
utime_func(dst, ns=(st.st_atime_ns, st.st_mtime_ns))
TypeError: _nop() got an unexpected keyword argument 'ns'
|
msg159871 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-05-03 19:57 |
New changeset cdc4e0f8135d by Larry Hastings in branch 'default':
Issue #14127: Fix no-op stub for platforms that lack some "os" functions.
http://hg.python.org/cpython/rev/cdc4e0f8135d
|
msg159891 - (view) |
Author: Richard Oudkerk (sbt) *  |
Date: 2012-05-04 00:16 |
bba131e48852 causes crashes on Windows.
The attached patch fixes the crash and makes test_os pass for me.
However, using "PyErr_ExceptionMatches(PyExc_RuntimeError)" to check whether to try again using narrow strings is ugly. Maybe utime_read_time_arguments() should be changed to have three possible return values.
|
msg159904 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-05-04 06:22 |
> bba131e48852 causes crashes on Windows.
>
> The attached patch fixes the crash and makes test_os pass for me.
>
> However, using "PyErr_ExceptionMatches(PyExc_RuntimeError)" to check
> whether to try again using narrow strings is ugly. Maybe
> utime_read_time_arguments() should be changed to have three possible
> return values.
I appreciate the feedback, and the patch. And I agree--we should be able to find a better fix than that particular band-aid. Can we hold off on checking in a patch for now?
TBH I don't understand why it should crash, and therefore how your patch helps. Trying again using narrow strings should always work; indeed, the code did that before I touched it. Can you describe how it crashes?
(p.s. Considering that I can't test on Windows myself, I'm pretty happy that the code works as well as it does!)
|
msg159909 - (view) |
Author: Richard Oudkerk (sbt) *  |
Date: 2012-05-04 07:51 |
> TBH I don't understand why it should crash, and therefore how your patch
> helps. Trying again using narrow strings should always work; indeed, the
> code did that before I touched it. Can you describe how it crashes?
The important part of the patch is the removal of the "!" in
if (!utime_read_time_arguments(&ua)) {
Without that change, if utime_read_time_arguments(&ua) fails then the unicode path is wrongly chosen. Then PyUnicode_AsUnicode(upath) is called when upath has not been initialized.
|
msg159911 - (view) |
Author: Richard Oudkerk (sbt) *  |
Date: 2012-05-04 08:03 |
Without the check for RuntimeError
os.utime("foo", times=(5,5), ns=(5,5))
raises
TypeError("TypeError: 'str' does not support the buffer interface")
because we have fallen through to the narrow path. The correct error is
RuntimeError("utime: you may specify either 'times' or 'ns' but not both")
|
msg159912 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-05-04 09:17 |
Let me recap, just to make sure I have it straight. There are two errors on Windows:
* The ! on (what is currently) line 3770 is wrong:
if (!utime_read_time_arguments(&ua)) {
* If you pass in a Unicode string but also pass in both times and ns,
you get the wrong error: effectively it's complaining that the string
is not narrow, when it should be complaining about having both times
and ns.
For the former, obviously removing the ! is correct. But I like your idea of making the utime_read_time_argument() return value tell you what happened; that's what the Windows code needs to know.
So here it is! Please see the attached patch.
|
msg159913 - (view) |
Author: Richard Oudkerk (sbt) *  |
Date: 2012-05-04 09:30 |
> Let me recap, just to make sure I have it straight. There are two errors
> on Windows:
That's right. The patch looks good and passes for me on Windows.
|
msg159914 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-05-04 09:32 |
New changeset fc5d2f4291ac by Larry Hastings in branch 'default':
Issue #14127: Fix two bugs with the Windows implementation.
http://hg.python.org/cpython/rev/fc5d2f4291ac
|
msg159919 - (view) |
Author: Richard Oudkerk (sbt) *  |
Date: 2012-05-04 10:48 |
There is another problem causing a fatal error in test_posix on Unix.
The attached patch fixes it: *ua->path should be decrefed not ua->path.
|
msg159920 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-05-04 10:52 |
Looks good to me. You're a core contributor, yes? If not let me know and I'll commit it.
Though I must admit I'm baffled how I haven't seen that crash. I've run the unit tests a zillion times on this patch.
|
msg159921 - (view) |
Author: Richard Oudkerk (sbt) *  |
Date: 2012-05-04 10:57 |
> Looks good to me. You're a core contributor, yes? If not let me know and
> I'll commit it.
I will commit.
> Though I must admit I'm baffled how I haven't seen that crash. I've run
> the unit tests a zillion times on this patch.
Were you running test_posix or only test_os?
|
msg159922 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-05-04 11:00 |
By "the unit tests" I meant I ran the whole suite: Lib/test/regrtest.py. I know that runs test_os, and I assume it runs test_posix too.
I just ran test_posix by hand and it passed.
I'm developing on Linux (64-bit) in case that helps.
|
msg159924 - (view) |
Author: Richard Oudkerk (sbt) *  |
Date: 2012-05-04 11:15 |
> I'm developing on Linux (64-bit) in case that helps.
I tested it on 32 bit Linux.
I have committed it, but I forgot to put the issue number in the commit message.
|
msg159925 - (view) |
Author: Martin v. Löwis (loewis) *  |
Date: 2012-05-04 11:22 |
Revision 4deb7c1f49b9
|
msg160050 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-05-06 00:40 |
New changeset 709850f1ec67 by Larry Hastings in branch 'default':
Update Misc/NEWS for issues #14127 and #14705. (And, technically, #10148.)
http://hg.python.org/cpython/rev/709850f1ec67
|
msg160054 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2012-05-06 04:58 |
New changeset 05274ab06182 by Larry Hastings in branch 'default':
Update Misc/NEWS for issues #14127 and #14705. (And, technically, #10148.)
http://hg.python.org/cpython/rev/05274ab06182
|
msg163526 - (view) |
Author: Larry Hastings (larry) *  |
Date: 2012-06-23 03:00 |
Closing this, as I have now removed os.utimensat and os.futimesat. As well as os.futimens, os.futimes, and os.lutimes. And in fact retooled os.utime in a pretty major way. (See #14626.)
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:27 | admin | set | github: 58335 |
2018-01-18 02:59:27 | jcea | set | nosy:
+ jcea
|
2012-06-23 03:00:19 | larry | set | status: open -> closed resolution: fixed messages:
+ msg163526
stage: patch review -> resolved |
2012-05-06 04:58:44 | python-dev | set | messages:
+ msg160054 |
2012-05-06 00:40:17 | python-dev | set | messages:
+ msg160050 |
2012-05-04 11:22:39 | loewis | set | messages:
+ msg159925 |
2012-05-04 11:15:53 | sbt | set | messages:
+ msg159924 |
2012-05-04 11:00:19 | larry | set | messages:
+ msg159922 |
2012-05-04 10:57:08 | sbt | set | messages:
+ msg159921 |
2012-05-04 10:52:13 | larry | set | messages:
+ msg159920 |
2012-05-04 10:48:58 | sbt | set | files:
+ utime_read_time_arguments.patch
messages:
+ msg159919 |
2012-05-04 09:32:56 | python-dev | set | messages:
+ msg159914 |
2012-05-04 09:30:48 | sbt | set | messages:
+ msg159913 |
2012-05-04 09:17:10 | larry | set | files:
+ larry.utime.win32.bugfix.1.patch
messages:
+ msg159912 |
2012-05-04 08:03:49 | sbt | set | messages:
+ msg159911 |
2012-05-04 07:51:55 | sbt | set | messages:
+ msg159909 |
2012-05-04 06:22:21 | larry | set | messages:
+ msg159904 |
2012-05-04 00:16:43 | sbt | set | files:
+ utime-hack.patch nosy:
+ sbt messages:
+ msg159891
|
2012-05-03 19:57:40 | python-dev | set | messages:
+ msg159871 |
2012-05-03 12:57:15 | vstinner | set | messages:
+ msg159857 |
2012-05-03 08:24:23 | larry | set | messages:
+ msg159841 |
2012-05-03 08:18:57 | vstinner | set | messages:
+ msg159840 |
2012-05-03 07:37:27 | larry | set | messages:
+ msg159837 |
2012-05-03 07:31:05 | python-dev | set | messages:
+ msg159836 |
2012-04-29 17:57:52 | vstinner | set | messages:
+ msg159637 |
2012-04-26 19:30:09 | larry | set | messages:
+ msg159416 |
2012-04-26 18:59:02 | Arfrever | set | title: add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds -> add st_*time_ns fields to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds |
2012-04-26 18:40:14 | loewis | set | messages:
+ msg159410 title: add st_*time_ns fields to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds -> add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds |
2012-04-26 09:48:30 | larry | set | messages:
+ msg159361 |
2012-04-25 00:10:29 | larry | set | files:
+ larry.utime.ns.2.patch
messages:
+ msg159239 |
2012-04-24 22:15:16 | larry | set | messages:
+ msg159222 |
2012-04-24 22:10:16 | vstinner | set | messages:
+ msg159221 |
2012-04-24 22:05:15 | larry | set | files:
+ larry.utime.ns.1.patch keywords:
+ patch messages:
+ msg159219
|
2012-04-19 22:58:15 | larry | set | messages:
+ msg158771 title: add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds -> add st_*time_ns fields to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds |
2012-04-19 22:08:47 | python-dev | set | messages:
+ msg158764 |
2012-04-03 14:19:59 | larry | set | messages:
+ msg157423 |
2012-03-20 01:27:24 | larry | set | files:
+ larry.st_mtime_ns.patch.9.txt
messages:
+ msg156375 |
2012-03-19 23:41:40 | vstinner | set | messages:
+ msg156369 |
2012-03-19 23:18:05 | larry | set | files:
+ larry.st_mtime_ns.patch.8.txt
messages:
+ msg156368 |
2012-03-19 22:15:35 | vstinner | set | messages:
+ msg156366 |
2012-03-19 22:03:54 | larry | set | files:
+ larry.st_mtime_ns.patch.7.txt
messages:
+ msg156364 |
2012-03-17 17:33:50 | larry | set | files:
+ larry.st_mtime_ns.patch.6.txt assignee: larry messages:
+ msg156181
stage: patch review |
2012-03-16 07:01:05 | larry | set | files:
+ larry.st_mtime_ns.patch.4.txt
messages:
+ msg156000 |
2012-03-15 21:50:47 | larry | set | messages:
+ msg155962 |
2012-03-15 20:02:15 | gregory.p.smith | set | nosy:
+ gregory.p.smith
|
2012-03-15 19:40:07 | pitrou | set | messages:
+ msg155946 |
2012-03-15 19:26:41 | pitrou | set | messages:
+ msg155944 |
2012-03-15 19:20:27 | larry | set | files:
+ larry.st_mtime_ns.patch.3.txt
messages:
+ msg155942 |
2012-03-15 19:19:29 | larry | set | messages:
+ msg155941 |
2012-03-15 19:15:38 | pitrou | set | messages:
+ msg155940 |
2012-03-15 19:12:22 | larry | set | files:
+ larry.parsekwonly.diff.1.txt
messages:
+ msg155938 |
2012-03-15 17:53:21 | gvanrossum | set | messages:
+ msg155919 |
2012-03-15 07:11:33 | larry | set | files:
+ larry.st_mtime_ns.patch.2.txt
messages:
+ msg155868 |
2012-03-14 19:47:58 | larry | set | messages:
+ msg155784 |
2012-03-14 17:21:37 | pitrou | set | messages:
+ msg155774 |
2012-03-14 17:19:56 | gvanrossum | set | messages:
+ msg155773 |
2012-03-14 12:14:45 | vstinner | set | messages:
+ msg155747 title: add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds -> add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds |
2012-03-14 10:04:12 | pitrou | set | messages:
+ msg155740 |
2012-03-14 06:18:53 | larry | set | files:
+ larry.st_mtime_ns.patch.1.txt
messages:
+ msg155724 |
2012-03-13 17:29:45 | larry | set | messages:
+ msg155635 |
2012-03-13 16:44:01 | loewis | set | messages:
+ msg155632 title: add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds -> add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds |
2012-03-13 14:57:17 | gvanrossum | set | messages:
+ msg155620 |
2012-03-13 13:26:53 | vstinner | set | messages:
+ msg155608 |
2012-03-13 13:06:42 | vstinner | set | title: os.stat and os.utime: allow preserving exact metadata -> add st_*time_ns fileds to os.stat(), add ns keyword to os.*utime*(), os.*utimens*() expects a number of nanoseconds |
2012-03-13 13:05:08 | vstinner | set | nosy:
+ eric.araujo, maubp, shaurz, python-dev messages:
+ msg155605
|
2012-03-08 00:21:29 | ncoghlan | set | nosy:
+ ncoghlan messages:
+ msg155138
|
2012-03-04 00:57:02 | gvanrossum | set | messages:
+ msg154874 |
2012-03-04 00:52:28 | Arfrever | set | messages:
+ msg154873 |
2012-03-04 00:50:52 | pitrou | set | messages:
+ msg154872 |
2012-03-04 00:50:32 | gvanrossum | set | messages:
+ msg154871 |
2012-03-04 00:48:38 | larry | set | messages:
+ msg154870 |
2012-03-04 00:43:12 | pitrou | set | messages:
+ msg154869 |
2012-03-04 00:40:23 | larry | set | messages:
+ msg154868 |
2012-03-04 00:37:58 | pitrou | set | messages:
+ msg154866 |
2012-03-04 00:24:23 | larry | set | messages:
+ msg154865 |
2012-03-04 00:21:09 | pitrou | set | messages:
+ msg154864 |
2012-03-04 00:19:15 | vstinner | set | nosy:
+ vstinner messages:
+ msg154863
|
2012-02-27 03:58:22 | rosslagerwall | set | nosy:
+ rosslagerwall
|
2012-02-26 21:05:45 | loewis | set | messages:
+ msg154389 |
2012-02-26 20:38:49 | larry | set | messages:
+ msg154383 |
2012-02-26 19:00:04 | gvanrossum | set | messages:
+ msg154378 |
2012-02-26 18:46:31 | loewis | set | messages:
+ msg154377 |
2012-02-26 17:59:00 | Arfrever | set | nosy:
+ Arfrever
|
2012-02-26 16:18:07 | pitrou | set | messages:
+ msg154368 |
2012-02-26 16:07:18 | larry | set | messages:
+ msg154367 |
2012-02-26 16:05:35 | pitrou | set | nosy:
+ pitrou messages:
+ msg154366
|
2012-02-26 15:59:43 | larry | set | messages:
+ msg154365 |
2012-02-26 15:56:44 | larry | set | messages:
+ msg154363 |
2012-02-26 15:46:09 | gvanrossum | set | nosy:
+ gvanrossum messages:
+ msg154362
|
2012-02-26 14:38:28 | r.david.murray | set | nosy:
+ r.david.murray messages:
+ msg154355
|
2012-02-26 12:05:37 | loewis | set | nosy:
+ loewis messages:
+ msg154334
|
2012-02-26 09:42:59 | larry | set | messages:
+ msg154319 |
2012-02-26 09:23:21 | larry | create | |