Page MenuHomeFreeBSD

Move the inclusion of bsd.cpu.mk from sys.mk to bsd.opts.mk.
ClosedPublic

Authored by imp on Nov 19 2015, 4:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jan 13, 2:38 AM
Unknown Object (File)
Tue, Dec 31, 6:48 AM
Unknown Object (File)
Sun, Dec 29, 10:43 PM
Unknown Object (File)
Nov 24 2024, 1:16 AM
Unknown Object (File)
Nov 18 2024, 4:24 PM
Unknown Object (File)
Nov 6 2024, 11:51 AM
Unknown Object (File)
Sep 27 2024, 9:36 PM
Unknown Object (File)
Sep 27 2024, 8:06 PM

Details

Reviewers
sjg
bdrewery
Group Reviewers
portmgr
Summary

Moving this has no effect on the in-tree users of bsd.*.mk,
though I haven't yet tested its effect on ports.

There's a number of good effects here. It reduces global namespace
pollution a bit, since sys.mk is included always. It makes it so that this
file can be influenced by all the MK_* variables floating around. It makes
it so that kernel builds can set a CPUTYPE to optimize to more easily
than the various hacks that exist today to kinda-sorts do the job. since bsd.init.mk
is called for all .mk files that do building, and since bsd.opts.mk is
included from there, this should neatly solve the problem without
introducing new ones.

Theoretically, there may be people that use make without any of the
bsd.*.mk files at all, and who also rely on this to get CFLAGS set in
a particular way. My experience suggests this is a vanishingly small group
of people.

Test Plan

make buildworld/installworld for all the architectures
ditto with the kernel
Maybe a full ports exp run.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1222
Build 1226: arc lint + arc unit

Event Timeline

imp retitled this revision from to Move the inclusion of bsd.cpu.mk from sys.mk to bsd.opts.mk..
imp updated this object.
imp edited the test plan for this revision. (Show Details)
imp edited the test plan for this revision. (Show Details)
imp added reviewers: bdrewery, sjg.
sjg edited edge metadata.

Looks ok thanks

This revision is now accepted and ready to land.Nov 19 2015, 5:52 PM

Any of these done not before a bsd.port.pre.mk are going to break. Once they include bsd.port.mk (which is really /usr/share/mk/bsd.port.mk which includes bsd.own.mk->bsd.opts.mk->bsd.cpu.mk, and then /usr/ports/Mk/bsd.port.mk) then they are safe. Anything in Mk/ is most likely safe already due to the same.

Given the scope of usage of MACHINE_CPU here, I would advise getting an exp-run before committing this. Just open a bugzilla PR in ports, assign to portmgr with exp-run in subject, go into advanced flags and change EXP-RUN to a question mark.

antoine@ handles these and has good turn around time on it.

Mk/bsd.qt.mk:. if ${ARCH} == i386 && empty(MACHINE_CPU:Msse2)
archivers/paq/Makefile:.if ${ARCH} == "amd64" || ( ${ARCH} == "i386" && !empty(MACHINE_CPU:Msse2) )
astro/boinc-setiathome-v7/Makefile:.if !empty(MACHINE_CPU:Maltivec)
audio/ardour/Makefile:. elif ${MACHINE_CPU:Msse}
audio/audacity/Makefile:OPTIONS_DEFAULT_i386= ${MACHINE_CPU:tu:MSSE}
audio/beast/Makefile:OPTIONS_DEFAULT_i386= ${MACHINE_CPU:Msse:tu}
audio/mpg123/Makefile:.if ${MACHINE_CPU:M3dnow}
audio/mpg123/Makefile:.elif ${MACHINE_CPU:Msse}
audio/mpg123/Makefile:.elif ${MACHINE_CPU:Mi586}
audio/mpg123/Makefile:.elif ${MACHINE_CPU:Mi486}
audio/soundtouch/Makefile:OPTIONS_DEFAULT= ${MACHINE_CPU:tu:MSOFTFP:S/SOFTFP/INTEGER_SAMPLES/}
audio/soundtouch/Makefile:OPTIONS_DEFAULT_i386= ${MACHINE_CPU:tu:MSSE}
audio/wavpack/Makefile:.if ${MACHINE_CPU:Mmmx}
benchmarks/wrk/Makefile:.if ${ARCH} == i386 && empty(MACHINE_CPU:Mi586)
biology/ugene/Makefile:.if ${MACHINE_CPU:Msse2}
devel/libast/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:Mmmx}
devel/ponscripter-sekai/Makefile:OPTIONS_DEFAULT=MANPAGES ${MACHINE_CPU:tu:MSOFTFP:S/SOFTFP/TREMOR/}
devel/sdl20/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:Mmmx}
devel/sdl20/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:M3dnow}
devel/sdl20/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:Msse}
devel/sdl20/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:Msse2}
emulators/xsystem35/Makefile:OPTIONS_DEFAULT_i386= ${MACHINE_CPU:tu:MMMX}
games/openbor/Makefile:OPTIONS_DEFAULT= ${MACHINE_CPU:tu:MSOFTFP:S/SOFTFP/TREMOR/}
graphics/GraphicsMagick/Makefile:.if ${MACHINE_CPU:Msse}
graphics/GraphicsMagick/Makefile:.if ${MACHINE_CPU:Msse2}
graphics/GraphicsMagick/Makefile:.if ${MACHINE_CPU:Msse3}
graphics/hdr_tools/Makefile:.if ${MACHINE_CPU:Msse2}
graphics/imlib2/Makefile:.if ${ARCH} == "i386" && !empty(MACHINE_CPU:Mmmx)
graphics/opencolorio/Makefile:.if ${MACHINE_CPU:Msse2}
graphics/opencv/Makefile:. if ${MACHINE_CPU:Msse}
graphics/opencv/Makefile:. if ${MACHINE_CPU:Msse2}
graphics/opencv/Makefile:. if ${MACHINE_CPU:Msse3}
lang/mosh/Makefile:.if ${MACHINE_CPU:Msse3}
lang/mosh/Makefile:.elif ${MACHINE_CPU:Msse2}
lang/mosh/Makefile:.elif ${MACHINE_CPU:Msse}
lang/mosh/Makefile:.elif ${MACHINE_CPU:Mmmx}
math/crlibm/Makefile:.if !empty(MACHINE_CPU:Msse2)
math/fftw3/Makefile:#Users must add altivec to MACHINE_CPU when desired:
math/fftw3/Makefile:. if !empty(MACHINE_CPU:Mavx)
math/fftw3/Makefile:. elif !empty(MACHINE_CPU:Msse2)
math/fftw3/Makefile:. if !empty(MACHINE_CPU:Mavx)
math/fftw3/Makefile:. elif !empty(MACHINE_CPU:Msse)
math/fftw3/Makefile:. elif !empty(ARCH:Mpowerpc*) && !empty(MACHINE_CPU:Maltivec)
math/gretl/Makefile:.if !empty(MACHINE_CPU:Msse2)
math/libflame/Makefile:.if !empty(MACHINE_CPU:Msse3)
math/msieve/Makefile:.if ${MACHINE_CPU:Mathlon}
math/msieve/Makefile:.if ${MACHINE_CPU:Msse}
math/msieve/Makefile:.if ${MACHINE_CPU:Msse2}
math/sfft/Makefile:.if !${ARCH:Mamd64} && !${MACHINE_CPU:Msse2}
multimedia/ffmpeg/Makefile:OPTIONS_DEFAULT_i386= ${MACHINE_CPU:tu:MMMX} ${MACHINE_CPU:tu:MSSE}
multimedia/ffmpeg0/Makefile:.if defined(MACHINE_CPU) && (${MACHINE_CPU:Msse} == "sse" || ${MACHINE_CPU:Mamd64} == "amd64")
multimedia/ffmpeg0/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:Mmmx} == "" && ${MACHINE_CPU:Mamd64} == ""
multimedia/gstreamer-ffmpeg/Makefile:.if defined(MACHINE_CPU) && (${MACHINE_CPU:Msse} == "sse" || ${MACHINE_CPU:Mamd64} == "amd64")
multimedia/gstreamer-ffmpeg/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:Mmmx} == "" && ${MACHINE_CPU:Mamd64} == ""
multimedia/libdvbcsa/Makefile:.if defined(MACHINE_CPU) && ${MACHINE_CPU:Msse2}
multimedia/libdvbcsa/Makefile:.elif defined(MACHINE_CPU) && ${MACHINE_CPU:Mmmx}
multimedia/mpeg4ip/Makefile:.if (defined(MACHINE_CPU) && ${MACHINE_CPU:Mmmx} == "mmx") && !defined(PACKAGE_BUILDING)
multimedia/vid.stab/files/patch-CMakeModules_FindSSE.cmake:+ EXEC_PROGRAM(make ARGS "-V MACHINE_CPU" OUTPUT_VARIABLE CPUINFO)
security/beecrypt/Makefile:.if ${MACHINE_CPU:M$o} != ""
security/gnupg1/Makefile:.if ${MACHINE_CPU:Mi586}
security/john/Makefile:. if ${MACHINE_CPU:Msse2}
security/john/Makefile:. elif ${MACHINE_CPU:Mmmx}
security/john/Makefile: '/(fopenmp|-m${MACHINE_CPU:Msse2})$$/s,#(OMPFLAGS =),\1,' \
www/chromium/Makefile:.if ! ${MACHINE_CPU:Msse2}
www/firefox/Makefile.options: ${MACHINE_CPU:tu:MSOFTFP:S/SOFTFP/INTEGER_SAMPLES/}

Any of these done not before a bsd.port.pre.mk are going to break. Once they include bsd.port.mk (which is really /usr/share/mk/bsd.port.mk which includes bsd.own.mk->bsd.opts.mk->bsd.cpu.mk, and then /usr/ports/Mk/bsd.port.mk) then they are safe. Anything in Mk/ is most likely safe already due to the same.

I am not sure all of those can be converted to include bsd.port.pre.mk, some of them use MACHINE_CPU do determine OPTIONS_DEFAULT (which has to be before bsd.port.options.mk/bsd.port.pre.mk)

Any of these done not before a bsd.port.pre.mk are going to break. Once they include bsd.port.mk (which is really /usr/share/mk/bsd.port.mk which includes bsd.own.mk->bsd.opts.mk->bsd.cpu.mk, and then /usr/ports/Mk/bsd.port.mk) then they are safe. Anything in Mk/ is most likely safe already due to the same.

I am not sure all of those can be converted to include bsd.port.pre.mk, some of them use MACHINE_CPU do determine OPTIONS_DEFAULT (which has to be before bsd.port.options.mk/bsd.port.pre.mk)

These would be easy to fix with a bsd.init.mk being included, but there are a lot of them.

Not sure if this will come in. We could include bsd.cpu.mk in sys.mk if ../../Mk/bsd.port.mk exists.(see full logic in share/mk/bsd.port.mk). This kind of check in sys.mk gives us a chance for a ports_make.conf too.

Regards,
Bryan Drewery

Not sure if this will come in. We could include bsd.cpu.mk in sys.mk if ../../Mk/bsd.port.mk exists.(see full logic in share/mk/bsd.port.mk). This kind of check in sys.mk gives us a chance for a ports_make.conf too.

I literally ground my teeth when I read this, so I thought I'd think about it...

On the one hand, when in the ports tree, this would be more robust way to move forward.
We won't miss anything, and it won't fail silently like the current patch will.

On the other hand, I worry about false positives. There's been many times we've done
something similar and been burned. On the other hand, this pattern is much more
specific and less generic than where we've hit issues in the past.

There's some problems with the current bsd.cpu.mk file that might be useful to document
at this point in our deliberations. Ports don't generally use CPUCFLAGS in any meaningful
way, but /usr/src does. /usr/src generally doesn't use MACHINE_CPU for anything (though
there's one or two places where it does), but ports use it to influence the build.

The lone example of MACHINE_CPU in /usr/src occurs after an include of bsd.own.mk, so
this change won't break that. And it would be easy to do something different if we decided
we wanted to make a change.

One could view the current bsd.cpu.mk as two different files residing in one. One is CFLAGS focused,
while the other is MACHINE_CPU focused. I'd like to think about splitting this file in two as
an alternate path.

So I'll try both paths to see which one makes the most sense.

By the way "Not sure if this will come in" meant the comment since I mailed it in! Success.

This was fixed in r292084 under D4383