Repository navigation
Conversation
The citation checker had a cache of its own. Its own directory, its own environment variable that appeared in no document, no way to look inside it, no way to empty it, nothing recording what any of it was, and it lived for the length of one CI job so every pull request fetched every cited file again. This replaces it with one cache, keyed by repository, tag, platform and configuration, with a record beside each entry saying where it came from and the sha256 of what arrived. The hash is checked on the way back out and an entry that no longer matches is deleted and refetched rather than used.
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.
The M1 checklist line is "the artifact cache keyed on repository, tag, platform and configuration". This is that.
What was there before
The citation checker had a cache. It held source files pulled out of
dotnet/runtimeanddotnet/roslyn, and it had all of the following problems at once. Its own directory, separate from anything else. Its own environment variable,XRAY_CITE_CACHE, which was written down in no document and so was findable only by reading the source. No way to look inside it. No way to empty it. Nothing recording what any of the files in it were or where they came from. And it lived for the length of one CI job, so every pull request fetched every cited file over again.It also believed whatever was sitting on disk. That is the part that matters, and it is the part this PR is mostly about.
The key
A name, and the four ways two things with the same name can differ.
dotnet/runtimeanyfor something that does not varyanyThe only thing in the cache today varies along two of those four. A source file is the same text whatever machine asked for it and whatever configuration that machine builds in, so it says
anytwice rather than leaving the axes out.The other two are there for what is coming, and putting them in now is a judgement call worth stating rather than burying. A checked and a release
libclrjit.so, from the same commit, on the same platform, have the same file name, roughly the same size, and are different programs. A cache that mixed those up would hand a lesson the wrong JIT, and the lesson would run, print output, and pin numbers that are correct for a runtime nobody was using. Nothing would go red. Adding an axis to a key after the fact means invalidating every cache anybody has, so the time to be right about the layout is while it holds four text files.A key becomes a path, one directory per part, file last. Anything that is not a letter, a digit, a dot, an underscore or a hyphen becomes a hyphen. A part that is empty, or
., or.., or an absolute path, is refused rather than escaped.Why every entry has a hash
The point of this is not speed.
Everything else in this repository is checked by regenerating it and comparing against what is committed. A cache exists precisely so that the thing does not get regenerated, which makes it the one set of files here that is read back and believed. So each entry has a small JSON file beside it recording the address it came from, its size, when it arrived and the sha256 of the bytes. The hash is checked on every read. An entry that no longer matches is deleted and fetched again, with a line saying so, rather than used.
This is not mainly about an attacker, although it covers one. It is about the ordinary way a cache goes wrong, which is that somebody edits a file in it to try something out, forgets, and then every run on that machine reads the edit for the next six months and nothing anywhere notices.
The commands
There were none before. There are five now.
xray cache clearempties it.xray cache keyexists so that a workflow does not type a key by hand and then disagree with the tool about what a key is. The layout version is on the front of it, so changing the layout abandons an old cache instead of restoring it into a tool that would read it wrong.Proving it works
xray cache --selftest, sixteen assertions, no network, so it runs anywhere.A cache that lost everything and refetched every time would pass a test that only checks the answers come out right, and would show up as nothing worse than a slow build. A cache that handed back the wrong entry would show up as a lesson with confident, wrong numbers. Neither of those goes red on its own, so the cases go at them directly.
The first two are the control. A cache that stored nothing would pass every case below them.
One decision that changed a test
The citation self test used to point at a cache directory of its own, so that running it would not touch a real one. It now uses the shared cache.
The reason is that until the pin lands there are no citations in this repository, so the four files that self test resolves are the only things anything here ever fetches. Leaving it pointed somewhere private would mean the cache is exercised by nothing at all until November. Its cases that expect a refusal are unaffected, because nothing stores a 404, so the network is still reached whatever is on disk.
That makes it possible to check the cache is doing its job rather than assume it. Running the citation self test twice and listing the cache in between shows the timestamps unchanged, so the second run read from disk rather than fetching again.
CI
A
cachejob for the self test, which is a new required context and needs adding to branch protection after the first run.The
citejob now restores and saves the cache between runs. The path and the key both come out of the tool rather than being typed into the workflow, and the key carries a hash ofpin.json, because what that job is allowed to fetch is exactly what the pin names. It ends withxray cache list, so the log says what was fetched and from where. A cache is a set of files a program wrote while nobody was looking, and the run that fills it is the run that ought to say what went in.Checked before pushing
Not in this PR
Nothing fetches a binary yet.
xray get E1, which would use this to pull a checked JIT, is the next thing that wants it and is deliberately separate. E1 is declared and detected today, but the tool still cannot go and get it for you.