Bug 246720 - math/saga: fix build on GCC architectures
Summary: math/saga: fix build on GCC architectures
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Piotr Kubaj
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-25 11:19 UTC by Piotr Kubaj
Modified: 2020-06-15 15:33 UTC (History)
2 users (show)

See Also:
bugzilla: maintainer-feedback? (rhurlin)


Attachments
patch (1.04 KB, patch)
2020-05-25 11:19 UTC, Piotr Kubaj
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Kubaj freebsd_committer 2020-05-25 11:19:19 UTC
Created attachment 214841 [details]
patch

Returning NULL as bool is not ok.
--- globe_gores.lo ---
globe_gores.cpp: In member function 'bool CGlobe_Gores::Add_Gore(int, int)':
globe_gores.cpp:210:16: error: converting to 'bool' from 'std::nullptr_t' requires direct-initialization [-fpermissive]
  210 |   return( NULL );
Comment 1 Fernando Apesteguía freebsd_committer 2020-05-25 11:30:38 UTC
^Triage: Reporter is committer, assign accordingly.
Comment 2 Rainer Hurling 2020-05-25 15:39:55 UTC
Hi Piotr,

Thanks for the report and patch. I was not aware about this problem with gcc, most platforms seems to have clang now?

As a first test on my side I tried a build with your patch and clang, which seems to work. Some more builds with clang and gcc on Poudriere should follow, then some functional tests, so it needs some more time.

-----------------------------
One problem with using gcc I am not sure about is:
If option OPENMP is enabled, the port uses libomp instead of libgomp (patched via files/patch-configure.ac and via Makefile). Probably it would be better to rework the patch for this case to use libomp for clang and to stay with libgomp for gcc ...
-----------------------------

As you are the first guy reporting such a problem, what is your reason to build with gcc?

Thanks again,
Rainer
Comment 3 Piotr Kubaj freebsd_committer 2020-05-25 15:58:12 UTC
All powerpc architectures on 11 and 12 use GCC. They switched to Clang only in head.

I can see that OPENMP is enabled only on amd64 and i386.
Comment 4 Rainer Hurling 2020-06-01 09:27:01 UTC
(In reply to Piotr Kubaj from comment #3)
Hi Piotr,

Sorry for the delay. I had been on a business trip.

It turns out that I have no possibility to test anything for powerpc, because here on my side there are only amd64 boxes around. Did you tried the port with USE_GCC=yes?


As I asked before in comment #2: what is your reason to build with gcc? Is it just out of curiosity or is it to get more ports working on powerpc?

If the second is true, we could mark math/saga broken for powerpc on 11 and 12, until we get an acceptable solution with gcc. What do you think about it?


[1] https://wiki.freebsd.org/powerpc/ports
Comment 5 Piotr Kubaj freebsd_committer 2020-06-01 11:07:42 UTC
(In reply to Rainer Hurling from comment #4)

>It turns out that I have no possibility to test anything for powerpc, because here on my side there are only amd64 boxes around. Did you tried the port with USE_GCC=yes?

USE_GCC=yes won't do anything, USES=compiler:c++11-lib already sets the compiler to GCC.

It's GCC9 that has this issue.

>As I asked before in comment #2: what is your reason to build with gcc? Is it just out of curiosity or is it to get more ports working on powerpc?
I'm building with GCC, because it's the only compiler available on stable/11 and stable/12 for powerpc. Note that this port already built on powerpc64 previously so this is a regression: http://pylon.nyi.freebsd.org/data/latest-per-pkg/saga/7.5.0_3/121powerpc64-quarterly.log

>If the second is true, we could mark math/saga broken for powerpc on 11 and 12, until we get an acceptable solution with gcc. What do you think about it?
Well, I already attached to this bug a patch that I think is acceptable.
Comment 6 Rainer Hurling 2020-06-01 15:01:48 UTC
Comment on attachment 214841 [details]
patch

(In reply to Piotr Kubaj from comment #5)

> USE_GCC=yes won't do anything, USES=compiler:c++11-lib already sets the compiler to GCC.
Oops, sorry, USE_GCC=yes slipt in because of some tests, I tried on amd64 and i386. Of course, USES=compiler:c++11-lib is sufficient here.


> Note that this port already built on powerpc64 previously so this is a regression: http://pylon.nyi.freebsd.org/data/latest-per-pkg/saga/7.5.0_3/121powerpc64-quarterly.log
Because of very big changes in PROJ4 (graphics/proj) SAGA GIS has to rework a lot of its proj4 code and introduces some newer techniques like the added globe cores tool (from 2020-02-17 on).

It's interesting, why no other gcc based OSses complains about this 'return( NULL )'. Perhaps of the very old gcc version, used on FreeBSD by powerpc?


> Well, I already attached to this bug a patch that I think is acceptable.
I can confirm, that your patch also builds for amd64/i386, on real life boxes and with Poudriere.


I would be fine, if you could commit this patch. Many thanks again for your work.
Comment 7 Rainer Hurling 2020-06-15 05:04:37 UTC
Hi  Piotr,

Just as a reminder. Is there any reason to not commit this patch?

Best wishes,
Rainer
Comment 8 Piotr Kubaj freebsd_committer 2020-06-15 09:15:52 UTC
Ah, I was busy with other things, sorry!
Comment 9 commit-hook freebsd_committer 2020-06-15 09:17:51 UTC
A commit references this bug:

Author: pkubaj
Date: Mon Jun 15 09:16:49 UTC 2020
New revision: 538865
URL: https://svnweb.freebsd.org/changeset/ports/538865

Log:
  math/saga: fix build on GCC architectures

  Returning NULL as bool is not ok.
  --- globe_gores.lo ---
  globe_gores.cpp: In member function 'bool CGlobe_Gores::Add_Gore(int, int)':
  globe_gores.cpp:210:16: error: converting to 'bool' from 'std::nullptr_t' requires direct-initialization [-fpermissive]
    210 |   return( NULL );

  PR:		246720
  Approved by:	rhurlin@gwdg.de (maintainer)

Changes:
  head/math/saga/files/patch-src_tools_projection_pj__proj4_globe__gores.cpp
Comment 10 Rainer Hurling 2020-06-15 15:25:03 UTC
Hi Piotr,

Just for the record:

I told about your fix for this problem on the saga-gis-developer list and it is upstream already[1]. So the next official release should contain your patch :)


[1] https://sourceforge.net/p/saga-gis/code/ci/d0f801d3b7d18e61504a31bc4091c98de0a18c46/
Comment 11 Piotr Kubaj freebsd_committer 2020-06-15 15:33:15 UTC
(In reply to Rainer Hurling from comment #10)
Thanks!