Refuse a package that asks a server for more than its targetAbi claims - #367
Open
iderex wants to merge 1 commit into
Open
Refuse a package that asks a server for more than its targetAbi claims#367iderex wants to merge 1 commit into
iderex wants to merge 1 commit into
Conversation
`0.2.0.0` shipped compiled against `10.11.11` while `build.yaml` claimed `targetAbi: "10.11.0.0"`. An assembly's reference table is stamped with the versions it was compiled against, a reference above what the host carries does not bind, and a `10.11.0` server loads the archive, throws in `GetTypes()` and reports the plugin `NotSupported`. Nothing has to be called for that to happen, so every route was green while it went out. `abi-floor.yaml` compiles the source against the floor and says nothing about the artefact; `package.yaml` and `publish.yaml` built the artefact and never compared it with the claim. Two halves, and neither works without the other. The package moves to the floor. The `net9.0` target compiles against `10.11.0` rather than `10.11.11`, which is what claiming that floor already meant, so the lock file and every route move with it and none of them loses locked mode. The `net10.0` target is unchanged: `12.0.0` is not published, the release candidate it builds against stamps `12.0.0.0`, and that is exactly what `build-jf12.yaml` claims. The refusal holds it there. `scripts/check-package-abi.sh` reads the reference table of every assembly in a built package, through `tools/package-abi`, and refuses a `MediaBrowser.*` or `Jellyfin.*` reference above the claim, naming the assembly and both versions. It runs on both routes that produce a package and on every claimed line. A package it could not read - no assembly, an assembly it cannot open, no host reference at all - is refused rather than passed, because those three print the same nothing as a package that is within its claim. `scripts/prove-package-abi-refusals.sh` builds the plugin at the floor and at the next published version above it and drives both through the check, beside the cases where the answer could not be read and the two accepting directions a careless comparison gets wrong. The transcripts, including the run with the refusal deleted, are in the pull request body. Part of #360. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com>
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
1 participant
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 #360.
0.2.0.0shipped asking a10.11.0server for five assemblies at10.11.11.0whilebuild.yamlclaimedtargetAbi: "10.11.0.0". A reference above what the host carries does not bind, so that server loads the archive, throws inGetTypes()and reports the pluginNotSupported. Nothing has to be called for it to happen, and every route here was green while it went out:abi-floor.yamlcompiles the SOURCE against the floor and says nothing about the artefact, and the two routes that build the artefact never compared it with the claim.The ruling on this issue was that the package moves to the floor and the claim stands, because moving the claim would drop every server below
10.11.11in order to accommodate a packaging accident, silently. This is that, plus the refusal that holds it there. The two land together because the check would red the mainline on its own, which the issue body says.Half one: the package moves to the floor
The
net9.0target compiles against10.11.0rather than10.11.11. That is what claiming that floor already meant, and doing it in the project rather than in the two workflows means the lock file records the graph that ships and every route keeps locked mode.The reference table before and after, read with the reader this change adds:
at
f4f65a9, which is whatorigin/masterwas when I started:and on this head:
The
net10.0target is untouched, and that is a measurement rather than an omission.12.0.0is not published, the release candidate that line builds against stamps its assemblies12.0.0.0, andbuild-jf12.yamlclaims12.0.0.0. So that line already sits at its claim:Only the 10.11 line was the defect this issue names.
Half two: the refusal
scripts/check-package-abi.shreads the reference table of every assembly in a built package, throughtools/package-abi, and refuses aMediaBrowser.*orJellyfin.*reference above thetargetAbiof the packaging file the package was built from, naming the assembly, its version and the claim. It reaches no network, starts no server and needs no container.It runs on both routes that produce a package -
package.yamlper claimed line, andpublish.yamlbefore the bill of materials and before anything is uploaded, because a release that exists is never touched again and a tag cannot be spent twice.A set it could not read is refused rather than passed, which is the third done-when. An archive that does not unpack, a package with no assembly in it, an assembly the reader cannot open and an assembly with no host reference at all each print the same nothing as a package within its claim.
Watched refusing
scripts/prove-package-abi-refusals.shbuilds the plugin at the floorbuild.yamldeclares and at the next published release of that line, resolved from the feed rather than written down, and drives both through the check. Run on this head:And deleting the refusal turns the first of those green
The done-when asks for that directly. I deleted the block that compares the versions and left everything else, then put it back. The package is the one built one published version above its floor, at
10.11.2:with the comparison deleted, the same package and the same claim:
It does not only stop refusing, it prints a sentence that is false about the package in front of it, which is the failure the whole check exists against arriving one level up.
The suite, and the three readers this board runs over its own documents
777 on both targets, with the plugin compiled against the floor rather than against
10.11.11.That reader caught something rather than agreeing with me. Widening the comment above
JellyfinVersionmoved two line numbersdocs/storage.mdpastes out of the project file, and it named both. They are corrected to what the file now produces rather than deleted.The documents that said this was undecided
README.md,docs/compatibility.mdanddocs/operating.mdeach carried a sentence saying which of the two moves is open. The first two now say which way it went and that the answer does not reach a release that already exists;docs/operating.mddescribes the published release rather than the question, so it is unchanged. None of them claims the published0.2.0.0is repaired, because it is not: it carries the references it shipped with until a later release replaces it, which is #152's.docs/bridge.mdread a member list out of "the reference assemblies each target framework compiles against" and named10.11.11fornet9.0. That sentence would have become untrue, so the page now names the move and carries a reading of the same three members injellyfin.common10.11.0, stated as the weaker reading it is - a count of a name in the assembly rather than a signature.The means
Bash and a small .NET reader, which is the means this repository already uses for a check that judges an artefact: the packaging routes are shell steps, and reading an assembly's metadata tables needs a managed reader, so
tools/package-abiis the smallest surface that answers it. It is modelled ontools/full-disk-probedown to theDirectory.Build.propsthat stops inheritance, it references no package at all -System.Reflection.Metadatais in the shared framework - and it prints rather than judges, so the rule lives in one shell script a fixture harness can drive.What this does not claim
No server was started and nothing was installed. Every reading above is of a file in this repository or of an assembly built from it on this machine. That a package built against the floor now loads on a
10.11.0server is not measured here;release-install.yamlmeasures the published release, and the published release is unchanged.The two new contexts are not required.
the package check refuses what it says it refusesis a new check-run name and nothing requires it, for the same reason the six on #30 were not required; a red job here does not by itself hold a merge today.The proof builds twice and reaches the package feed. It is a job of its own in
package.yamlrather than a step in the matrix, so it costs one pair of builds per run rather than one pair per line.I watched no other guard on this board bite. The suite is named as passing, which is a different statement from a guard watched failing for the reason it names, and only the check this change adds was watched doing that.
Nobody else has read this. No second reader was available tonight, and the transcripts above stand in place of one.
The
Scope:lineIt named
.github/workflows/package.yaml,.github/workflows/publish.yamlandscripts/. The answer on this issue - that the package moves to the floor rather than the claim moving to the package - puts the project file and its lock files under it, and the documents that said the question was open follow from the same answer. I have widened the line on the issue to name what the work touches, the way the line on #30 was widened when an answer put a workflow change under it.