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
fdo#67235
https://bugs.freedesktop.org/67235
https://bugs.freedesktop.org/67387
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

   Disadvantages:

    - we break ABI/API in point release! OTOH, it is .1, not .5 or
      something like that...

   Advantages:

    - 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).

     http://api.libreoffice.org/docs/common/ref/com/sun/star/form/validation/XValidatableFormComponent.html#getCurrentValue

   - 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
      them...)

   - A few things that are expected to be no-ops will no longer be:

     1) x.setTime(x.getModel().getCurrentValue())

        in Basic: x.Time = x.Model.CurrentValue

     2) x.getModel().Time = x.getTime()

       in C++, probably something like
       x.getModel().setPropertyValue("Time", x.getTime())

       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()
        methods.)

   Disadvantages:

   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
      be used.

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
   (with css.util.Date).

   Advantages:

    * 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.

   Disadvantages:

    * 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.

   Disadvantage:

   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 mailing list