Fixed libcxx math.h for doxygen #25895

burgerbecky · 2024-09-24T03:23:33Z

Doxygen includes math.h using "C" code which will fail compilation when including math.h due to a patch that was applied that assumed that "extern "C" {" is acceptable when compiling "C" code. By wrapping this line with #ifdef __cplusplus, Doxygen 1.11 compiles fine on Snow Leopard.

Description

When building Doxygen 1.11 on Snow Leopard, the compilation will fail due to math.h not being compatible with the C compiler that that port is using. Simply wrapping the 'extern "C" {' with #ifdef __cplusplus allowed doxygen to build with no errors.

Type(s)

bugfix
enhancement
security fix

Tested on

macOS x.y
Xcode x.y / Command Line Tools x.y.z

Verification

Have you

[X ] followed our Commit Message Guidelines?
[X ] squashed and minimized your commits?
[ X] checked that there aren't other open pull requests for the same change?
[X ] referenced existing tickets on Trac with full URL in commit message?
[ X] checked your Portfile with port lint --nitpick?
[X ] tried existing tests with sudo port test?
[ X] tried a full install with sudo port -vst install?
[ X] tested basic functionality of all binary files?
[ X] checked that the Portfile's most important variants haven't been broken?

Doxygen includes math.h using "C" code which will fail compilation when including math.h due to a patch that was applied that assumed that "extern "C" {" is acceptable when compiling "C" code. By wrapping this line with #ifdef __cplusplus, Doxygen 1.11 compiles fine on Snow Leopard.

macportsbot · 2024-09-24T03:23:41Z

Notifying maintainers:
@jeremyhu for port libcxx, llvm-10, llvm-11, llvm-12, llvm-3.3, llvm-3.4, llvm-3.7, llvm-5.0, llvm-6.0, llvm-7.0, llvm-8.0, llvm-9.0, llvm-devel.
@larryv for port llvm-3.3, llvm-3.4, llvm-3.7.
@cjones051073 for port llvm-devel.
@MarcusCalhoun-Lopez for port libstdcxx_clang_fix.
@catap for port clang-11-bootstrap, libcxx-powerpc, libcxx.
@barracuda156 for port libcxx-powerpc.

barracuda156 · 2024-09-24T05:14:21Z

What is going on here?

P. S. Please mark as checked only those items which have actually been done. I have some doubts, for example, that anything at all was tested with libcxx-powerpc port, even building it to begin with.

burgerbecky · 2024-09-24T05:55:01Z

What is going on here?

P. S. Please mark as checked only those items which have actually been done. I have some doubts, for example, that anything at all was tested with libcxx-powerpc port, even building it to begin with.

Yes, I build on PowerPC. I have a G4 and a G5 Xserve on the rack for just such a thing. My websites are https://burgerbecky.com and https://logicware.com I'm the CEO of Olde Skuul, and I do vintage software development. I found this issue because I was building Doxygen for a G5 iMac, a G5 Xserve (Both running Leopard), but the main one was a Xeon XServe running Snow Leopard. I use Snow Leopard so buildbot can access CodeWarrior running under Carbon since it's the last OS that still supported Rosetta for PowerPC.

Google Rebecca Heineman if you don't believe me.

burgerbecky · 2024-09-24T05:56:58Z

To trigger issue. Run a Snow Leopard Intel Mac. "port install doxygen". It will fail due to a compilation error due to code being compiled in C, that doesn't support "extern "C""
Apply patch. Build Doxygen. PROFIT!

burgerbecky · 2024-09-24T06:04:30Z

What is going on here?

P. S. Please mark as checked only those items which have actually been done. I have some doubts, for example, that anything at all was tested with libcxx-powerpc port, even building it to begin with.

Proof

barracuda156 · 2024-09-24T07:23:54Z

@burgerbecky Thank you for a detailed response. Perhaps I never saw errors with doxygen since on powerpc clangs are broken and gcc is used.

  doxygen @1.11.0_0 requested_variants='' platform='darwin 10' archs='ppc' date='2024-06-29T01:28:00+0800'
36-25% port -v deps doxygen
Full Name: doxygen @1.11.0_0
Build Dependencies:   path:bin/cmake:cmake, port:bison, port:flex,
                      path:bin/perl:perl5, port:python312, path:bin/git:git,
                      port:gcc14
Library Dependencies: port:libiconv, path:share/doc/libgcc/README:libgcc,
                      path:lib/libMacportsLegacySupport.dylib:legacy-support

burgerbecky · 2024-09-24T08:44:40Z

I hope to see why clang is so darn slow on ppc. Likely vm is hit or massive stalls from load hit store.

barracuda156 · 2024-09-24T08:55:44Z

I hope to see why clang is so darn slow on ppc. Likely vm is hit or massive stalls from load hit store.

Which clang do you use on ppc? I think @kencu has hacked clang-5.0 ages ago to build on ppc, but it did not generate correct code, so was abandoned. By default in MacPorts no clang later than 3.x will even build on ppc, AFAIK. And those are useless anyway, even if built.

kencu · 2024-09-24T14:30:41Z

math.h has been the same for years ( like 10 years.now ) and it has always worked fine.

this math.h header should only be called in when using the c++ compiler anyway, and never when compiling C code.

legacy-support was recently added to doxygen.

for some reason now, the c++ header directory is being used when compiling C code in doxygen.

So that is the real error to fix here.

the 10.6 build log is here:

https://build.macports.org/builders/ports-10.6_x86_64-builder/builds/214013/steps/install-port/logs/stdio

/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_textproc_doxygen/doxygen/work/doxygen-1.11.0/deps/sqlite3 && /opt/local/bin/clang-mp-11  -I/opt/local/include -pipe -Os -DNDEBUG -isystem/opt/local/include/libcxx/v1 -nostdinc++ -isystem/opt/local/include/LegacySupport -I/opt/local/include -DSQLITE_OMIT_LOAD_EXTENSION=1 -arch x86_64 -mmacosx-version-min=10.6 -fvisibility=hidden -MD -MT deps/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o -MF CMakeFiles/sqlite3.dir/sqlite3.c.o.d -o CMakeFiles/sqlite3.dir/sqlite3.c.o -c /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_textproc_doxygen/doxygen/work/doxygen-1.11.0/deps/sqlite3/sqlite3.c
In file included from /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_textproc_doxygen/doxygen/work/doxygen-1.11.0/deps/libmscgen/gd.c:4:
/opt/local/include/libcxx/v1/math.h:316:8: error: expected identifier or '('
extern "C" {
       ^
1 error generated.

kencu · 2024-09-24T14:36:34Z

exactly WHY the cxxflags are being used when compiling C code is not yet clear to me from a casual look over the doxygen Portfile.

kencu · 2024-09-24T14:45:05Z

there is an error in legacysupport-1.1

it looks like the cxxflags are being added to cppflags instead of cxxflags

append ls_cache_cppflags " -nostdinc++ -isystem${prefix}/include/libcxx/v1"

so that is probably the issue to fix.

kencu · 2024-09-24T14:54:46Z

having said that, this commit may well be OK… the math.h header in llvm’s c++ directory is wrapped with a cplusplus test too:

https://github.com/llvm/llvm-project/blob/3fbf6f8bb183ad8b9157e50c442479f4ca7a9b8d/libcxx/include/math.h#L304

burgerbecky · 2024-09-24T16:55:47Z

math.h has been the same for years ( like 10 years.now ) and it has always worked fine.

this math.h header should only be called in when using the c++ compiler anyway, and never when compiling C code.

legacy-support was recently added to doxygen.

for some reason now, the c++ header directory is being used when compiling C code in doxygen.

So that is the real error to fix here.

the 10.6 build log is here:

https://build.macports.org/builders/ports-10.6_x86_64-builder/builds/214013/steps/install-port/logs/stdio
/opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_textproc_doxygen/doxygen/work/doxygen-1.11.0/deps/sqlite3 && /opt/local/bin/clang-mp-11  -I/opt/local/include -pipe -Os -DNDEBUG -isystem/opt/local/include/libcxx/v1 -nostdinc++ -isystem/opt/local/include/LegacySupport -I/opt/local/include -DSQLITE_OMIT_LOAD_EXTENSION=1 -arch x86_64 -mmacosx-version-min=10.6 -fvisibility=hidden -MD -MT deps/sqlite3/CMakeFiles/sqlite3.dir/sqlite3.c.o -MF CMakeFiles/sqlite3.dir/sqlite3.c.o.d -o CMakeFiles/sqlite3.dir/sqlite3.c.o -c /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_textproc_doxygen/doxygen/work/doxygen-1.11.0/deps/sqlite3/sqlite3.c
In file included from /opt/local/var/macports/build/_opt_bblocal_var_buildworker_ports_build_ports_textproc_doxygen/doxygen/work/doxygen-1.11.0/deps/libmscgen/gd.c:4:
/opt/local/include/libcxx/v1/math.h:316:8: error: expected identifier or '('
extern "C" {
       ^
1 error generated.

This is my exact error

kencu · 2024-09-25T00:49:35Z

using the cxxflags when compiling C code was bound to eventually cause a failure.

burgerbecky · 2024-09-25T03:05:53Z

Can the workflow get approval?

kencu · 2024-09-25T10:15:57Z

no, we can’t push this as is and shouldn’t tie down the CI either imho

rebuilding (and revbumping) every single clang compiler will bring the buildbot sysyem to it’s knees for days.

So it will have to be done in a piecemeal, spread-out way, starting with the compilers actually used on 10.6 (clang-11).

And the proper fix for doxygen’s build failure is to fix legacysupport to modify the cxxflags instead of the cppflags anyway.

notifying @cjones051073 to orchestrate

cjones051073 · 2024-09-25T10:19:45Z

Ken - no rev bump is done, or needed here, as the fixes only affect builds currently not working anyway.

I do agree though I would first like to see someone test if fixing legacysupport addresses the issue. Thats not to say these changes couldn't perhaps also go in but I would prefer to see that route tested first.

kencu · 2024-09-25T15:46:37Z

??

to get the new math.h installed will require revbumping any of these that built on 10.6, Chris.

everything up to clang-17 has been built on 10.6, so they would all need revbumping.

kencu · 2024-09-25T15:48:13Z

of course, we can skip the math.h fix as it is really unnecessary once legacysupport has been rewritten properly.

cjones051073 · 2024-09-25T16:38:38Z

??

to get the new math.h installed will require revbumping any of these that built on 10.6, Chris.

everything up to clang-17 has been built on 10.6, so they would all need revbumping.

I perhaps did not explain myself well. My point was in this case I agree rev bumping all LLVM versions, and this forcing the buildbots to build them all again, on all OSes versions, when the issue is only on one ancient OS, and seemingly (so far) only affecting one port (doxygen is completely wasteful and should be avoided.

So, whilst you are correct 'by the book' the ports should be rev bump to force rebuilds, in this case I would not do it and just leave it to interest users to pick up themselves, given those affected are the very (very) small minority corner case.

If you still disagree, and think the changes here have to go with a rev-bump, then I am going to object to merging them at all as I do not think the impact of the fix justifies all the builedbot build time it implies.

ryandesign · 2024-09-30T04:37:53Z

If this PR changes files that are installed, the revision must be increased.

It sounds like the changes in this PR are not the correct way to fix this problem.

reneeotten · 2024-12-06T20:56:11Z

It appears the consensus is that this isn't the correct way to fix it. Combined with inactivity for 2 months, closing this PR.

macportsbot assigned jeremyhu, larryv, cjones051073 and MarcusCalhoun-Lopez Sep 24, 2024

macportsbot added the maintainer: requires approval label Sep 24, 2024

kencu mentioned this pull request Oct 5, 2024

initial nodejs14->22 changes #26031

Merged

12 tasks

reneeotten closed this Dec 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed libcxx math.h for doxygen #25895

Fixed libcxx math.h for doxygen #25895

burgerbecky commented Sep 24, 2024

macportsbot commented Sep 24, 2024

barracuda156 commented Sep 24, 2024 •

edited

Loading

burgerbecky commented Sep 24, 2024

burgerbecky commented Sep 24, 2024

burgerbecky commented Sep 24, 2024

barracuda156 commented Sep 24, 2024

burgerbecky commented Sep 24, 2024

barracuda156 commented Sep 24, 2024

kencu commented Sep 24, 2024 •

edited by ryandesign

Loading

kencu commented Sep 24, 2024

kencu commented Sep 24, 2024 •

edited

Loading

kencu commented Sep 24, 2024

burgerbecky commented Sep 24, 2024 •

edited by ryandesign

Loading

kencu commented Sep 25, 2024

burgerbecky commented Sep 25, 2024

kencu commented Sep 25, 2024 •

edited

Loading

cjones051073 commented Sep 25, 2024

kencu commented Sep 25, 2024 •

edited

Loading

kencu commented Sep 25, 2024

cjones051073 commented Sep 25, 2024

ryandesign commented Sep 30, 2024

reneeotten commented Dec 6, 2024

Fixed libcxx math.h for doxygen #25895

Fixed libcxx math.h for doxygen #25895

Conversation

burgerbecky commented Sep 24, 2024

Description

Type(s)

Tested on

Verification

macportsbot commented Sep 24, 2024

barracuda156 commented Sep 24, 2024 • edited Loading

burgerbecky commented Sep 24, 2024

burgerbecky commented Sep 24, 2024

burgerbecky commented Sep 24, 2024

barracuda156 commented Sep 24, 2024

burgerbecky commented Sep 24, 2024

barracuda156 commented Sep 24, 2024

kencu commented Sep 24, 2024 • edited by ryandesign Loading

kencu commented Sep 24, 2024

kencu commented Sep 24, 2024 • edited Loading

kencu commented Sep 24, 2024

burgerbecky commented Sep 24, 2024 • edited by ryandesign Loading

kencu commented Sep 25, 2024

burgerbecky commented Sep 25, 2024

kencu commented Sep 25, 2024 • edited Loading

cjones051073 commented Sep 25, 2024

kencu commented Sep 25, 2024 • edited Loading

kencu commented Sep 25, 2024

cjones051073 commented Sep 25, 2024

ryandesign commented Sep 30, 2024

reneeotten commented Dec 6, 2024

barracuda156 commented Sep 24, 2024 •

edited

Loading

kencu commented Sep 24, 2024 •

edited by ryandesign

Loading

kencu commented Sep 24, 2024 •

edited

Loading

burgerbecky commented Sep 24, 2024 •

edited by ryandesign

Loading

kencu commented Sep 25, 2024 •

edited

Loading

kencu commented Sep 25, 2024 •

edited

Loading