-z test too.

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”)

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.

This entire function can be reduced to the following oneliner:

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

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

I would add a very visible separator comment above this line

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

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.

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.

Hmm, bisect across an epoch

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

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

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.

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