Created attachment 231058 [details] Patch for nfft (WIP) Doing a bit of work fftw3 related ports I noticed that this port pretty much violates all guidelines listed in Porters Handbook so I figured that I should try to get it in better shape. A few notes, The configure script throws errors if you use something else than bash. The current version pretty much ignores C/LD* flags set and does its own thing (including using -march=native). It sets "-O3 -fomit-frame-pointer -malign-double -fstrict-aliasing -ffast-math" compared to "-O2 -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing". There's obviously a difference but are there any options we should still keep? It now also throws "ld: error: ../../.libs/libnfft3.so: undefined reference to cexpl" on older versions of FreeBSD (see https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=216862) and at least for now I can't figure out how it worked before. My current workaround is to link libopenlibm but I don't think that's a great solution or even a correct one. A pair of eyes would be appreciated especially regarding the last issue I've CCed people who've touched this somewhat recently for feedback.
(In reply to Daniel Engberg from comment #0) Hi Daniel, I built the attached version of the port and I don't see any instances of fomit-frame-pointer in the logs: /usr/local/poudriere/data/logs/bulk/latest-per-pkg/nfft/3.5.2_2$ grep fomit-frame-pointer * /usr/local/poudriere/data/logs/bulk/latest-per-pkg/nfft/3.5.2_2$ I understand that those are the flags the port uses unconditionally regardless of C/LD FLAGS? On the issue of cexpl(). It is unfortunate that we don't have it in older versions of FreeBSD. nfft does not provide a "test" target to check for the correctness of the built artifacts, so I would mark the port BROKEN for versions not having cexpl(). math/openlibm seems a mature project, but still, it might lead to very subtle bugs to use different math libraries depending on the FreeBSD version. Since this is a leaf port, this shouldn't cause a cascade of problems. For curiosity I just did a git reset --hard a94f8e0af96a4182e9428d3d02f20c3d8325ecf7 and rebuilt nfft-3.5.1. It builts OK even in 12.2{amd64,i386}. Inspecting the code, cexpl() was already there, but I see that it is checked for its declaration in configure, so maybe somehow its usage was conditional but I can't see how.
Hi, "-O3 -fomit-frame-pointer -malign-double -fstrict-aliasing -ffast-math" is what the current version in tree sets not using the patch. The current version in tree doesn't choke on cexpl() but I wonder since we both use amd64 (?) if -march=native somehow defines it? I can't really figure out how this worked before... :/ Best regards, Daniel
(In reply to Daniel Engberg from comment #2) Thanks, I misunderstood the flags part. gcc seems to have cexpl() as a builtin function, but I see we use clang now and before with 3.5.1, so that can not be the explanation.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=fa49f6526d6cc24ebdf94471e0a4ae7635297ea6 commit fa49f6526d6cc24ebdf94471e0a4ae7635297ea6 Author: Daniel Engberg <diizzy@FreeBSD.org> AuthorDate: 2022-03-12 07:48:21 +0000 Commit: Daniel Engberg <diizzy@FreeBSD.org> CommitDate: 2022-03-12 07:48:36 +0000 math/nfft: Rework port to follow Porters Handbook * Use upstream release archive * Respect flags set by framework and do not use -march=native * Add support for OpenMP and enable it on aarch64 and amd64 by default * Overall rework of Makefile * Enable all options to match packaging in other distros PR: 261255 math/nfft/Makefile | 45 +- math/nfft/distinfo | 6 +- math/nfft/files/cpow.c (gone) | 56 -- math/nfft/files/patch-3rdparty_Makefile.am (gone) | 14 - math/nfft/pkg-plist | 670 +--------------------- 5 files changed, 36 insertions(+), 755 deletions(-)
Committed with additional changes compared to attched patch