Bug 106388 - [patch] poppler-0.64.0 fails to compile when nss is present: ‘PRBool’ has not been declared
Summary: [patch] poppler-0.64.0 fails to compile when nss is present: ‘PRBool’ has not...
Status: RESOLVED MOVED
Alias: None
Product: poppler
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-03 18:14 UTC by William Bader
Modified: 2018-08-21 11:19 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch to fix the compiler errors (1008 bytes, patch)
2018-05-03 18:14 UTC, William Bader
Details | Splinter Review

Description William Bader 2018-05-03 18:14:11 UTC
Created attachment 139323 [details] [review]
patch to fix the compiler errors

This happens when compiling for x86_64 on Fedora 27 and CentOS 7.
I get compile errors like
/usr/include/nss3/hasht.h:48:29: error: ‘PRBool’ has not been declared
     void (*destroy)(void *, PRBool);
I needed to add #include <prtypes.h> to poppler/SignatureInfo.cc, qt5/src/poppler-form.cc, and utils/pdfsig.cc.
I attached a patch.
Comment 1 Albert Astals Cid 2018-05-04 13:17:00 UTC
Isn't that a bug in nss?

If hasht.h needs PRBool it should include prtypes.h itself.

It actually does here

./hasht.h:8:#include "prtypes.h"

Which nss version are you running?

I see how fixing it here is easier for you than getting nss fixed in fedora, but I'm not convinced we should be work-arounding when third party libraries break themselves
Comment 2 William Bader 2018-05-04 14:56:59 UTC
Thanks for looking at it. It looks like RedHat patches the source to remove the include.

My Fedora 27 laptop has nss-util-devel-3.36.1-1.0.fc27.x86_64
/usr/include/nss3/hasht.h does not have any #includes.
I think I didn't have problems before because I only recently installed nss-util-devel in order to build another application.

I checked older CentOS 7 and 6 systems. They do not have #includes in hasht.h either. The packages are nss-util-devel-3.28.4-3.el7.x86_64 and nss-util-devel-3.28.4-1.el6_9.x86_64

prtypes.h is in /usr/include/nspr4/prtypes.h in nspr-devel-4.19.0-1.fc27.x86_64, nspr-devel-4.13.1-1.0.el7_3.x86_64, and nspr-devel-4.13.1-1.el6.x86_64

The nss github source mirror has an include. https://github.com/nss-dev/nss/blob/master/lib/util/hasht.h
The include was added five years ago https://github.com/nss-dev/nss/blame/master/lib/util/hasht.h in https://github.com/nss-dev/nss/commit/e4f3c454aca2c338191e921989e1617f182ba8fb

RedHat removes the include.
On Fedora 27, I downloaded the source rpm with dnf download --source nss-util-devel
and unpacked it with rpm2cpio ../nss-util-3.36.1-1.0.fc27.src.rpm | cpio -idmv --no-absolute-filenames
It has a patch file hasht-dont-include-prtypes.patch that removes the include.
The comments in nss-util.spec say
-----
# Local patches
Patch2: hasht-dont-include-prtypes.patch
Patch3: pkcs1sig-include-prtypes.patch
# TODO: investigate whether this patch should also be applied to
# nss-softokn and nss and whether it should be submitted upstream.
# First ensure that it won't cause any FIPS tests breakage.
Patch4: nss-util-3.19.3-ldflags.patch
# To revert the change in:
# https://bugzilla.mozilla.org/show_bug.cgi?id=1377940
Patch5: nss-util-sql-default.patch
-----
hasht-dont-include-prtypes.patch removes #include "prtypes.h" from nss/lib/util/hasht.h
pkcs1sig-include-prtypes.patch adds #include "prtypes.h" to nss/lib/util/pkcs1sig.c
nss-util-3.19.3-ldflags.patch changes nss-util-3.29.0/nss/coreconf/Linux.mk to add -Wl,-z,now to DSO_LDOPTS
nss-util-sql-default.patch changes lib/util/utilpars.c to make dbType = NSS_DB_TYPE_SQL the default even if NSS_DISABLE_DBM is not set.


Regards,
William
Comment 3 Albert Astals Cid 2018-05-06 15:18:35 UTC
Can you bring up with the fedora people why they are removing that include? It doesn't make any sense
Comment 4 William Bader 2018-05-08 03:04:24 UTC
I looked around on RedHat's bugzilla.
Removing the include for prtypes.h five years ago was an incorrect attempt at a fix for a glibc build error.
About four years ago, someone filed a bug report about the incorrect removal of the include. One of the nearly two dozen comments even notes that the bug breaks poppler.
Supposedly the bug was fixed in September 2017, but the fix was not included in Fedora 26 or 27.
So this is not a poppler bug. It is a RedHat bug and should hopefully be fixed in Fedora 28 and RHEL 7.5 or 8.
I have some links below.
Regards,
William

https://bugzilla.redhat.com/show_bug.cgi?id=1113172
Bug 1113172 - hasht.h really should include prtypes.h
2014-06-25 10:33 EDT by Andrew Schultz

https://bugzilla.redhat.com/show_bug.cgi?id=1113172#c14
Tomas Mraz 2017-07-11 10:28:31 EDT
Yes, actually 2 downstream patches from the rpm should be dropped:
Patch2: hasht-dont-include-prtypes.patch
Patch3: pkcs1sig-include-prtypes.patch  
Both of these are an incorrect fix for the original issue (bug 953277 - rawhide build of glibc fails due to fatal error from nss3/hasht.h)
The glibc build needs to learn to add -I/usr/include/nspr4 to CFLAGS instead.
This is little bit complicated by the thing that nss-config --includedir cannot return two directories.
Thus glibc build must call also nspr-config --includedir. On the other hand nss-config --cflags could be fixed to properly return -I/usr/include/nss3 -I/usr/include/nspr4 and if glibc used it instead of nss-config --includedir, it would work as well.

https://bugzilla.redhat.com/show_bug.cgi?id=1113172#c15
David Tardon 2017-09-04 09:17:29 EDT
This breaks build of poppler 0.59.0 now. TBH, I find it hard to believe that such thing could be left rotting for 2 years (and even closed as NOTABUG at one point!)

https://bugzilla.redhat.com/show_bug.cgi?id=1113172#c19
Andrew Schultz 2017-11-16 14:18:41 EST
This was fixed in rawhide back in September (comment 16) with version 3.32.0-3, but is not included with f26 or f27 builds.

https://bugzilla.redhat.com/show_bug.cgi?id=953277
Bug 953277 - rawhide build of glibc fails due to fatal error from nss3/hasht.h
2013-04-17 15:57 EDT by Patsy Franklin

https://bugzilla.redhat.com/show_bug.cgi?id=1489339
Bug 1489339 - glibc: don't rely on downstream modification of hasht.h
2017-09-07 05:01 EDT by Daiki Ueno
Comment 5 Albert Astals Cid 2018-05-17 22:52:42 UTC
Rex is this something you could help with? Given you have some more fedora-knowledge?
Comment 6 Rex Dieter 2018-05-18 02:11:02 UTC
Sure, I can help fix fedora packaging bugs not resolved yet, follow progress starting at:
https://bugzilla.redhat.com/show_bug.cgi?id=1113172#c21

Though there's a chance we may have to live with the status quo wrt f26/f27 as adjusting this may break glibc building (I'll see how far I get).

Either way, it's clear (to me) this isn't an upstream poppler bug, more a downstream issue.
Comment 7 GitLab Migration User 2018-08-21 11:19:14 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/poppler/poppler/issues/618.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.