ScFiltersTest::testOrcusODSStyleInterface fails with liborcus 0.13.3

classic Classic list List threaded Threaded
5 messages Options
Olivier Tilloy Olivier Tilloy
Reply | Threaded
Open this post in threaded view
|

ScFiltersTest::testOrcusODSStyleInterface fails with liborcus 0.13.3

Hello,

I initially filed
https://bugs.documentfoundation.org/show_bug.cgi?id=115931 but it was
suggested to me to raise the issue on the mailing list instead.

So here it goes: when building libreoffice 6.0 with an external
liborcus (version 0.13.3), unit tests fail:

subsequent_filters-test.cxx:2398:Assertion
Test name: ScFiltersTest::testOrcusODSStyleInterface
equality assertion failed
- Expected: Color: R:254 G:255 B: 204
- Actual : Color: R:255 G:255 B: 255

This is most likely caused by that upstream change:
https://gitlab.com/orcus/orcus/commit/f821995022df8dd1e580dd22cf131584b2b1ac4f

I'm not familiar with that code, but I came up with a tentative patch
(which I will actually put to the test soon):

--- a/sc/source/filter/orcus/interface.cxx
+++ b/sc/source/filter/orcus/interface.cxx
@@ -839,7 +839,7 @@ void ScOrcusStyles::fill::applyToItemSet
         return;
     }

-    rSet.Put(SvxBrushItem(maBgColor, ATTR_BACKGROUND));
+    rSet.Put(SvxBrushItem(maFgColor, ATTR_BACKGROUND));
 }

Thoughts and feedback welcome!

Regards,

 Olivier
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Kohei Yoshida-6 Kohei Yoshida-6
Reply | Threaded
Open this post in threaded view
|

Re: ScFiltersTest::testOrcusODSStyleInterface fails with liborcus 0.13.3

Hi there,

> On February 22, 2018 at 8:01 AM Olivier Tilloy <[hidden email]> wrote:
>
>
> Hello,
>
> I initially filed
> https://bugs.documentfoundation.org/show_bug.cgi?id=115931 but it was
> suggested to me to raise the issue on the mailing list instead.
>
> So here it goes: when building libreoffice 6.0 with an external
> liborcus (version 0.13.3), unit tests fail:
>
> subsequent_filters-test.cxx:2398:Assertion
> Test name: ScFiltersTest::testOrcusODSStyleInterface
> equality assertion failed
> - Expected: Color: R:254 G:255 B: 204
> - Actual : Color: R:255 G:255 B: 255
>
> This is most likely caused by that upstream change:
> https://gitlab.com/orcus/orcus/commit/f821995022df8dd1e580dd22cf131584b2b1ac4f
>
> I'm not familiar with that code, but I came up with a tentative patch
> (which I will actually put to the test soon):
>
> --- a/sc/source/filter/orcus/interface.cxx
> +++ b/sc/source/filter/orcus/interface.cxx
> @@ -839,7 +839,7 @@ void ScOrcusStyles::fill::applyToItemSet
>          return;
>      }
>
> -    rSet.Put(SvxBrushItem(maBgColor, ATTR_BACKGROUND));
> +    rSet.Put(SvxBrushItem(maFgColor, ATTR_BACKGROUND));
>  }
>
> Thoughts and feedback welcome!

Yes, so, you have raised this in a timely fashion.  While I was backporting a patch against orcus for the 6-0 branch last night, I did notice the possibility of this very issue, and was thinking of how to address it for that branch.  Since you've already discovered this and emailed it to the list, let me provide my thoughts on this.

First of all, your patch should work fine.  However, with this change, we need to ensure that liborcus version 0.13.3 or later be used.  If you used an earlier version this change does break things.

What I was thinking of proposing is backporting the following commits:

20945a9a4de6684010fd5b3603595e6da543807d
a1c36eff089c3cd695bd78090575ca1c7677121e

raise the orcus baseline to 0.13.3, and commit all of these as a single backport commit to ensure that these all happen atomically.  This way if someone wants to build from the 6-0 branch using the system liborcus then the configure script should be able to catch it and fail.

How does this sound?

Kohei
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Olivier Tilloy Olivier Tilloy
Reply | Threaded
Open this post in threaded view
|

Re: ScFiltersTest::testOrcusODSStyleInterface fails with liborcus 0.13.3

On Thu, Feb 22, 2018 at 2:21 PM, Kohei Yoshida <[hidden email]> wrote:

> Hi there,
>
>> On February 22, 2018 at 8:01 AM Olivier Tilloy <[hidden email]> wrote:
>>
>>
>> Hello,
>>
>> I initially filed
>> https://bugs.documentfoundation.org/show_bug.cgi?id=115931 but it was
>> suggested to me to raise the issue on the mailing list instead.
>>
>> So here it goes: when building libreoffice 6.0 with an external
>> liborcus (version 0.13.3), unit tests fail:
>>
>> subsequent_filters-test.cxx:2398:Assertion
>> Test name: ScFiltersTest::testOrcusODSStyleInterface
>> equality assertion failed
>> - Expected: Color: R:254 G:255 B: 204
>> - Actual : Color: R:255 G:255 B: 255
>>
>> This is most likely caused by that upstream change:
>> https://gitlab.com/orcus/orcus/commit/f821995022df8dd1e580dd22cf131584b2b1ac4f
>>
>> I'm not familiar with that code, but I came up with a tentative patch
>> (which I will actually put to the test soon):
>>
>> --- a/sc/source/filter/orcus/interface.cxx
>> +++ b/sc/source/filter/orcus/interface.cxx
>> @@ -839,7 +839,7 @@ void ScOrcusStyles::fill::applyToItemSet
>>          return;
>>      }
>>
>> -    rSet.Put(SvxBrushItem(maBgColor, ATTR_BACKGROUND));
>> +    rSet.Put(SvxBrushItem(maFgColor, ATTR_BACKGROUND));
>>  }
>>
>> Thoughts and feedback welcome!
>
> Yes, so, you have raised this in a timely fashion.  While I was backporting a patch against orcus for the 6-0 branch last night, I did notice the possibility of this very issue, and was thinking of how to address it for that branch.  Since you've already discovered this and emailed it to the list, let me provide my thoughts on this.
>
> First of all, your patch should work fine.  However, with this change, we need to ensure that liborcus version 0.13.3 or later be used.  If you used an earlier version this change does break things.
>
> What I was thinking of proposing is backporting the following commits:
>
> 20945a9a4de6684010fd5b3603595e6da543807d
> a1c36eff089c3cd695bd78090575ca1c7677121e
>
> raise the orcus baseline to 0.13.3, and commit all of these as a single backport commit to ensure that these all happen atomically.  This way if someone wants to build from the 6-0 branch using the system liborcus then the configure script should be able to catch it and fail.
>
> How does this sound?

That sounds correct to me.

I'm building packages for ubuntu 18.04 where liborcus is at version
0.13.3 in the archive already, which is why I've started hitting the
issue. I'll test the change I suggested earlier as a distro-patch, and
it can be dropped later when your changes are merged and released.

Regards,

 Olivier
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Rene Engelhard Rene Engelhard
Reply | Threaded
Open this post in threaded view
|

Re: ScFiltersTest::testOrcusODSStyleInterface fails with liborcus 0.13.3

In reply to this post by Kohei Yoshida-6
Hi,

On Thu, Feb 22, 2018 at 08:21:40AM -0500, Kohei Yoshida wrote:

> > I initially filed
> > https://bugs.documentfoundation.org/show_bug.cgi?id=115931 but it was
> > suggested to me to raise the issue on the mailing list instead.
> >
> > So here it goes: when building libreoffice 6.0 with an external
> > liborcus (version 0.13.3), unit tests fail:
> >
> > subsequent_filters-test.cxx:2398:Assertion
> > Test name: ScFiltersTest::testOrcusODSStyleInterface
> > equality assertion failed
> > - Expected: Color: R:254 G:255 B: 204
> > - Actual : Color: R:255 G:255 B: 255
> >
> > This is most likely caused by that upstream change:
> > https://gitlab.com/orcus/orcus/commit/f821995022df8dd1e580dd22cf131584b2b1ac4f
[...]

> Yes, so, you have raised this in a timely fashion.  While I was backporting a patch against orcus for the 6-0 branch last night, I did notice the possibility of this very issue, and was thinking of how to address it for that branch.  Since you've already discovered this and emailed it to the list, let me provide my thoughts on this.
>
> First of all, your patch should work fine.  However, with this change, we need to ensure that liborcus version 0.13.3 or later be used.  If you used an earlier version this change does break things.
>
> What I was thinking of proposing is backporting the following commits:
>
> 20945a9a4de6684010fd5b3603595e6da543807d
> a1c36eff089c3cd695bd78090575ca1c7677121e
>
> raise the orcus baseline to 0.13.3, and commit all of these as a single backport commit to ensure that these all happen atomically.  This way if someone wants to build from the 6-0 branch using the system liborcus then the configure script should be able to catch it and fail.
>
> How does this sound?

I think that should be done. I uploaded 0.13.3 to Debian (and Ubuntu
synced it) for 6.1 needing it (and bumped the build and runtime
dependency there already..) but when 6.0 breaks with it (should have
known better and tried...) that imho should be done, yes...

Will backport both patches above as a local patch for now.

Regards,

Rene
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Kohei Yoshida-6 Kohei Yoshida-6
Reply | Threaded
Open this post in threaded view
|

Re: ScFiltersTest::testOrcusODSStyleInterface fails with liborcus 0.13.3

On Thu, 2018-02-22 at 15:14 +0100, Rene Engelhard wrote:
>
> I think that should be done.

Here we go.

https://gerrit.libreoffice.org/#/c/50213/

Hopefully the Jenkins builds will be all green.

Thanks,

Kohei
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice