Bug Hunting Session
Bug 92693 - EDITING: Function MINUTE() always 1 minute greater than minute in timefield/timestampfield in ReportDesigner
Summary: EDITING: Function MINUTE() always 1 minute greater than minute in timefield/t...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: Other Linux (All)
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.1.0 target:5.0.1 target:4.4.6
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-12 17:13 UTC by Robert Großkopf
Modified: 2016-10-25 19:17 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
Open the report report_hour_minute_second. MINUTE gives wrong value. (13.17 KB, application/vnd.oasis.opendocument.database)
2015-07-12 17:13 UTC, Robert Großkopf
Details
experimental untested patch (923 bytes, patch)
2015-07-13 10:48 UTC, Lionel Elie Mamane
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Großkopf 2015-07-12 17:13:56 UTC
Created attachment 117191 [details]
Open the report report_hour_minute_second. MINUTE gives wrong value.

Open the attached database.
Open the report report_hour_minute_second.
You could see the timestampfields and timefields. There is extracted also the value for HOUR, MINUTE and SECOND. All extracted values for MINUTE are 1 minute greater than the right value of the timestampfields or timefields.

Tried to get the TIMEVALUE this way because of bug92654, but got another bug.
Comment 1 Alex Thurgood 2015-07-12 18:39:09 UTC
Confirmed on LO 4432

Seems to be a rounding problem/

If I set the time value of record 1 to less than 30s, e.g. 23, or 29, I get a minutes value of 7.

In Robert's example, all the values for seconds are above 30 and appear to be rounded up.

Indeed, as soon as I set the seconds value to 30, the minute value gets rounded up to 8.
Comment 2 Robert Großkopf 2015-07-13 05:50:00 UTC
Have tried the rounding and got another bug with SECOND:

17:37:33 will get
HOUR: 17
MINUTE: 38
SECOND: 32

9:39:31 will get
HOUR: 9
MINUTE: 40
SECOND: 30

This all would happen when MINUTE is between 30 and 45 and SECOND is greater than 30.

Don't know if we should open another bug for this behavior.
Comment 3 Lionel Elie Mamane 2015-07-13 07:57:48 UTC
Julien, if you want to take this one, try this:

In the external "jfreereport", in libformula, file source/org/pentaho/reporting/libraries/formula/function/datetime/MinuteFunction.java, replace

    final BigDecimal minutesAsInt = minutes.setScale(0, BigDecimal.ROUND_HALF_UP);
by
    final BigDecimal minutesAsInt = minutes.setScale(0, BigDecimal.ROUND_DOWN);


Since this is an external, you will need to do that by adding a patch in external/jfreereport/patches/, reference it in UnpackedTarball_jfreereport_libformula.mk, and build with --without-system-jfreereport.

(This suggestion is untested, but I'm fairly confident it will work.)

Actually, comparing with the HOURS and SECONDS functions, I think it should be something like:

    final BigDecimal minutesAsInt =  NumberUtil.performIntRounding(minutes);

That would be even better.
Comment 4 Julien Nabet 2015-07-13 09:22:50 UTC
Lionel: I tried to apply this https://wiki.documentfoundation.org/Development/Patching_External_Libraries but it failed. (what's target, what's module for jfreereport?)
The few times I made a patch for an external lib, it was after several tries and I had only succeeded by chance. This time, it doesn't work.
If you know a better way/url which always works, just tell me! :-)

BTW, I put tdf#67832 since perhaps an upgrade of this lib may already fix this.
Comment 5 Lionel Elie Mamane 2015-07-13 09:34:32 UTC
module jfreereport
target UnpackedTarball_jfreereport_libformula or ExternalProject_jfreereport_libformula
Comment 6 Julien Nabet 2015-07-13 09:42:53 UTC
"cd <module>" can't work since there's no jfreereport in root LO sources.

So I tried "cd external/jfreereport"
"make -r" gives
[build BIN] top level modules: jfreereport
[build LOC] top level modules: jfreereport
[build ALL] top level modules: build-non-l10n-only build-l10n-only
make: Nothing to be done for 'all'.

Then, I tried:
make -r ExternalProject_jfreereport_libformula.rebuild
if [ -f /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ExternalProject_jfreereport_libformula.done ] ; then \
	touch /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ExternalProject_jfreereport_libformula.done ; \
	make ;\
fi
julien@julienPC:~/compile-libreoffice/libreoffice/external/jfreereport$ make ExternalProject_jfreereport_libformula.genpatch
if [ -d /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ExternalProject_jfreereport_libformula -a -d  /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ExternalProject_jfreereport_libformula.org ] ; then \
	( \
	    patch_file=$(pwd)/ExternalProject_jfreereport_libformula.new.patch.1; \
		cd /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ ; \
		diff -ur ExternalProject_jfreereport_libformula.org ExternalProject_jfreereport_libformula > $patch_file; \
	    echo "Patch $patch_file generated" ; \
	); \
else \
	echo "Error: No pristine tarball available for ExternalProject_jfreereport_libformula" 1>&2 ; \
fi
Error: No pristine tarball available for ExternalProject_jfreereport_libformula


finally, I tried this:
make -r UnpackedTarball_jfreereport_libformula.rebuild
if [ -f /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/UnpackedTarball_jfreereport_libformula.done ] ; then \
	touch /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/UnpackedTarball_jfreereport_libformula.done ; \
	make ;\
fi
julien@julienPC:~/compile-libreoffice/libreoffice/external/jfreereport$ make ExternalProject_jfreereport_libformula.genpatch
if [ -d /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ExternalProject_jfreereport_libformula -a -d  /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ExternalProject_jfreereport_libformula.org ] ; then \
	( \
	    patch_file=$(pwd)/ExternalProject_jfreereport_libformula.new.patch.1; \
		cd /home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/ ; \
		diff -ur ExternalProject_jfreereport_libformula.org ExternalProject_jfreereport_libformula > $patch_file; \
	    echo "Patch $patch_file generated" ; \
	); \
else \
	echo "Error: No pristine tarball available for ExternalProject_jfreereport_libformula" 1>&2 ; \
fi
Error: No pristine tarball available for ExternalProject_jfreereport_libformula
Comment 7 Lionel Elie Mamane 2015-07-13 09:46:27 UTC
Sorry, module "external/jfreereport"

The error is probably because you have --with-system(-jfreereport)

Then, do the make commands from the toplevel, it should automatically download the tarballs.
Comment 8 Julien Nabet 2015-07-13 09:55:31 UTC
"make jfreereport.clean" in external fails.
julien@julienPC:~/compile-libreoffice/libreoffice/external$ make jfreereport.clean
make: *** No rule to make target 'jfreereport.clean'. Arrêt.

I don't have --with-system(-jfreereport) in my autogen.input

I think we both waste our time here. Certainly the time you explain to me all the stuff would be sufficient for you to make this patch at least 3 times.

So I give up with this bugtracker, I'll try to patch an external lib when the process to do it will be more appropriate for dumbs like me:-)

Also I don't know if patching an old version of jfreereport worth it (see tdf#67832 you had created)
Comment 9 Lionel Elie Mamane 2015-07-13 10:47:54 UTC
I was sneakily trying to get you familiarised with externals so that you can do extra work in the future ;)

Here's what I did after some fumbling around (I myself haven't touched an external since they moved to the subdirectory external/, so there was fumbling associated)

from top directory:

"make jfreereport.clean" works

edit autogen.input, add --without-system-jfreereport

"make jfreereport" works

then:

$ make jfreereport.clean
$ export patches=t
$ make jfreereport
$ emacs workdir/UnpackedTarball/jfreereport_libformula/source/org/pentaho/reporting/libraries/formula/function/datetime/MinuteFunction.java 
## cd external/freereport && make 'UnpackedTarball_jfreereport_libformula' does nothing
## cd external/freereport && make 'UnpackedTarball_jfreereport_libformula.rebuild' doesn't seem to rebuild either
## ditto with "ExternalProject_jfreereport_libformula" and "ExternalPackage_jfreereport_libformula"
## aaah!!! This works:
$ make jfreereport_libformula.rebuild
## the part before "rebuild" is the name of the directory in workdir/UnpackedTarball/; this seems to be the important part
$ make jfreereport_libformula.genpatch
## works also...

This created the attached patch, Julien, could you test it? (stick it in external/jfreereport/patches, add it to UnpackedTarball_jfreereport_libformula.mk)
Comment 10 Lionel Elie Mamane 2015-07-13 10:48:33 UTC
Created attachment 117200 [details]
experimental untested patch
Comment 11 Lionel Elie Mamane 2015-07-13 10:53:15 UTC
FWIW, the latest sources on github (of libformula) still have this bug. If we can localise a bugtracker... 't would be nice to give that upstream, too.
Comment 12 Julien Nabet 2015-07-13 12:56:41 UTC
I renamed the patch tdf92693_fix.patch,
changed UnpackedTarball_jfreereport_libformula.mk to have this:
     18 $(eval $(call gb_UnpackedTarball_add_patches,jfreereport_libformula,\
     19         external/jfreereport/patches/common_build.patch \
     20         external/jfreereport/patches/libformula-time-notz.patch \
     21         external/jfreereport/patches/tdf92693_fix.patch \
     22 ))

then "make jfreereport.clean && make jfreereport.build", it failed with this:

[build PAT] jfreereport_libserializer
The text leading up to this was:
--------------------------
|diff -ur jfreereport_libformula.org/source/org/pentaho/reporting/libraries/formula/function/datetime/MinuteFunction.java jfreereport_libformula/source/org/pentaho/reporting/libraries/formula/function/datetime/MinuteFunction.java
|--- jfreereport_libformula.org/source/org/pentaho/reporting/libraries/formula/function/datetime/MinuteFunction.java	2015-07-13 12:26:32.731046395 +0200
|+++ jfreereport_libformula/source/org/pentaho/reporting/libraries/formula/function/datetime/MinuteFunction.java	2015-07-13 12:28:54.359003561 +0200
--------------------------
No file to patch.  Skipping patch.
1 out of 1 hunk ignored
Patch FAILED: /home/julien/compile-libreoffice/libreoffice/external/jfreereport/patches/tdf92693_fix.patch
/home/julien/compile-libreoffice/libreoffice/solenv/gbuild/UnpackedTarball.mk:166: recipe for target '/home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/jfreereport_libformula.done' failed
make[1]: *** [/home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/jfreereport_libformula.done] Error 1
make[1]: *** Attente des tâches non terminées....
Makefile:104: recipe for target 'jfreereport.build' failed
make: *** [jfreereport.build] Error 2
Comment 13 Julien Nabet 2015-07-13 13:05:53 UTC
Lionel: just for information, I created a pull request, see https://github.com/pentaho/pentaho-reporting/pull/637
Comment 14 Lionel Elie Mamane 2015-07-13 13:13:43 UTC
(In reply to Julien Nabet from comment #12)
> I renamed the patch tdf92693_fix.patch,

> then "make jfreereport.clean && make jfreereport.build", it failed with this:

Try "tdf92693_fix.patch.1", according to https://wiki.documentfoundation.org/Development/Patching_External_Libraries the end of the name (when a digit) is significaant, else "3" is assumed.
Comment 15 Julien Nabet 2015-07-13 13:51:06 UTC
Same result with:
$(eval $(call gb_UnpackedTarball_add_patches,jfreereport_libformula,\
	external/jfreereport/patches/common_build.patch \
	external/jfreereport/patches/libformula-time-notz.patch \
	external/jfreereport/patches/tdf92693_fix.patch.1 \
))

julien@julienPC:~/compile-libreoffice/libreoffice/external/jfreereport$ ls -1 patches
common_build.patch
flow-engine.patch
libbase-1.1.6-deprecated.patch
libfonts-1.1.6-deprecated.patch
libformula-time-notz.patch
liblayout.patch
libloader-1.1.6-deprecated.patch
librepository-1.1.6-deprecated.patch
sac.patch
tdf92693_fix.patch.1


[build PRJ] jfreereport_libserializer
1 out of 1 hunk FAILED -- saving rejects to file source/org/pentaho/reporting/libraries/formula/function/datetime/MinuteFunction.java.rej
Patch FAILED: /home/julien/compile-libreoffice/libreoffice/external/jfreereport/patches/tdf92693_fix.patch.1
/home/julien/compile-libreoffice/libreoffice/solenv/gbuild/UnpackedTarball.mk:166: recipe for target '/home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/jfreereport_libformula.done' failed
make[1]: *** [/home/julien/compile-libreoffice/libreoffice/workdir/UnpackedTarball/jfreereport_libformula.done] Error 1
make[1]: *** Attente des tâches non terminées....
Makefile:104: recipe for target 'jfreereport' failed
make: *** [jfreereport] Error 2
Comment 16 Julien Nabet 2015-07-13 13:59:53 UTC
I tried again your process, I could check your fix is ok.
I genereted a patch, I'm testing right now.
Comment 17 Julien Nabet 2015-07-13 14:04:21 UTC
At least! :-)
https://gerrit.libreoffice.org/#/c/17012/
Comment 18 Julien Nabet 2015-07-13 14:12:48 UTC
Sorry, I meant "at last!" not "at least!"
Comment 19 Commit Notification 2015-07-13 15:44:21 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=429280541ff1fbbbf3f0482211a659c96c3347d2

tdf#92693: ReportBuilder: MINUTES() rounded to nearest for fractional minutes

It will be available in 5.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 20 Commit Notification 2015-07-13 19:54:49 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-5-0":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=379046962f055d26484796690c3dbc491e23b0a5&h=libreoffice-5-0

tdf#92693: ReportBuilder: MINUTES() rounded to nearest for fractional minutes

It will be available in 5.0.1.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 21 Julien Nabet 2015-07-14 14:13:46 UTC
Lionel: just for info, I gave up the pull request, they required additional things.
Comment 22 Julien Nabet 2015-07-14 19:12:45 UTC
Let's put this one to FIXED.

If someone wants to cherry-pick in 4.4 branch for 4.4.6, don't hesitate! :-)
Comment 23 Robert Großkopf 2015-07-14 20:05:20 UTC
Have tested it with 
Version: 5.0.1.0.0+
Build ID: b0153639c17d40061480a7bbde11fa0249e3051f
TinderBox: Linux-rpm_deb-x86_64@46-TDF, Branch:libreoffice-5-0, Time: 2015-07-14_03:50:43
Locale: de-DE (de_DE.UTF-8)

Well done. Seems it works right with MINUTE.

Should I open another bugreport for SECOND?
Try something like
23:43:31
12.07.2015 09:40:45
Both seem to lose 1 second.
Comment 24 Commit Notification 2015-07-28 15:32:53 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2bf98bfc8e088a102f883039219816aef148867a&h=libreoffice-4-4

tdf#92693: ReportBuilder: MINUTES() rounded to nearest for fractional minutes

It will be available in 4.4.6.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.