Run every gate a second time on a machine that has never seen this - #38
Merged
Conversation
Every job in CI starts from a runner image with an SDK on it, a NuGet folder, a home directory the SDK has already written to and a package manager somebody has already used. Every machine this has been written on is the same, only more so. A reader has none of that and neither does a container, so a defect that only shows up without those things passes everywhere it is tried. There were three of them. A bare ubuntu:24.04 cannot start .NET at all. It aborts on "Couldn't find a valid ICU package installed on the system" before reaching any code here, and dotnet-install.sh says in its own help that it will not install dependencies for you. global.json names 10.0.100 and rolls forward, so every machine anybody had used resolved a 10.0.4xx and nothing here had ever run on the version the file actually names. The two feature bands disagree about the working directory of a file run with dotnet run, and four files were reading a path relative to it. Every process the build starts now gets XRAY_HERE, and the build refuses lesson code that hands a string literal to File, Directory or Path.Combine, with two self test cases to prove the rule goes red. The regeneration self test was copying a lesson without the two Directory.Build.props files above it, so it built its cases with the SDK's settings instead of this repository's. It has sixteen cases now where it had fourteen. One more thing turned up on the way. A lesson that failed to compile reported only the error stream, and the SDK puts "The build failed" there and the compiler's diagnostics on the output stream, so the report was a sentence with the reason thrown away. Both streams now. The cold job runs docs/probes/cold-start.sh on the four platforms, in a bare container with the checkout mounted read only on Linux and with everything inheritable emptied on macOS and Windows. It is the same script the writeup and the test fleet run, rather than a copy in a YAML file that would drift from the page within a month. docs/probes/cold-start.md has the numbers and one thing that is not explained: run the script twice at the same work directory and the second run fails, exactly alternating, for reasons that are written down as far as they are known. The script uses a directory nothing has ever used, which is the honest version of cold anyway.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the last box on M1: cold execution on four platforms, meaning a fresh machine with no caches and no local state.
Why
Every other job in CI starts from a runner image that already has an SDK, a NuGet folder, a home directory the SDK has written to, and a package manager somebody has used. Every machine this book has been written on is the same, only more so. A reader has none of that, and neither does a container, which is what the one click environment is going to be.
The failure this is looking for is not a slow first run. It is a build that passes on every machine it is tried on and is still wrong, because the thing that would have made it fail is sitting on all of those machines. That is not hypothetical. There were three of them.
What it found
A bare
ubuntu:24.04cannot start .NET at all. Not a lesson failing, the runtime aborting before reaching any code in this repository:.NET links against the system ICU on Linux and the image does not ship one.
dotnet-install.shsays in its own help that it does not resolve dependencies, and this is what that sentence means. None of the four hosted runners would ever have told us, because all four have it. The package is named after the ICU release rather than the distribution, so the script looks the name up instead of writinglibicu74down.global.jsonnames an SDK nothing here had ever run on. It says10.0.100withrollForward: latestFeature, so any machine that already has a10.0.4xxuses that, which is every developer machine and every hosted runner. A reader who installs with--jsonfile global.jsongets exactly10.0.100.Those two bands disagree about something four files were relying on.
dotnet run some/file.csdoes not promise the working directory of the file it runs: on 10.0.400 the child inherits the directory the SDK was started in, on 10.0.100 it can get the directory the file itself is in. A boss fight living inboss/and readinglesson.csis correct under one and looking one directory too deep under the other, and which one you got was decided by whichever SDK the machine had resolved. Nothing was wrong with the code and nothing was wrong with the test. They had only ever met on one of the two SDKs this repository accepts.The fix is to stop guessing. Every process the build starts is handed
XRAY_HERE, the absolute path of the directory the file belongs to, and lessons start from that:Writing that down is not enough, because the next person writes
File.ReadAllLines("lesson.cs")and it works on their machine. So the build refuses it. Every lesson file, every generator block and both files of every boss fight are scanned, and a call toFile,DirectoryorPath.Combinewhose first argument is a string literal fails the build with the file and line named. Two self test cases prove the rule goes red, one breaking a lesson and one breaking a boss fight.The regeneration self test was building with the wrong settings. It copies a lesson somewhere temporary, breaks the copy on purpose, and requires the build to refuse it. The copy took the lesson directory and nothing above it, and what is above it is two
Directory.Build.propsfiles. So the cases were compiled with the SDK's defaults rather than this repository's: different target framework rule, no invariant globalization, warnings not errors. It was testing a build that the build never does. It has sixteen cases now where it had fourteen.One thing found along the way
A lesson that failed to compile reported only its error stream, and the SDK puts "The build failed. Fix the build errors and run again." there while the compiler's actual diagnostics go to the output stream. So the report was a sentence with the reason discarded a moment earlier by the program printing it. It reports both streams now, and that is the only reason the
CS0006lines in the writeup exist.What this adds to CI
A
coldjob with a four way matrix. Nosetup-dotnetstep, on purpose, since that is the one step every other job starts with and the one thing a cold machine does not have.On Linux it runs in a bare
ubuntu:24.04with the checkout mounted read only, so the container has to install everything down toca-certificatesand cannot quietly write build output back into the checkout. Windows containers are a whole operating system image rather than a directory tree and the macOS runners cannot run containers at all, so on those two the cold part is an emptyHOME,DOTNET_ROOT, NuGet folder and artifact cache, and a path with nodotneton it.The job runs
docs/probes/cold-start.shandcold-start.ps1, the same scripts the writeup tells a reader to run and the same ones the test fleet ran. A copy of the steps kept in the YAML would drift from the page within a month.Four new required contexts, taking the branch protection count from 17 to 21.
Measured
Every gate passes on all four from nothing, on the SDK
global.jsonnames rather than the one a developer machine resolves. The gates are the six step build offline, the regeneration self test with its sixteen cases, the prose lint, the numbers gate, the assertion self test and the cache self test.ubuntu:24.04, checkout read onlyubuntu:24.04, checkout read onlyThe 394 s on Windows is a slow download of the SDK that afternoon and nothing else. The same machine did every other phase faster than the other three.
One thing that is not explained, and is written down as such
Run the script twice at the same work directory and the second run fails. Three lessons and a blueprint generator die with an analyzer assembly missing out of the packages folder, and the packages folder is empty on those runs.
It alternates exactly, and did so for thirteen runs in a row. It is keyed to the work directory path: four runs at four different paths all passed, then a second run at one of those paths failed. It is not the build servers, because the failing run had none running and disabling them changes nothing. It is not the home directory, the packages folder or the temporary directory, because all three live inside the work directory and are deleted with it. So there is state outside the work directory keyed by its path, and I did not find it.
The script therefore uses a directory nothing has ever used, with the process id in the name, and deletes it when the run passes. That is not a fix. It is the honest version of what cold means, and it makes the probe reliable, because a directory the machine has used before is not cold whatever is going on. The whole thing is in the writeup rather than quietly worked around, so the next person to see it does not have to find it again.
Checked before pushing
All six on all four platforms, cold, plus warm locally.