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

[kevans]

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.

[imp]

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]

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.

[imp]

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.

[kevans]

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.

[kevans]

tools/build/depend-cleanup.sh

244

Hmm, bisect across an epoch

[kevans]

Incorporate review feedback

[des]

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]

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]

tools/build/depend-cleanup.sh

225

nit: ${objepoch:-unknown}

[brooks]

tools/build/depend-cleanup.sh

122

I'd be tempted to do

if [ -z "$SRCTOP" -o  ! -f "$SRCTOP/Makefile.inc1" ]; then

[kevans]

tools/build/depend-cleanup.sh

122

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

[des]

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

[kevans]

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.