Page MenuHomeFreeBSD
Differential D22059

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

Authored by brooks on Oct 16 2019, 5:31 PM.
Tags
None
Referenced Files
F103225039: D22059.id63409.diff
Fri, Nov 22, 10:02 AM
Unknown Object (File)
Thu, Nov 21, 9:13 AM
Unknown Object (File)
Wed, Nov 20, 1:56 AM
Unknown Object (File)
Wed, Nov 20, 1:29 AM
Unknown Object (File)
Tue, Nov 19, 12:30 PM
Unknown Object (File)
Tue, Nov 19, 7:31 AM
Unknown Object (File)
Fri, Nov 15, 10:27 AM
Unknown Object (File)
Sun, Nov 10, 8:39 PM
View All Files
Subscribers
bdrewery
emaste

Details

Reviewers
imp
bdrewery
Commits
rS354227: Allow bsd.compat.mk to be reliably included outside Makefile.inc1.
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 - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

brooks created this revision.Oct 16 2019, 5:31 PM
Herald added subscribers: emaste, bdrewery. · View Herald TranscriptOct 16 2019, 5:31 PM
Harbormaster completed remote builds in B27074: Diff 63374.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
brooks added a comment.Oct 17 2019, 5:18 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.
Harbormaster completed remote builds in B27089: Diff 63409.Oct 17 2019, 6:57 PM
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.

bdrewery added a comment.Oct 17 2019, 7:57 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.

imp added a comment.Oct 17 2019, 10:25 PM
In D22059#482133, @bdrewery wrote:
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...

brooks added a comment.Oct 30 2019, 6:24 PM

Any further comments on this version?

imp added a comment.Oct 30 2019, 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.Oct 30 2019, 7:56 PM
This revision is now accepted and ready to land.Oct 30 2019, 7:56 PM
brooks updated this revision to Diff 63815.Oct 30 2019, 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.Oct 30 2019, 10:29 PM
Harbormaster completed remote builds in B27288: Diff 63815.Oct 30 2019, 10:29 PM
brooks updated this revision to Diff 63816.Oct 30 2019, 10:31 PM
  • Apply patch that arc applied, but didn't submit?!?!
Harbormaster completed remote builds in B27289: Diff 63816.Oct 30 2019, 10:31 PM
brooks edited the summary of this revision. (Show Details)Oct 30 2019, 10:32 PM
brooks marked an inline comment as done.
imp accepted this revision.Oct 30 2019, 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.Oct 30 2019, 10:46 PM
brooks marked an inline comment as done.Oct 30 2019, 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.Oct 30 2019, 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.Oct 30 2019, 11:04 PM
Harbormaster completed remote builds in B27290: Diff 63817.Oct 30 2019, 11:04 PM
imp accepted this revision.Oct 30 2019, 11:30 PM
This revision is now accepted and ready to land.Oct 30 2019, 11:30 PM
Closed by commit rS354227: Allow bsd.compat.mk to be reliably included outside Makefile.inc1. (authored by brooks). · Explain WhyOct 31 2019, 8:37 PM
This revision was automatically updated to reflect the committed changes.
brooks added a commit: rS354227: Allow bsd.compat.mk to be reliably included outside Makefile.inc1..

Revision Contents
Changeset List

PathSize
head/
share/
mk/
21 lines
34 lines
1 line

Diff 63845

View Options

head/share/mk/bsd.README

Loading...
View Options

head/share/mk/bsd.compat.mk

Loading...
View Options

head/share/mk/bsd.prog.mk

Loading...