Bug 233426 - sysutils/facter: Fails to build when devel/rapidjson is installed
Summary: sysutils/facter: Fails to build when devel/rapidjson is installed
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: FreeBSD Puppet Team
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2018-11-23 04:26 UTC by Vladyslav Movchan
Modified: 2019-01-13 03:57 UTC (History)
2 users (show)

See Also:
koobs: merge-quarterly?


Attachments
Use /usr/local/include/leatherman/vendor before /usr/local/include (412 bytes, text/plain)
2018-11-23 04:26 UTC, Vladyslav Movchan
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Vladyslav Movchan 2018-11-23 04:26:51 UTC
Created attachment 199470 [details]
Use /usr/local/include/leatherman/vendor before /usr/local/include

Build of sysutils/facter fails when devel/rapidjson is installed in the system.

Command which fails and error message:
```
cd /usr/ports/sysutils/facter/work/facter-3.12.1/lib && /usr/bin/c++  -DBOOST_ALL_DYN_LINK -DBOOST_LOG_WITHOUT_WCHAR_T -DBOOST_SYSTEM_NO_DEPRECATED -DLEATHERMAN_I18N -DLEATHERMAN_LOGGING_NAMESPACE=\"puppetlabs.facter\" -DLEATHERMAN_USE_LOCALES -DPROJECT_DIR=\"/usr/ports/sysutils/facter/work/facter-3.12.1\" -DPROJECT_NAME=\"FACTER\" -DUSE_CPPHOCON -DUSE_JRUBY_SUPPORT -DUSE_OPENSSL -DUSE_YAMLCPP -Dlibfacter_EXPORTS -I/usr/ports/sysutils/facter/work/facter-3.12.1/lib/inc -I/usr/ports/sysutils/facter/work/facter-3.12.1/../vendor/nowide/include -I/usr/local/openjdk8/include -I/usr/local/openjdk8/include/freebsd -I/usr/local/include -I/usr/local/include/leatherman/vendor -Wno-deprecated-register -std=c++11 -Wall -Wextra -Wno-unused-parameter -Wno-tautological-constant-out-of-range-compare -O2 -pipe -O2 -pipe -march=haswell -fstack-protector -fno-strict-aliasing -O2 -pipe -O2 -pipe -march=haswell -fstack-protector -fno-strict-aliasing -fPIC -o CMakeFiles/libfactersrc.dir/src/facts/resolver.cc.o -c /usr/ports/sysutils/facter/work/facter-3.12.1/lib/src/facts/resolver.cc
--- lib/CMakeFiles/libfactersrc.dir/src/facts/external/json_resolver.cc.o ---
In file included from /usr/ports/sysutils/facter/work/facter-3.12.1/lib/src/facts/external/json_resolver.cc:9:
/usr/local/include/rapidjson/reader.h:1340:32: error: no member named 'RawNumber' in 'facter::facts::external::json_event_handler'
cont = handler.RawNumber(str, SizeType(length), false);
~~~~~~~ ^
/usr/local/include/rapidjson/reader.h:1401:23: note: in instantiation of function template specialization 'rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseNumber<0, rapidjson::FileReadStream, facter::facts::external::json_event_handler>' requested here
ParseNumber<parseFlags>(is, handler);
^
/usr/local/include/rapidjson/reader.h:501:13: note: in instantiation of function template specialization 'rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::ParseValue<0, rapidjson::FileReadStream, facter::facts::external::json_event_handler>' requested here
ParseValue<parseFlags>(is, handler);
^
/usr/local/include/rapidjson/reader.h:527:16: note: in instantiation of function template specialization 'rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::Parse<0, rapidjson::FileReadStream, facter::facts::external::json_event_handler>' requested here
return Parse<kParseDefaultFlags>(is, handler);
^
/usr/ports/sysutils/facter/work/facter-3.12.1/lib/src/facts/external/json_resolver.cc:211:30: note: in instantiation of function template specialization 'rapidjson::GenericReader<rapidjson::UTF8<char>, rapidjson::UTF8<char>, rapidjson::CrtAllocator>::Parse<rapidjson::FileReadStream, facter::facts::external::json_event_handler>' requested here
auto result = reader.Parse(stream, handler);
^
1 error generated.
*** [lib/CMakeFiles/libfactersrc.dir/src/facts/external/json_resolver.cc.o] Error code 1
```

It is possible to see that -I/usr/local/include has been put before -I/usr/local/include/leatherman/vendor
As the result /usr/local/include/rapidjson/reader.h is included instead of /usr/local/include/leatherman/vendor/rapidjson/reader.h

Attached patch fixes this problem by moving -I/usr/local/include/leatherman/vendor in front of -I/usr/local/include
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2018-11-23 04:38:17 UTC
If there's a way to make user supplied *FLAGS (CFLAGS, CPPFLAGS, LDFLAGS, et al) all and always come *last*, that would be preferable over making specific includes come first/before.

Alternatively and better, under the assumption that cmake doesn't clobber user-supplied flags by default or when used properly in a standards compliant / by convention manner, then identifying and fixing whatever the facter's build system files do that clobber these flags would be ideal (and then sending it upstream), treating this issue as 'honour user-supplied flags', meaning they always come last to override the build system provided/derived values.
Comment 2 Vladyslav Movchan 2018-11-23 17:30:24 UTC
I might be wrong, but to me it looks like "-I/usr/local/include" is not part of user supplied flags. It is rather included into each of the following variables which are constructed/identified by cmake and later supplied into include_directories() command:
${Boost_INCLUDE_DIRS}
${YAMLCPP_INCLUDE_DIRS}
${CURL_INCLUDE_DIRS}
${CPPHOCON_INCLUDE_DIRS}
${OPENSSL_INCLUDE_DIRS} (optionaly)
Comment 3 Romain Tartière freebsd_committer freebsd_triage 2019-01-10 18:39:29 UTC
I got some time to look at this and I tried to reproduce the problem today, but it seems that the build succeed, rapidjson being installed or not on the system (FreeBSD 11.2, ports from few days ago).

The committed changes to the port between the PR creation and now seems unrelated, so their was either some problem in the ports framework itself that got fixed, or your local setup make something go wrong.

Can you please try again and report success or failure?

Thanks!
Comment 4 Vladyslav Movchan 2019-01-11 11:59:54 UTC
Thanks for looking into this.

I retried with fresh ports and I was able to reproduce original problem on 12.0-RELEASE-p2 and 13.0-CURRENT hosts.
To make sure this is not something related to FreeBSD base version I made a test on VM image of 11.2-RELEASE downloaded from freebsd.org and it still fails for me.

I've executed only following steps to trigger this issue on fresh VM:
portsnap fetch extract
pkg install -y rapidjson cmake openjdk8 dialog4ports ruby boost-libs leatherman cpp-hocon yaml-cpp
cd /usr/ports/sysutils/facter
make install
Comment 5 commit-hook freebsd_committer freebsd_triage 2019-01-12 06:40:24 UTC
A commit references this bug:

Author: romain
Date: Sat Jan 12 06:39:42 UTC 2019
New revision: 490027
URL: https://svnweb.freebsd.org/changeset/ports/490027

Log:
  Fix build when devel/rapidjson is installed

  devel/leatherman include an old version of RapidJSON that is not compatible
  with what devel/rapidjson install.  Reorder includes so that the version
  included with devel/leatherman is found before the one of devel/rapidjson
  because it is what is wanted.

  While here, fix `make test`.

  No need to bump PORTREVISION.

  PR:		233426
  Reported by:	vladislav.movchan@gmail.com

Changes:
  head/sysutils/facter/Makefile
  head/sysutils/facter/files/patch-lib_CMakeLists.txt
Comment 6 Romain Tartière freebsd_committer freebsd_triage 2019-01-12 06:41:27 UTC
I mixed up leatherman and facter and ending up verifying leatherman was building correctly.  Indeed it does.

So, I could reproduce the problem and see that the proposed fix seems to improve the situation ;-) After more testing, it looks like just reordering the includes does the trick, so I committed this.

Thank you for your report!
Comment 7 Vladyslav Movchan 2019-01-12 09:14:32 UTC
Thank you for your efforts
I retried from scratch with the FreeBSD 11.2 VM and fresh ports (where patch-lib_CMakeLists.txt is already present) and I still see original problem.
For some reason just reordering includes doesn't fix the problem for me.
Comment 8 Romain Tartière freebsd_committer freebsd_triage 2019-01-13 03:11:46 UTC
Reopen, more test needed
Comment 9 Romain Tartière freebsd_committer freebsd_triage 2019-01-13 03:22:39 UTC
Yeah, I can reproduce this in a clean jail.  Maybe ccache is tricking on me on my usual test environment :-/.
Comment 10 commit-hook freebsd_committer freebsd_triage 2019-01-13 03:33:47 UTC
A commit references this bug:

Author: romain
Date: Sun Jan 13 03:33:12 UTC 2019
New revision: 490112
URL: https://svnweb.freebsd.org/changeset/ports/490112

Log:
  Fix build when rapidjson is installed (again)

  PR:		233426
  Submitted by:	vladislav.movchan@gmail.com

Changes:
  head/sysutils/facter/files/patch-lib_CMakeLists.txt
Comment 11 Romain Tartière freebsd_committer freebsd_triage 2019-01-13 03:39:38 UTC
This will hopefully definitively fix this issue!

For your information, there is some traction upstream for unvendoring rapidjson from leatherman [1, 2].  I guess this would make this kind of changes useless, so would allow us to remove that patch in the future.  As a consequence, I will not try to have that change merges upstream.

References:
1. https://tickets.puppetlabs.com/browse/LTH-130
2. https://github.com/puppetlabs/leatherman/pull/265
3. https://github.com/puppetlabs/leatherman/pull/289
Comment 12 Vladyslav Movchan 2019-01-13 03:57:05 UTC
Thanks a lot Romain!