Bug 272651 - biology/bifrost: Pass the value of the CPUTYPE variable to the build
Summary: biology/bifrost: Pass the value of the CPUTYPE variable to the build
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: Jason W. Bacon
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-07-22 06:28 UTC by Yuri Victorovich
Modified: 2023-08-12 14:44 UTC (History)
2 users (show)

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

Attachments
patch (1.23 KB, patch)
2023-07-22 06:28 UTC, Yuri Victorovich 		no flags Details | Diff
patch (1.17 KB, patch)
2023-08-12 08:16 UTC, Yuri Victorovich 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer freebsd_triage 2023-07-22 06:28:52 UTC 
Created attachment 243536 [details]
patch
Comment 1 Benjamin Takacs 2023-07-22 07:09:41 UTC 
Why adding an OPTION instead of supporting CPUTYPE?
Comment 2 Yuri Victorovich freebsd_committer freebsd_triage 2023-07-22 07:31:34 UTC 
(In reply to Benjamin Takacs from comment #1)

> Why adding an OPTION instead of supporting CPUTYPE?

Because:
* this project defined an option based on "native" vs. "none" choice, which makes sense
* CPUTYPE is functionally equivalent to the NATIVE option in this case
* CPUTYPE would require more lines in the Makefile in this case
* IMO NATIVE option is more intuitive


Yuri
Comment 3 Daniel Engberg freebsd_committer freebsd_triage 2023-07-22 09:37:57 UTC 
Please don't do this, it (native) breaks in various ways and is non functional on anything non amd64.

As Benjamin pointed out use CPUFLAGS and you can pass that variable to CMAKE if needed for whatever reason.
Comment 4 Daniel Engberg freebsd_committer freebsd_triage 2023-07-22 09:38:31 UTC 
(In reply to Daniel Engberg from comment #3)
...err, CPUFLAGS --> CPUTYPE of course
Comment 5 Jason W. Bacon freebsd_committer freebsd_triage 2023-07-22 11:57:01 UTC 
Wouldn't it be better to just specify CFLAGS/CXXFLAGS/FFLAGS outside the individual ports?

Adding

CXXFLAGS+=-march=native

to /etc/make.conf on systems where analyses are run will optimize the build of all ports that respect the env (which should in theory be all of them, but we know better).
Comment 6 Daniel Engberg freebsd_committer freebsd_triage 2023-08-11 03:19:05 UTC 
Yes, but that would be redundant to defining CPUTYPE per port ;-)
Comment 7 Yuri Victorovich freebsd_committer freebsd_triage 2023-08-11 17:00:44 UTC 
(In reply to Daniel Engberg from comment #6)

Thanks Daniel,

I agree - NATIVE option is a bad idea.
I'll update the patch.


Yuri
Comment 8 Yuri Victorovich freebsd_committer freebsd_triage 2023-08-12 08:16:48 UTC 
Created attachment 244037 [details]
patch

This patch passes CPUTYPE to cmake when it is set.
Comment 9 commit-hook freebsd_committer freebsd_triage 2023-08-12 14:43:23 UTC 
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=f38473fec0a852349f186e1d28358fc2ce5636de

commit f38473fec0a852349f186e1d28358fc2ce5636de
Author:     Jason W. Bacon <jwb@FreeBSD.org>
AuthorDate: 2023-08-12 14:38:47 +0000
Commit:     Jason W. Bacon <jwb@FreeBSD.org>
CommitDate: 2023-08-12 14:38:47 +0000

    biology/bifrost: Control -march via CPUTYPE make variable

    Also disable hard-coded -O3 in cmake to respect user env

    PR:             272651
    Reported by:    yuri
    Reviewed by:    nimaje+fbz@bureaucracy.de, dizzy

 biology/bifrost/Makefile                   |  5 ++++-
 biology/bifrost/files/patch-CMakeLists.txt | 17 ++++++++---------
 2 files changed, 12 insertions(+), 10 deletions(-)
Comment 10 Jason W. Bacon freebsd_committer freebsd_triage 2023-08-12 14:44:42 UTC 
Thanks for the input!