Page MenuHomeFreeBSD
Differential D16815

Allow building world without inheriting $PATH
ClosedPublic
Actions

Authored by arichardson on Aug 20 2018, 4:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 20, 1:04 PM
Unknown Object (File)
Sat, Apr 20, 1:04 PM
Unknown Object (File)
Sat, Apr 20, 1:04 PM
Unknown Object (File)
Sat, Apr 20, 1:04 PM
Unknown Object (File)
Sat, Apr 20, 1:04 PM
Unknown Object (File)
Sat, Apr 20, 1:04 PM
Unknown Object (File)
Sat, Apr 20, 1:04 PM
Unknown Object (File)
Sat, Apr 20, 1:04 PM
View All Files
Subscribers
None

Details

Reviewers
jhb
brooks
bdrewery
imp
emaste
Commits
rS340157: Allow building world without inheriting $PATH
Summary

Allow building world without inheriting $PATH

Inheriting $PATH during the build phase can cause the build to fail when
compiling on a different system due to missing build tools or incompatible
versions somewhere in $PATH. This has cause build failures for us before
due to the jenkins slaves still running FreeBSD 10.
Listing the tools we depend on explicitly instead of just using whatever
happens to be in $PATH allows us to check that we don't accidentally add a
new build dependency.

All tools that do no need to be bootstrapped will now be symlinked to
${WORLDTMP}/legacy/bin and during the build phase $PATH will only contain
${WORLDTMP}. There is also a new variable "BOOTSTRAP_ALL_TOOLS" which can
be set to force compiling almost all bootstrap tools instead of symlinking
them. This will not bootstrap tools such as cp,mv, etc. since they may be
used during the build and for those we should really only be using POSIX
compatible options.

Furthermore, this change is required in order to be able to build on
non-FreeBSD hosts. While the same binaries may exist on Linux/MacOS they
often accept different flags or produce incompatible output.

Test Plan

make tinderbox succeeds

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 20595
Build 20016: arc lint + arc unit

Event Timeline

arichardson created this revision.Aug 20 2018, 4:12 PM
Harbormaster completed remote builds in B18977: Diff 46985.Aug 20 2018, 4:12 PM
arichardson added a parent revision: D16814: Don't run cc --version during cleandir/obj stages.Aug 21 2018, 8:07 AM
arichardson updated this revision to Diff 47021.Aug 21 2018, 8:15 AM
arichardson edited the test plan for this revision. (Show Details)

added bzip2 to the required bootstrap tools

Harbormaster completed remote builds in B18996: Diff 47021.Aug 21 2018, 8:15 AM
jhb added a comment.Aug 21 2018, 9:01 AM

Just some minor nits. I think sanitizing $PATH to explicitly identify what build tools we need is good, certainly for cross-building from a non-FreeBSD host. This approach also seems to ensure that if we add a new build tool it will break on FreeBSD hosts right away forcing it to be handled (rather than only breaking on non-FreeBSD hosts).

Makefile.inc1
588

s/inhering/inheriting/ (there is an instance of that in the commit message as well)

671

Maybe a newline after this line?

2022

s/link/links/

2028

s/bootstrapping/BOOTSTRAPPING/?

2269

s/builing/building/

arichardson updated this revision to Diff 47024.Aug 21 2018, 9:50 AM
arichardson marked 5 inline comments as done.
arichardson edited the summary of this revision. (Show Details)

Fixed typos

moved gzip/bzip to compatible tools (we only compress/decompress files, and this should be compatible across platforms)

Harbormaster completed remote builds in B18999: Diff 47024.Aug 21 2018, 9:50 AM
arichardson mentioned this in D14317: Stop using unifdef to generate bsdxml.h.Aug 21 2018, 10:05 AM
arichardson updated this revision to Diff 47038.Aug 21 2018, 2:08 PM

Fix build of arm kernels in tinderbox by ensuring that cpp is in $PATH

Harbormaster completed remote builds in B19008: Diff 47038.Aug 21 2018, 2:08 PM
imp added a comment.Aug 21 2018, 2:21 PM

How does this interact with LOCAL_XTOOL_DIRS? Doesn't this totally break that feature?

arichardson added a comment.Aug 21 2018, 2:29 PM
In D16815#358208, @imp wrote:

How does this interact with LOCAL_XTOOL_DIRS? Doesn't this totally break that feature?

As far as I can tell LOCAL_XTOOL_DIRS is only added to the cross-tools list of subdirectories so it should just work (unless the build of LOCAL_XTOOL_DIRS depends on another tool that is not in $PATH)

imp added a comment.Aug 21 2018, 2:32 PM
In D16815#358212, @arichardson wrote:
In D16815#358208, @imp wrote:

How does this interact with LOCAL_XTOOL_DIRS? Doesn't this totally break that feature?

As far as I can tell LOCAL_XTOOL_DIRS is only added to the cross-tools list of subdirectories so it should just work (unless the build of LOCAL_XTOOL_DIRS depends on another tool that is not in $PATH)

A trivial test case would be nanobsd:

cd tools/tools/nanobsd/embedded ; sh ../nanobsd.sh -c qemu-amd64-uefi.cfg

which uses the feature to build mkimg.

arichardson added a comment.Aug 21 2018, 2:39 PM
In D16815#358213, @imp wrote:
In D16815#358212, @arichardson wrote:
In D16815#358208, @imp wrote:

How does this interact with LOCAL_XTOOL_DIRS? Doesn't this totally break that feature?

As far as I can tell LOCAL_XTOOL_DIRS is only added to the cross-tools list of subdirectories so it should just work (unless the build of LOCAL_XTOOL_DIRS depends on another tool that is not in $PATH)

A trivial test case would be nanobsd:

cd tools/tools/nanobsd/embedded ; sh ../nanobsd.sh -c qemu-amd64-uefi.cfg

which uses the feature to build mkimg.

Thanks, I'll try that and update the patch if necessary.

imp added a comment.Aug 21 2018, 2:40 PM
In D16815#358214, @arichardson wrote:
In D16815#358213, @imp wrote:
In D16815#358212, @arichardson wrote:
In D16815#358208, @imp wrote:

How does this interact with LOCAL_XTOOL_DIRS? Doesn't this totally break that feature?

As far as I can tell LOCAL_XTOOL_DIRS is only added to the cross-tools list of subdirectories so it should just work (unless the build of LOCAL_XTOOL_DIRS depends on another tool that is not in $PATH)

A trivial test case would be nanobsd:

cd tools/tools/nanobsd/embedded ; sh ../nanobsd.sh -c qemu-amd64-uefi.cfg

which uses the feature to build mkimg.

Thanks, I'll try that and update the patch if necessary.

You'll need the fix we talked about elsewhere for sys* vs sys too.

arichardson updated this revision to Diff 47083.Aug 22 2018, 10:03 AM

Add unifdef to build tools if MK_AMD!=no
Add the universe toolchain to $PATH

Harbormaster completed remote builds in B19033: Diff 47083.Aug 22 2018, 10:03 AM
arichardson updated this revision to Diff 47084.Aug 22 2018, 10:10 AM

unbreak delete-old step

Harbormaster completed remote builds in B19034: Diff 47084.Aug 22 2018, 10:10 AM
arichardson mentioned this in rS338190: Stop using unifdef to generate bsdxml.h.Aug 22 2018, 11:56 AM
arichardson mentioned this in rS338191: Use unifdef -x1 instead of ignoring the shell exit code.
arichardson updated this revision to Diff 47088.Aug 22 2018, 1:05 PM

Make nanobsd.sh work without $PATH in $TMPPATH

Harbormaster completed remote builds in B19037: Diff 47088.Aug 22 2018, 1:05 PM
imp added inline comments.Aug 22 2018, 9:02 PM
Makefile.inc1
2212

Picking a random spot to comment...

It looks like we have two lists of things. One with a path, and one without. That seems inefficient. Is there some way we can have only one list?

arichardson updated this revision to Diff 47157.Aug 23 2018, 9:15 AM
  • Use some bmake variable modifiers to avoid duplication
  • Fix typos in comments
Harbormaster completed remote builds in B19088: Diff 47157.Aug 23 2018, 9:15 AM
arichardson updated this revision to Diff 49796.Oct 30 2018, 2:43 PM
arichardson marked an inline comment as done.
  • rebased on HEAD
  • fixed an incorrect defined()
Harbormaster completed remote builds in B20513: Diff 49796.Oct 30 2018, 2:43 PM
arichardson mentioned this in D17779: Build kernel-toolchain before kernel during make universe.Oct 31 2018, 5:58 PM
arichardson edited the test plan for this revision. (Show Details)Nov 1 2018, 11:50 AM
arichardson retitled this revision from Allow building world without inhering $PATH to Allow building world without inheriting $PATH.
arichardson added a child revision: D17800: Build the elftoolchain libraries as part of bootstrap-tools.Nov 1 2018, 12:43 PM
arichardson updated this revision to Diff 49878.Nov 1 2018, 12:58 PM

Fix build of some kernels by adding "stat" to the required tools

Harbormaster completed remote builds in B20570: Diff 49878.Nov 1 2018, 12:58 PM
emaste added inline comments.Nov 1 2018, 8:39 PM
Makefile.inc1
584

Can we use the ${} var name instead of tmp/legacy in the comment, in case it changes in the future? Our path names for build tools etc. are somewhat obtuse and I hope we can clean them up in a subsequent commit.

2174

"for by"

brooks accepted this revision.Nov 1 2018, 10:51 PM

Module @emaste's comments on comments, I think this is ready to go.

This revision is now accepted and ready to land.Nov 1 2018, 10:51 PM
arichardson updated this revision to Diff 49932.Nov 2 2018, 1:39 PM

Fixed comments.

Unless anyone can see any outstanding issues, I will commit this either later today or tomorrow.

This revision now requires review to proceed.Nov 2 2018, 1:39 PM
Harbormaster completed remote builds in B20595: Diff 49932.Nov 2 2018, 1:39 PM
This revision was not accepted when it landed; it landed in state Needs Review.Nov 5 2018, 7:51 PM
Closed by commit rS340157: Allow building world without inheriting $PATH (authored by arichardson). · Explain Why
This revision was automatically updated to reflect the committed changes.
arichardson marked an inline comment as done.

Revision Contents
Changeset List

PathSize
5 lines
211 lines
tools/
build/
53 lines
tools/
nanobsd/
embedded/
2 lines

Diff 49932

View Options

Makefile

Loading...
View Options

Makefile.inc1

Loading...
View Options

tools/build/Makefile

Loading...
View Options

tools/tools/nanobsd/embedded/common

Loading...