Page MenuHomeFreeBSD

[RFC] build: introduce the notion of a build epoch
ClosedPublic

Authored by kevans on Aug 9 2025, 7:41 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 10, 3:56 AM
Unknown Object (File)
Fri, Oct 10, 3:56 AM
Unknown Object (File)
Fri, Oct 10, 3:56 AM
Unknown Object (File)
Fri, Oct 10, 3:56 AM
Unknown Object (File)
Thu, Oct 9, 10:44 PM
Unknown Object (File)
Tue, Sep 30, 2:13 AM
Unknown Object (File)
Sep 3 2025, 5:58 PM
Unknown Object (File)
Aug 31 2025, 8:19 AM
Subscribers

Details

Summary

Idea and file format shamelessly stolen from CheriBSD, but reimplemented
in terms of the standard FreeBSD build system. We'll use this in some
events that call for a deeper cleansing, typically reserved for
situations where the dependencies are too complicated to unwind. This
notably does not preclude us from doing separate cleansing of world for
specific src.conf(5) knob changes that would require a rebuild.

In the FreeBSD version, we either stamp the OBJTOP we're cleaning
(either bootstrap or the full OBJDIR) with the current epoch and bail
out for unstamped objdirs, or we compare and either rm -rf or
cleandir as necessary.

This is very much so a prototype for discussion, so pardon any
horrifying oversights.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kevans requested review of this revision.Aug 9 2025, 7:41 PM

Concept seems good, looks a bit overly complex in the impl, but that can be cleaned up

tools/build/depend-cleanup.sh
122

-z test too.

des added inline comments.
tools/build/depend-cleanup.sh
122

Not necessary as long as it's quoted.

Leaving out the ! for simplicity, [ -d $SRCTOP ] is unsafe because if SRCTOP is unset or null, it becomes [ -d ], which is always true because -d is not seen as a unary operator, but as a string which evaluates to false if null and true otherwise; but [ -d "$SRCTOP" ] becomes [ -d "" ] which is the unary “is a directory” operator with a null argument, which always evaluates to false.

Our test(1) manual page doesn't explain this very well, unfortunately, but POSIX is unambiguous (look for the paragraph beginning with “In the following list”)

202

Shouldn't this come before clean_world? I suppose it doesn't matter since clean_world isn't called until after extract_epoch is defined, but it still bothers me.

206

This entire function can be reduced to the following oneliner:

awk 'int($1) > 0 { epoch = $1 } END { print epoch }' "$@"
227

I would argue that if we don't have an epoch, we must clean.

235

I would add a very visible separator comment above this line

244

I would also argue, as I did on the call, that if the epoch goes backward, we should simply clean.

tools/build/depend-cleanup.sh
122

Yes. Once upon a time, "" -> "." which is always a directory, at least as far as the kernel is concerned. V7 test just did a stat of the arg. V7 kernel would see this as an error, but at some point this became ".". I didn't take the time to deep dive to see when (since I thought the v7 nami did this, but I just checked and I was wrong).

tl;dr: This is one of those edge cases that burned me years ago and I still have the scar.

tools/build/depend-cleanup.sh
244

What scenario are you picturing where the epoch goes backward that isn't due to someone screwing up the epoch file? If it's from a different branch, ideally we clean for that as an independent fact apart from the epoch rather than relying on not accidentally having equivalent epochs at some point.

tools/build/depend-cleanup.sh
244

Hmm, bisect across an epoch

kevans marked 5 inline comments as done.

Incorporate review feedback

tools/build/depend-cleanup.sh
174

I would just do [ -s "$1" ] || return but maybe that's a style violation?

244

Yep, or rolling back a local change that you were testing, or any other number of scenarios.

kevans added inline comments.
tools/build/depend-cleanup.sh
174

This was what I had originally and it failed, but then it just hit me that I was being silly; just need to explicitly return 0 to avoid propagating the exit status of [. Fixed locally as suggested.

des added inline comments.
tools/build/depend-cleanup.sh
225

nit: ${objepoch:-unknown}

This revision is now accepted and ready to land.Aug 10 2025, 12:55 AM
brooks added inline comments.
tools/build/depend-cleanup.sh
122

I'd be tempted to do

if [ -z "$SRCTOP" -o  ! -f "$SRCTOP/Makefile.inc1" ]; then
kevans added inline comments.
tools/build/depend-cleanup.sh
122

Incorporated locally, with message changed to: "$SRCTOP: Not the root of a src tree"

Do we have a style guide for shell scripts? If we don't, we should.

In D51848#1185145, @des wrote:

Do we have a style guide for shell scripts? If we don't, we should.

Not that I'm aware of, but +1.

This revision was automatically updated to reflect the committed changes.