[PowerPC] SuiteSparse_CHOLMOD 7.4.0 build failure: cholmod_dbound.c: error: two or more data types in declaration specifiers (Libm already defines Real)

[barracuda156]

[ 43%] Building C object CMakeFiles/CHOLMOD.dir/Utility/cholmod_dbound.c.o
/opt/local/bin/gcc-mp-13 -DBLAS_Apple -DCHOLMOD_EXPORTS -DHAVE_KEYWORD__THREAD -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/build -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Check -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Cholesky -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Utility -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/MatrixOps -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Modify -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Partition -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Supernodal -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Include -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/SuiteSparse_metis/GKlib -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/SuiteSparse_metis/libmetis -I/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/SuiteSparse_metis/include -isystem /opt/local/include -pipe -O3 -DNDEBUG -I/opt/local/include -arch ppc -mmacosx-version-min=10.6 -fPIC -std=gnu11 -MD -MT CMakeFiles/CHOLMOD.dir/Utility/cholmod_dbound.c.o -MF CMakeFiles/CHOLMOD.dir/Utility/cholmod_dbound.c.o.d -o CMakeFiles/CHOLMOD.dir/Utility/cholmod_dbound.c.o -c /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Utility/cholmod_dbound.c
/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_macports_release_tarballs_ports_math_SuiteSparse/SuiteSparse_CHOLMOD/work/SuiteSparse-7.4.0/CHOLMOD/Utility/cholmod_dbound.c:14:33: error: two or more data types in declaration specifiers
   14 | #define Real                    double
      |                                 ^~~~~~
make[2]: *** [CMakeFiles/CHOLMOD.dir/Utility/cholmod_dbound.c.o] Error 1

[barracuda156]

@szhorvat pointed to the problem: https://trac.macports.org/ticket/69025#comment:2

If it is possible to pass -D__NOEXTENSIONS__ conditional on macOS and powerpc, this would fix the issue.

[szhorvat]

This is the same issue that I pointed out in #634 (comment)

math.h seems to use Real and Imag in some old OS X versions on some platforms. You can see this here:

https://opensource.apple.com/source/Libm/Libm-213/ppc.subproj/math.h.auto.html

As @barracuda156 says, it seems that defining __NOEXTENSIONS__ prevents the use of these non-standard symbols. __NOEXTENSIONS__ is undocumented, but it seems that defining _POSIX_C_SOURCE would also fix this. Does SuiteSparse make use of any extensions that defining these symbols would remove? If not, it would be great to include this workaround in SuiteSparse 7.5.0 directly.

@barracuda156 Do you know which specific OS X versions / platforms require the fix, to reduce the chance of breaking something on newer OS X?

[barracuda156]

It is there from archaic versions (Libm-40.2) through 10.6.8 (Libm-315), which is the last to support PowerPC: https://github.com/apple-oss-distributions/Libm/blob/845f432413a66ea992f4797c66157c9824421719/Source/PowerPC/math.h#L704-L707

This is in 10.5.8 (Libm-292.4): https://github.com/apple-oss-distributions/Libm/blob/d9d7b69507796c31e17089f3aaedde1716a6002b/Source/PowerPC/math.h#L713-L716

So affected platforms, AFAIU, will be every macOS until 10.6.8, but only when built for ppc and ppc64.

[DrTimothyAldenDavis]

What would the fix look like if it were added to SuiteSparse?

I have a single file, SuiteSparse_config/cmake_modules/SuiteSparsePolicy.cmake, (and identical copies of that file inside GraphBLAS and LAGraph cmake_modules folders). There could be an option added there. What would the test look like?

I could add something like this, but it wouldn't simplify your solution in the Portfile

option ( SUITESPARSE_NO_MAC_LIBM_EXTENSIONS "...describe me here ... " OFF )
if ( SUITESPARSE_NO_MAC_LIBM_EXTENSIONS )
    add_compile_definitions ( __NO_EXTENSIONS__ )
endif ( )

I'm very reluctant to add a hard coded #define __NO_EXTENSIONS__ for all cases on all platforms, because it can be dangerous to #define tokens that start with __. I might break something else on another system that happens to use that token. The token __NO_EXTENSIONS__ is too generic. If the token were __NO_MAC_LIBM_EXTENSIONS__ or something that would be safer but of course we can't change that.

Maybe something like the following could be added to my SuiteSparsePolicy.cmake:

if ( APPLE )
    if ( CMAKE_MACOSX_DEPLOYMENT_TARGET ... is something ) ??
       add_compile_definitions ( __NO_EXTENSIONS__ )
    endif ( )
endif ( )

see https://cmake.org/cmake/help/latest/variable/APPLE.html and https://cmake.org/cmake/help/latest/envvar/MACOSX_DEPLOYMENT_TARGET.html
https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_DEPLOYMENT_TARGET.html#variable:CMAKE_OSX_DEPLOYMENT_TARGET

What values of CMAKE_MACOSX_DEPLOYMENT_TARGET would trigger the need for this __NO_EXTENSIONS__ flag?

Also -- in looking through the macports configuration for SuiteSparse, I found this:

https://github.com/macports/macports-ports/blob/a8065a850395e101ae56f0d7c8a3e0451f33e3b8/math/SuiteSparse/Portfile#L144

CHOLMOD has modules with different licenses. In particular, the MatrixOps, Modify, Supernodal, MATLAB, and GPU modules are GPL-2.0+, not LGPL. See the Doc/License.txt file or you can grep for SPDX, or see:

SuiteSparse/CHOLMOD/Include/cholmod.h

Lines 8 to 21 in bf98131

	// Each Module of CHOLMOD has its own license, and a shared cholmod.h file.
	// CHOLMOD/Check: SPDX-License-Identifier: LGPL-2.1+
	// CHOLMOD/Cholesky: SPDX-License-Identifier: LGPL-2.1+
	// CHOLMOD/Utility: SPDX-License-Identifier: LGPL-2.1+
	// CHOLMOD/Partition: SPDX-License-Identifier: LGPL-2.1+
	// CHOLMOD/Demo: SPDX-License-Identifier: GPL-2.0+
	// CHOLMOD/GPU: SPDX-License-Identifier: GPL-2.0+
	// CHOLMOD/MATLAB: SPDX-License-Identifier: GPL-2.0+
	// CHOLMOD/MatrixOps: SPDX-License-Identifier: GPL-2.0+
	// CHOLMOD/Modify: SPDX-License-Identifier: GPL-2.0+
	// CHOLMOD/Supernodal: SPDX-License-Identifier: GPL-2.0+
	// CHOLMOD/Tcov: SPDX-License-Identifier: GPL-2.0+

Those licenses have been in place ever since CHOLMOD was created in 2005.

Can you update the macports Portfile with the correct license?

[barracuda156]

@DrTimothyAldenDavis Thank you very much for responding.

Of course it is not needed to use the flag unconditionally for all platforms, it will be at best redundant and potentially undesirable or breaking.

This is what will be disabled on macOS < 10.7 by __NO_EXTENSIONS__ (provided anything at all was used, likely not):

Intel:
https://github.com/apple-oss-distributions/Libm/blob/845f432413a66ea992f4797c66157c9824421719/Source/Intel/math.h#L555-L636

PowerPC: https://github.com/apple-oss-distributions/Libm/blob/845f432413a66ea992f4797c66157c9824421719/Source/PowerPC/math.h#L695-L800

If we have no issues here, then the flag is to be used for 10.6 and below.

[DrTimothyAldenDavis]

Yes, I did look at those extensions and I don't use any of them, so __NO_EXTENSIONS__ seems safe to add.

What does the CMAKE_MACOSX_DEPLOYMENT_TARGET variable contain? I tried adding

message ( STATUS "macos: ${CMAKE_MACOSX_DEPLOYMENT_TARGET}" )
on my M1 Mac, but the variable was empty. Is there another way to test for the Mac OSX version in cmake?

[DrTimothyAldenDavis]

or maybe this?

https://cmake.org/cmake/help/latest/variable/CMAKE_OSX_ARCHITECTURES.html

[DrTimothyAldenDavis]

or if you're fine with the fix you have here, then this looks safe to me:

macports/macports-ports@e485b0a

[DrTimothyAldenDavis]

you can probably also remove the KLU patch, with __NO_EXTENSIONS__ defined, right?

[barracuda156]

CMAKE_OSX_ARCHITECTURES will work fine, since we only need this on PowerPC really. The values are ppc and ppc64. (Checking for CMAKE_SYSTEM_PROCESSOR will be less robust, since it will leave Rosetta case broken.)

I am not sure if deployment target is being set automatically. I can be set from the user side (or Macports side) though. Perhaps a check for the arch is a better choice here.

you can probably also remove the KLU patch, with __NO_EXTENSIONS__ defined, right?

Yes, that has been done already.

[DrTimothyAldenDavis]

OK. I am not sure if this is the right thing to add, but see this commit: 3892732

How is the CMAKE_OSX_ARCHITECTURES set? The variables are empty on my M1 Mac. I suppose you'll need to use

cmake -DCMAKE_OSX_ARCHITECTURES="ppc" or something?

[szhorvat]

@DrTimothyAldenDavis I'm not sure if using CMAKE_OSX_ARCHITECTURES is reliable in all scenarios, including cross-compilation and when multiple architectures are present (e.g. producing universal binaries). But I'm not very comfortable with CMake so maybe @mmuetzel can chime in.

How about doing it by OS X version instead?

if (APPLE AND CMAKE_SYSTEM_VERSION VERSION_LESS 11)
 ...
endif()

OS X 10.6 is Darwin version 10.x. Thus anything less than 11 will be 10.6 and lower.

This applies the workaround not only for PPC but for all very old Darwin systems. Based on this information I expect it wouldn't be a problem if the workaround is applied on old Intel platforms in addition to PPC.

[mmuetzel]

@szhorvat: That sounds reasonable. But shouldn't it be CMAKE_SYSTEM_VERSION (instead of CMAKE_HOST_SYSTEM_VERSION)? I'd guess the target would matter for this - not the host.

[szhorvat]

Yes, you are absolutely right. I mixed up the two. I'll edit my previous comment to avoid confusion.

[barracuda156]

@DrTimothyAldenDavis Having it empty should not create a problem (maybe PowerPC users would need to specify it, but Macports at least will do that), however given consideration for exotic builds, perhaps a suggestion of @szhorvat makes better sense then.

[DrTimothyAldenDavis]

OK. I just added this: 12202fa

SuiteSparse/SuiteSparse_config/cmake_modules/SuiteSparsePolicy.cmake

Lines 250 to 261 in 090e0bf

	#-------------------------------------------------------------------------------
	# MacOS workaround
	#-------------------------------------------------------------------------------
	# Older versions of math.h on the Mac (PowerPC only) define Real and Imag,
	# which conflicts the internal use of those symbols in KLU and CHOLMOD. This
	# can be disabled with -D__NO_EXTENSTIONS__
	if ( APPLE AND CMAKE_SYSTEM_VERSION VERSION_LESS 11 )
	message ( STATUS "MacOS: workaround for mangled math.h" )
	add_compile_definitions ( __NO_EXTENSTIONS__ )
	endif ( )

My M1 Mac is Darwin 22.6.0, and if I change the 11 to 23, it adds the -D__NO_EXTENSIONS__.

[mmuetzel]

I opened #690 that limits the scope of the workaround and fixes the name of the preprocessor definition.

[DrTimothyAldenDavis]

Oops. Well, Darwin's math.h is confusing: __WANT_EXTENSIONS__ has the extra underscore so I got confused. That is one totally mangled math.h file on the Mac...

[DrTimothyAldenDavis]

I think this is fixed now, in both the default dev branch and the development dev2 branch. Can you give it a try in macports?

[szhorvat]

A similar workaround was applied for 7.4.0 in macports/macports-ports#22029 and according to @barracuda156 it works. I don't have the possibility to test on PowerPC. I recommend you go ahead with 7.5.0 and don't wait for us.

Thanks for the fix!

[DrTimothyAldenDavis]

Ok. I will close this now but reopen it if it turns out that we still need some more work

[barracuda156]

@DrTimothyAldenDavis Thank you!

[DrTimothyAldenDavis]

We've made it a bit more general, for all packages in SuiteSparse, with this update from @mmuetzel :

#694

which is probably a good idea since none of SuiteSparse needs those extensions, and since I'm likely to use #define Real float in UMFPACK, when I extend it to handle single precision matrices.

[szhorvat]

@DrTimothyAldenDavis I don't want to open a new issue for this small question, so I'm asking here, even though it's unrelated. Is SPEX an API-compatible drop-in replacement for SLIP_LU?

[DrTimothyAldenDavis]

The SPEX package has had a major overhaul in its API, and it will get another revision in an upcoming release, as well, once we revise it for SPEX v3. So unfortunately it's not a simple drop-in replacement.

Once we reach SPEX v3, the API should become stable. It's been an iterative process to design this package and its API.