[go: up one dir, main page]

Bug 209945 - devel/jsoncpp: Does not build on 10.3 due to C++11 assumption on ppc64
Summary: devel/jsoncpp: Does not build on 10.3 due to C++11 assumption on ppc64
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: powerpc Any
: --- Affects Only Me
Assignee: Bernard Spil
URL:
Keywords: easy, patch, patch-ready
Depends on:
Blocks:
 
Reported: 2016-06-01 19:50 UTC by Mathieu Simon
Modified: 2016-09-11 08:59 UTC (History)
2 users (show)

See Also:
yuri: maintainer-feedback+


Attachments
Attemp to fix issues with older GCC version and C++11 enablement (936 bytes, patch)
2016-06-07 12:35 UTC, Mathieu Simon
no flags Details | Diff
Patch with 1.7.3 included (2.98 KB, patch)
2016-07-04 22:19 UTC, Yuri Victorovich
yuri: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mathieu Simon 2016-06-01 19:50:17 UTC
Dear maintainer

I do agree that this bug report is a bit odd - who would seriously run
Comment 1 Mathieu Simon 2016-06-01 19:59:31 UTC
Dear maintainer

I sent the message too early so sorry for the confusion.

It sounds odd but I was having some fun on some PPC64 hardware (iMac G5) where I wanted to build X11 which at some odd moment requires jsoncpp and fails currently.

I think the option it doesn't like is that it tests for the FreeBSD version and if it finds that it is on 10.x or later it assumes we have a modern compiler in the base system. 

In many and most common cases that is absolutely true, but PPC(64) doesn't ship with LLVM/clang in the base system but some older GCC 4.2.1 which doesn't support C++11. Same applies to other exotic architecture like SPARC.

I don't know what the best idea would be to work around this issue:
- Have a check that if PPC or PPC64 is found that no C++11 flag is set
  (then again all other architectures like ... sparc ... for what I know maybe
  others too)  would have the same issue.
- Have a check for the compiler at some point and see if it supports C++11
- Or if possible check for the compiler - if it's too old/certain architecture, depend on a modern compiler from ports instead.

I think the first option would be easiest for what I can tell but it's not the cleanest. (Yes it is foolish to run X.org on PPC64 hardware these days but I wanted to see how far I could get)

Regards
Mathieu
Comment 2 Yuri Victorovich freebsd_committer freebsd_triage 2016-06-01 20:34:25 UTC
COuld you suggest a patch that fixes it on PPC, and I will take it from there?
Comment 3 Mathieu Simon 2016-06-02 05:58:40 UTC
Hi Vladimir

I'll do that mainly by looking if similar compiler workarounds exist in other ports for more exotic architectures too.

I've been able to build jsoncpp with GCC 4.2.1 by quickly commenting out the section on the makefile assuming C++11 compat on 10.x which let the build pass. 

That's not a fix but it's to confirm that without C++11 support jsoncpp still builds with older GCC version.

I'll ping you as soon as I have something I can share for review.

-- Mathieu
Comment 4 Mathieu Simon 2016-06-07 12:35:05 UTC
Created attachment 171140 [details]
Attemp to fix issues with older GCC version and C++11 enablement

Hi Vladimir

I think I may have something, it seems to compile on:
- 10.3-amd64 (clang)   -> Builds, with C++11    (expected, no regression)
- 9.3-amd64  (GCC 4.2) -> Builds, without C++11 (expected, no regression)
- 10.3-ppc64 (GCC 4.2) -> Builds, without C++11 (expected, now fixed)

With this patch the port checks if the compiler is recent enough, not the OS type or OS version. This would have the added advantage of (quite likely) allowing DragonFly folks to compile lang/jsoncpp with C++11 support too since they ship newer GCC versions in their base and use the FreeBSD ports tree.

The idea is based on what Mk/Uses/compiler.mk does and what I found in compiler checks such as in x11/eaglemode/Makefile. 

I agree that this patch somewhat duplicates the efforts of compiler.mk. However with this we still allow building on non-C++11 compilers as currently.

The versions required for C++11 support of both clang and GCC support was based on for C++11 implementation status sites of each project:
- clang: http://clang.llvm.org/cxx_status.html
- GCC: https://gcc.gnu.org/projects/cxx-status.html#cxx11

Looking forward to your feedback.

-- Mathieu
Comment 5 Yuri Victorovich freebsd_committer freebsd_triage 2016-06-07 12:40:46 UTC
Ok, I approve the patch if committer doesn't see any regressions.
Comment 6 VK 2016-06-07 13:00:58 UTC
Comment on attachment 171140 [details]
Attemp to fix issues with older GCC version and C++11 enablement

Yuri, just this minor thing, and it's good to go. ;)
Comment 7 Mathieu Simon 2016-06-12 09:15:12 UTC
Hi Vladimir and Yuri

If I'm right Yuri is currently the maintainer and has given its OK for commit. However I'm not certain I'm getting what what Vladimir last wrote:

> Yuri, just this minor thing, and it's good to go. ;)

If I should rework the patch, let me know no issues if that needs to happen first ;-)

Thank your for your feedbacks so far.

-- Mathieu
Comment 8 Yuri Victorovich freebsd_committer freebsd_triage 2016-06-12 09:21:37 UTC
Vladimir should clarify. As far as I am concerned it can be committed if all required builds pass.
Comment 9 VK 2016-06-12 11:25:19 UTC
I apologize if I wasn't clear. I've set the maintainer-approval? request flag on the patch and meant Yuri should just mark it approved as that's the signal the maintainer is approving it, and the PR appears in the "Maintainer approved" saved search.

I generally avoid setting maintainer-approval+ on patches on behalf of maintainers because it gets mistaken and I get credited in the commit logs. :)
Comment 10 Yuri Victorovich freebsd_committer freebsd_triage 2016-06-12 11:43:37 UTC
Comment on attachment 171140 [details]
Attemp to fix issues with older GCC version and C++11 enablement

Ok!
Comment 11 Bernard Spil freebsd_committer freebsd_triage 2016-07-03 18:35:17 UTC
Tested on 9.3 and 10.3 both i386 and amd64 with poudriere testport OK
Comment 12 Yuri Victorovich freebsd_committer freebsd_triage 2016-07-04 22:19:06 UTC
Created attachment 172120 [details]
Patch with 1.7.3 included

I integrated 1.7.3 into the previous patch.
Comment 13 commit-hook freebsd_committer freebsd_triage 2016-07-05 06:45:07 UTC
A commit references this bug:

Author: brnrd
Date: Tue Jul  5 06:44:21 UTC 2016
New revision: 418065
URL: https://svnweb.freebsd.org/changeset/ports/418065

Log:
  devel/jsoncpp: Update to 1.7.3

    - Update to version 1.7.3 [2]
    - Fix compiler issues [1]

  PR:		209945
  Submitted by:	Mathieu Simon <freebsd@simweb.ch> [1]
  Reviewed by:	yuri@rawbw.com (maintainer) [2]

Changes:
  head/devel/jsoncpp/Makefile
  head/devel/jsoncpp/distinfo
  head/devel/jsoncpp/files/patch-include_json_config.h
  head/devel/jsoncpp/pkg-plist
Comment 14 VK 2016-07-09 00:18:56 UTC
Assigning to committer who committed.
Comment 15 commit-hook freebsd_committer freebsd_triage 2016-09-11 08:59:25 UTC
A commit references this bug:

Author: brnrd
Date: Sun Sep 11 08:59:05 UTC 2016
New revision: 421808
URL: https://svnweb.freebsd.org/changeset/ports/421808

Log:
  MFH: r417988 r418065

  devel/jsoncpp: Fix build on 10.3

    - Change OS version checks to compiler checks

  PR:		209954
  Submitted by:	Mathieu Simon <freebsd@simweb.ch>

  devel/jsoncpp: Update to 1.7.3

    - Update to version 1.7.3 [2]
    - Fix compiler issues [1]

  PR:		209945
  Submitted by:	Mathieu Simon <freebsd@simweb.ch> [1]
  Reviewed by:	yuri@rawbw.com (maintainer) [2]

  Approved by:	ports-secteam (junovitch)

Changes:
_U  branches/2016Q3/
  branches/2016Q3/devel/jsoncpp/Makefile
  branches/2016Q3/devel/jsoncpp/distinfo
  branches/2016Q3/devel/jsoncpp/files/patch-include_json_config.h
  branches/2016Q3/devel/jsoncpp/pkg-plist