Page MenuHomeFreeBSD

Start to split out the really x86 specific NOTES from the global notes file. Start with COMPAT_43, since it's really only relevant to x86.
ClosedPublic

Authored by imp on Aug 10 2019, 12:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 27 2023, 7:39 AM
Unknown Object (File)
Dec 20 2023, 8:22 AM
Unknown Object (File)
Nov 7 2023, 12:16 PM
Unknown Object (File)
Oct 31 2023, 2:42 AM
Unknown Object (File)
Oct 6 2023, 11:11 AM
Unknown Object (File)
Oct 5 2023, 8:54 PM
Unknown Object (File)
Sep 30 2023, 11:14 AM
Unknown Object (File)
Sep 8 2023, 8:46 AM
Subscribers

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

imp added inline comments.
sys/x86/conf/NOTES
12 ↗(On Diff #60628)

This comment is so FreeBSD 3.x and likely needs to be modernized

sys/conf/NOTES
339 ↗(On Diff #60628)

It would be good to copy the header bit over as well. I think in the MD NOTES files we keep the header blocks and try to keep the options sorted in the same relative order as MI NOTES.

sys/conf/makeLINT.mk
15 ↗(On Diff #60628)

I thought we'd just hack this file instead perhaps to add something like:

NOTES=
.if ${TARGET} == "amd64" || ${TARGET} == "i386"
NOTES+= x86/conf/NOTES
.endif

In particular, you could replace that with something using TARGET_CPUARCH in the future.

sys/x86/conf/NOTES
12 ↗(On Diff #60628)

Yeah, that's orthogonal, but probably it should say something like "this is only needed for legacy FreeBSD 1.x i386 a.out binaries"

sys/conf/NOTES
339 ↗(On Diff #60628)

ok

sys/conf/makeLINT.mk
15 ↗(On Diff #60628)

Yea, we could in the future, but now we can't. And we'd need to add back the ${.CURDIR}/NOTES too, which makes it too messy, imho, which is why I opted just for the default value.

sys/conf/makeLINT.mk
15 ↗(On Diff #60628)

In addition, I think that the current state of the values of MACHINE_CPUARCH would mean that using it here would be hard. Based on all this, I think my approach is simpler, but we can revisit once we have MACHINE_CPUARCH=x86 for i386/amd64 in place.

changes per jhb review, except the ifdef x86 in makeLINT.mk

sys/conf/makeLINT.mk
15 ↗(On Diff #60628)

Just to be clear, we wouldn't have to add back .CURDIR/NOTES, you'd keep the existing line and have a += under it:

NOTES=	${.CURDIR}/../../conf/NOTES ${.CURDIR}/NOTES
.if ${TARGET} == "amd64" || ${TARGET} == "i386"
NOTES+= ${.CURDIR}/../../x86/conf/NOTES
.endif

Either way is fine I guess, but I feel like this is clearer that we are just adding a new file instead of duplicating the default value of NOTES.

OTOH, I wonder if what we might not want instead is a 'NOTES_EXTRA'. You would modify makeLINT.mk to do something like this:

NOTES=	${.CURDIR}/../../conf/NOTES ${.CURDIR}/NOTES ${NOTES_EXTRA}

We might find that there are some other large "features" we might want to split out into an MI NOTES but only enable them for certain architectures.

An even simpler way perhaps might be to change makeLINT.mk to just use 'NOTES+=' instead of 'NOTES=', then you could change amd64/Makefile and i386/Makefile
to just have:

NOTES=${.CURDIR}/../../x86/conf/NOTES

And use '+=' in makeLINT.mk. Perhaps that is the simplest of all.

In D21203#461464, @jhb wrote:

OTOH, I wonder if what we might not want instead is a 'NOTES_EXTRA'. You would modify makeLINT.mk to do something like this:

NOTES=	${.CURDIR}/../../conf/NOTES ${.CURDIR}/NOTES ${NOTES_EXTRA}

We might find that there are some other large "features" we might want to split out into an MI NOTES but only enable them for certain architectures.

An even simpler way perhaps might be to change makeLINT.mk to just use 'NOTES+=' instead of 'NOTES=', then you could change amd64/Makefile and i386/Makefile
to just have:

NOTES=${.CURDIR}/../../x86/conf/NOTES

And use '+=' in makeLINT.mk. Perhaps that is the simplest of all.

True. Does the order matter? I guess it doesn't.

update per jhb's suggestions

This revision is now accepted and ready to land.Aug 12 2019, 5:57 PM