CMake: Streamline rebuilds #25440
CMake: Streamline rebuilds #25440
Conversation
github-actions
bot
added
the
👀 pr-needs-review
implicitfield
force-pushed
the
cmake-cleanup
branch
from
bbcd421
to
7d6042e
Compare
This is in preparation for installing LibC headers into the sysroot as symlinks rather than as plain copies.
This removes the need to have the install_libc_headers target, since LibC's headers will now get installed before anything gets built. By extension, this also prevents said target from re-running on every build.
This rule hasn't actually generated a `Prekernel` file since d068af8, so this wasn't doing anything other than causing this target to be re-ran all the time.
This also makes the Shell tests run on Lagom again, as they have been silently skipped ever since d46be35 (which moved the tests but didn't update the glob.)
implicitfield
force-pushed
the
cmake-cleanup
branch
from
7d6042e
to
9c053fe
Compare
|
@ADKaster Want to take a look? |
|
I love the effect of this PR! Commits 2 and 3 are obviously correct. Commit 1 might be correct, but it's the kind of thing that might subtly break. It relies on build tools following symlinks when deciding if the build is dirty. Ninja does do that as far as I know, but there was also a patch to teach it to stop doing it (which didn't land – see ninja-build/ninja#1186 and in particular ninja-build/ninja#1188 (comment)). So maybe that commit should be in a separate PR since it feels more risky? But I suppose we can also merge this and then undo the first commit if something does indeed break. I'll let this sit for another day or two in case @ADKaster (or someone else wants to commit). (I'd merge commits 2 and 3 immediately.) |
|
Good point re the symlinks. I guess we could use hardlinks instead, which might play a bit more predictably with build tools, but the main disadvantage with that is that hardlinks can't really cross filesystem boundaries, so having the |
github-actions
bot
removed
the
👀 pr-needs-review
|
Meant to comment on this after I had a glance a few days ago. No big comments to me, other than symlinks feeling a bit fragile. but less globs (or no globs at all) is a happy place to be |
Currently, performing a rebuild (without doing any actual changes) looks like this:
We can definitely do better than that, and with this PR we get:
Note that the fourth commit in this PR also fixes running
Shelltests on Lagom, which haven't ran since they were moved in d46be35, as the glob that was used for finding the tests was never updated.