More time-related API headache
Lionel Elie Mamane
lionel at mamane.lu
Mon Jul 29 03:40:34 PDT 2013
> * Completed Action Items
> + working on Date API fix for rc3 (Eike/Michael S)
> [ made it into rc3 - many thanks to all concerned !
> Eike, Lionel, Michael S ]
Well, about that family of things, I just handled fdo#67387 and
I put these as blocker to mark them as "decision needs to be taken
before next release".
They originate from the fact that some code was not adapted to the
"centisecond -> nanosecond precision in time datatypes" API change.
Essentially, some parts of the code use sal_Int32 to deal with a time,
instead of any of our multiple time datatypes, sometimes / often with
the help of automatic conversion operators of ::Time (from
tools/time.hxx). *Some* of that code (and in particular ::Time) was
adapted to now use sal_Int64, where the meaning is:
sal_int32 oldValue * 1E7 == sal_Int64 newValue
(1E7, that is 10^7 is the conversion factor between nanoseconds and
centiseconds; but no, these values are not "centi/nanoseconds since
midnight". They are written in decimal as HHMMSScc and
HHMMSSnnnnnnnnn, respectively, where HH is the hours, MM the
minutes, cc the centiseconds and nnnnnnnnn the nanoseconds)
The *very* annoying thing is that it bleeds into a (published!) API,
namely com.sun.star.awt.XTimeField. Why that API uses an integer
instead of com.sun.star.util.Time, I'm not sure; maybe just *old* API
compatibility from the days before com.sun.star.util.Time existed.
So, now the choices in front of us are:
1) Break ABI/API in 4.1.1 (wrt to 4.1.0) to align
com.sun.star.awt.XTimeField to our internal code:
long -> hyper
That's what I prepared at https://gerrit.libreoffice.org/5149
- we break ABI/API in point release! OTOH, it is .1, not .5 or
something like that...
- we don't break ABI/API again in 4.2
- since everything uses the same, simpler code, easier to
understand, to maintain, to fix, higher probability
that no bugs like fdo#67235 / fdo#67387 are left, ...
2) Same as 1), but in 4.2
Fixing fdo#67235 / fdo#67387 in 4.1 will be harder, since (IMHO)
some of our own code calls com.sun.star.awt.XTimeField. I don't
have a good plan to do that in my head (yet).
3) Leave com.sun.star.awt.XTimeField alone and deprecate it, add
com.sun.star.awt.XTimeFieldNano or com.sun.star.awt.XTimeField2,
forever having "bad" names in the new, recommended, interface
(e.g. setTime2 or setTimeNano instead of setTime).
Fixing fdo#67235 / fdo#67387 in 4.1 will be harder, but probably
not as much as solution 2); "just" need to switch all internal
callers to the new interface.
Note that this does not (by far) give as much backwards
compatibility as one may at first think:
- getCurrentValue() will still be changed to return a hyper (in an
Any) instead of a long (in an Any).
- C++ extension code that used our ::Time datatype to convert back
and forth from an integer gotten from getTime* / to be given to
setTime* will still compile and get WRONG results.
- Touching/accessing these values / settings through properties
instead of through com.sun.star.awt.XTimeField will *still* be
non-backwards compatible because the properties will *still* be
changed to being hyper (instead of long).
(Unless we do the same dance with the properties... Doubling
- A few things that are expected to be no-ops will no longer be:
in Basic: x.Time = x.Model.CurrentValue
2) x.getModel().Time = x.getTime()
in C++, probably something like
in Basic: x.Model.Time = x.Time
(The model has a true "Time" property and the control has a
pseudo-property "Time" through getTime() and setTime()
1) this is exactly the kind of cruft-baggage solution we
decided to avoid for LibreOffice 4.
2) Still incompatible with how (some ways that) these APIs are/can
4) Since we break API/ABI anyway, break it more violently and change
to using css.util.Time instead of integers.
To be coherent, we'd have to do the same to css.awt.XDateField
* nice, coherent API for the future.
* the break is also "more obvious"; no compiler will automatically
convert between long and struct css.util.Time, while they do
that silently between long and hyper.
* that's a bigger change to be done in a point release
(OTOH, it is .1, not .5 or something like that...). Unless QA
beats the crap out it, more risk of introducing bugs in such a
change (compared to 1) (see how late in the process
fdo#67387 and fdo#67235 were found...: from MARCH to JULY, that
is FOUR MONTHS; OTOG "nobody tests before the final release" or
something like that ;) ).
5) Keep only com.sun.star.awt.XTimeField, "fixing" the spots that
confuse it with hyper / nanoseconds.
From a database POV, the necessary resolution is whatever the user
wants; if they store "time of my appointment with the dentist",
then centiseconds is already overkill. If they store timing
information of a chemistry / physics experiment or of execution
traces of a computer program (profiling), then centiseconds may
very well not be enough.
I would like, at some point, to increase the resolution available
to users above centiseconds. With option 5, we'll have to break API
at that point... and take something like solutions 1 to 4 in
... LibreOffice 4.4? LibreOffice 4.5? We crammed the "timezone" API
change in 4.1.0.rc3 to avoid breaking date&time API in 4.x (x >=
2), so that does not look palatable. Or will we do a LibreOffice 5
to break API again?
More information about the LibreOffice