Page MenuHomeFreeBSD

Allow bsd.compat.mk to be reliably included outside Makefile.inc1.
ClosedPublic

Authored by brooks on Oct 16 2019, 5:31 PM.

Details

Summary

Replace explicit TARGET_* variables with COMPAT_* versions defined based on where the file is being included.

Also, require that bsd.compat.mk be included directly. It's not going to be widely used so always loading it in bsd.prog.mk doesn't make sense. Instead users can include it directly.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

brooks created this revision.Oct 16 2019, 5:31 PM
bdrewery accepted this revision.Oct 16 2019, 7:26 PM

Oops sorry for missing this before. TARGET_ are only useful in Makefile.inc1.

This revision is now accepted and ready to land.Oct 16 2019, 7:26 PM
imp requested changes to this revision.Oct 17 2019, 3:47 PM

The question is, do we need them to be TARGET_* in this makefile.inc1 usage?
I kinda think it does. Checking into *that* detail...

This revision now requires changes to proceed.Oct 17 2019, 3:47 PM
In D22059#482035, @imp wrote:

The question is, do we need them to be TARGET_* in this makefile.inc1 usage?
I kinda think it does. Checking into *that* detail...

I'm testing a refactor where we use TARGET_* with included from Makefile.inc1 (by checking that _LIBCOMPAT is defined) or MACHINE_ARCH/CPUTYPE elsewhere.

brooks updated this revision to Diff 63409.Oct 17 2019, 6:57 PM
  • Get ARCH and CPUTYPE from contextually appropriate sources.
brooks edited the summary of this revision. (Show Details)Oct 17 2019, 6:59 PM
brooks added inline comments.Oct 17 2019, 7:02 PM
share/mk/bsd.prog.mk
5 ↗(On Diff #63409)

We could chose to not include bsd.compat.mk here and require users to include it (after defining NEED_COMPAT/WANT_COMPAT). We'll probably need include it directly anyway to support more complicated programs like rtld.

In D22059#482035, @imp wrote:

The question is, do we need them to be TARGET_* in this makefile.inc1 usage?
I kinda think it does. Checking into *that* detail...

I had that thought but I don't think so. bsd.compat.mk is included from Makefiles at the point in the build where MACHINE=$TARGET and MACHINE_ARCH=$TARGET_ARCH from CROSSENV. buildlibcompat is a WMAKE_TGT so it should have these MACHINE= and MACHINE_ARCH= values set.

imp added a comment.Oct 17 2019, 10:25 PM
In D22059#482035, @imp wrote:

The question is, do we need them to be TARGET_* in this makefile.inc1 usage?
I kinda think it does. Checking into *that* detail...

I had that thought but I don't think so. bsd.compat.mk is included from Makefiles at the point in the build where MACHINE=$TARGET and MACHINE_ARCH=$TARGET_ARCH from CROSSENV. buildlibcompat is a WMAKE_TGT so it should have these MACHINE= and MACHINE_ARCH= values set.

Makefile.inc1 include Makefile.libcompat which includes bsd.compat.mk them without MACHINE=$TARGET. MACHINE/MACHINE_ARCH are still the host's at that point. It sets up the targets that are later used with MACHINE=TARGET etc. It's how it builds buildlibompat that's dependent on the target. We won't have a buildlibcompat since it's really:
build${libcompat}: .PHONY
and libcompat is set based on the target...

It's certianly at best confusing...

Any further comments on this version?

imp added a comment.Wed, Oct 30, 7:56 PM

I don't see anything wrong. the discussion that bdrewery and I were having was that there may be a more optimal way.

share/mk/bsd.prog.mk
5 ↗(On Diff #63409)

My lean is this way, since the need/want things are fairly specialized and of interest to only a few files.

imp accepted this revision.Wed, Oct 30, 7:56 PM
This revision is now accepted and ready to land.Wed, Oct 30, 7:56 PM
brooks updated this revision to Diff 63815.Wed, Oct 30, 10:29 PM
  • libstats: Fix ABI assertion.
  • Require that bsd.compat.mk be explicitly included and update docs.
This revision now requires review to proceed.Wed, Oct 30, 10:29 PM
brooks updated this revision to Diff 63816.Wed, Oct 30, 10:31 PM
  • Apply patch that arc applied, but didn't submit?!?!
brooks edited the summary of this revision. (Show Details)Wed, Oct 30, 10:32 PM
brooks marked an inline comment as done.
imp accepted this revision.Wed, Oct 30, 10:46 PM
imp added inline comments.
sys/kern/subr_stats.c
128 ↗(On Diff #63816)

unrelated noise?

This revision is now accepted and ready to land.Wed, Oct 30, 10:46 PM
brooks marked an inline comment as done.Wed, Oct 30, 11:03 PM
brooks added inline comments.
sys/kern/subr_stats.c
128 ↗(On Diff #63816)

Bah, it turns out that if you update the wrong tree arc will happily mess everything up. Update coming shortly

brooks updated this revision to Diff 63817.Wed, Oct 30, 11:04 PM
brooks marked an inline comment as done.
  • Allow bsd.compat.mk to be reliably included outside Makefile.inc1.
  • Get ARCH and CPUTYPE from contextually appropriate sources.
  • Require that bsd.compat.mk be explicitly included and update docs.
This revision now requires review to proceed.Wed, Oct 30, 11:04 PM
imp accepted this revision.Wed, Oct 30, 11:30 PM
This revision is now accepted and ready to land.Wed, Oct 30, 11:30 PM