Bug 699395

Summary:	Build of lcms2art on big endian fails
Product:	MuPDF	Reporter:	M.J.G. <mjg>
Component:	mupdf	Assignee:	Robin Watts <robin.watts>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	P4
Version:	master
Hardware:	PC
OS:	Linux
Customer:		Word Size:	---
Attachments:	Patch Big endian FTBFS fix for lcms2 2.10art

Description M.J.G. 2018-06-03 18:01:02 UTC

Created attachment 15217 [details]
Patch

This is a bug in Artifex' patched copy of lcms2.

0dc1153 ("Spread of context into all procedures and removal from
structures", 2017-04-26) missed a few spots that are relevant on big
endian only.

Add the missing ContextIDs in the call chain so that the build succeeds
again.

Comment 1 M.J.G. 2020-10-04 13:16:20 UTC

Created attachment 19916 [details]
Big endian FTBFS fix for lcms2 2.10art

Here is an updated patch to make your fork of lcms2 build from source on big endian architectures.

See e.g.:
https://koji.fedoraproject.org/koji/taskinfo?taskID=52714864

The issue is still the same (passing the context down for thread safety; on BigEndian byteReverse is not a no-op ...), the changed patch simply adapts to a name change (compared to the previous patch).

Comment 2 Ray Johnston 2020-10-04 17:56:41 UTC

Not sure if this should be Michael or someone else, but assigning it to him so
he can hunt for someone else to pass it to.

Comment 3 Michael Vrhel 2020-10-28 19:37:06 UTC

I think this falls under the area of Robin and or Chris.  I will push it to Robin.

Comment 4 Robin Watts 2020-10-29 10:27:52 UTC

(In reply to M.J.G. from comment #1)
> The issue is still the same (passing the context down for thread safety; on
> BigEndian byteReverse is not a no-op ...), the changed patch simply adapts
> to a name change (compared to the previous patch).

Regardless of endianness byteReverse doesn't need a ContextID. So... what's the point of passing it there?

Comment 5 Robin Watts 2020-10-29 10:39:42 UTC

We know this builds/runs on sparc, which is big endian, so I think we need to know more. Like some errors from the build, please?

Comment 6 M.J.G. 2020-10-29 19:26:50 UTC

(In reply to Robin Watts from comment #4)
> (In reply to M.J.G. from comment #1)
> > The issue is still the same (passing the context down for thread safety; on
> > BigEndian byteReverse is not a no-op ...), the changed patch simply adapts
> > to a name change (compared to the previous patch).
> 
> Regardless of endianness byteReverse doesn't need a ContextID. So... what's
> the point of passing it there?

The mentinoned commit 0dc1153 introduced the context argument to many functions, in particular it changed the call signature of _cmsAdjustEndianess32() and most call sites calling that function. It did not change the call within byteReverse(), and this mattered only if CMS_USE_BIG_ENDIAN is defined because otherwise byteReverse() was defined to be a noop. That's what necessitated my original patch - because otherwise mupdf back then did not build on big endian.

c9f2256 ("Remove ContextID from _cmsAdjustEndianess calls.", 2018-10-13) removed the ContextID argument from those calls. When that commit landed in the lcms2mt which mupdf bundles, mupdf with that lcms2mt and my first patch did not build any more. Since I overlooked c9f2256 at that time I simply adjusted my patch, thus passing down ContextID unnecessarily in that callchain.

I just checked: the mupdf package which I maintain in Fedora builds without the patch on all architectures now.

So, while this was a bug back when reported it has been fixed through c9f2256 and can be closed now.

Comment 7 Robin Watts 2020-10-30 15:23:00 UTC

Thanks for letting us know!