Skip to content

Commit 74a17fd

Browse files
lukasIOlukasIO
authored
fix(CI): gate changeset action on unmanaged crates (#1307)
1 parent 709dc98 commit 74a17fd

4 files changed

Lines changed: 144 additions & 13 deletions

File tree

‎.github/scripts/changeset_detect.py‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@
3535
"present": [[pkg, bump], ...], # bumps already in the changeset
3636
"missing": [pkg, ...], # required packages not yet covered
3737
"invalid": [{...}, ...], # bump lines in a format knope would ignore
38+
"unmanaged": [pkg, ...], # publishable crates missing from knope.toml
39+
"stale": [pkg, ...], # knope.toml entries with no workspace crate
3840
"changeset_content": str, # a changeset prefilled with `missing`
3941
}
4042
"""
@@ -204,6 +206,38 @@ def detect(meta, knope_packages, changed_files, present):
204206
}
205207

206208

209+
def is_publishable(pkg):
210+
"""True unless the crate opts out of publishing with `publish = false`.
211+
212+
`cargo metadata` encodes the manifest's `publish` field as: `null` (default,
213+
publishable to any registry), `[]` (`publish = false`), or a list of allowed
214+
registry names. Only the empty list means "never published".
215+
"""
216+
return pkg.get("publish") != []
217+
218+
219+
def reconcile_knope_config(meta, knope_packages):
220+
"""Find drift between the Cargo workspace and knope.toml.
221+
222+
Returns (unmanaged, stale):
223+
unmanaged publishable workspace crates absent from knope.toml. These
224+
silently escape changeset enforcement and are never released.
225+
Fix by adding a `[packages.<name>]` block, or by setting
226+
`publish = false` if the crate isn't meant to ship.
227+
stale knope.toml package names with no matching workspace crate
228+
(e.g. a renamed or removed crate leaving a dangling entry).
229+
230+
Note the rule is one-directional: publishable implies knope-managed, but a
231+
knope-managed crate may set `publish = false` (e.g. livekit-ffi / livekit-
232+
uniffi, which CI publishes via wrapper packages rather than `cargo publish`).
233+
"""
234+
workspace_names = {pkg["name"] for pkg in meta["packages"]}
235+
publishable = {pkg["name"] for pkg in meta["packages"] if is_publishable(pkg)}
236+
unmanaged = sorted(publishable - knope_packages)
237+
stale = sorted(knope_packages - workspace_names)
238+
return unmanaged, stale
239+
240+
207241
def load_cargo_metadata():
208242
"""Fetch workspace metadata (--no-deps avoids network access)."""
209243
return json.loads(subprocess.check_output(
@@ -227,12 +261,14 @@ def main():
227261
"direct": [], "downstream": [], "required": [],
228262
"present": sorted(present.items()), "missing": [],
229263
"invalid": invalid,
264+
"unmanaged": [], "stale": [],
230265
"changeset_content": "",
231266
})
232267
return # emit calls sys.exit, but guard against falling through
233268

234269
result = detect(meta, knope_packages, changed_files, present)
235270
result["invalid"] = invalid
271+
result["unmanaged"], result["stale"] = reconcile_knope_config(meta, knope_packages)
236272
emit(result)
237273

238274

‎.github/scripts/test_changeset_detect.py‎

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,28 @@
2929
import changeset_detect as cd
3030

3131

32-
def make_meta(packages, deps, workspace_root="/ws"):
32+
def make_meta(packages, deps, workspace_root="/ws", publish=None):
3333
"""Build a fake `cargo metadata` document.
3434
3535
packages: {name -> relative dir}
3636
deps: {name -> [dependency names]}
37+
publish: {name -> publish value}, mirroring cargo metadata's `publish`
38+
field (`[]` for `publish = false`, a list for restricted
39+
registries). Names omitted here get no `publish` key, i.e. the
40+
default publishable-to-any-registry state.
3741
"""
38-
return {
39-
"workspace_root": workspace_root,
40-
"packages": [
41-
{
42-
"name": name,
43-
"manifest_path": f"{workspace_root}/{rel}/Cargo.toml",
44-
"dependencies": [{"name": d} for d in deps.get(name, [])],
45-
}
46-
for name, rel in packages.items()
47-
],
48-
}
42+
publish = publish or {}
43+
pkgs = []
44+
for name, rel in packages.items():
45+
entry = {
46+
"name": name,
47+
"manifest_path": f"{workspace_root}/{rel}/Cargo.toml",
48+
"dependencies": [{"name": d} for d in deps.get(name, [])],
49+
}
50+
if name in publish:
51+
entry["publish"] = publish[name]
52+
pkgs.append(entry)
53+
return {"workspace_root": workspace_root, "packages": pkgs}
4954

5055

5156
# A small synthetic workspace:
@@ -220,6 +225,48 @@ def test_changeset_content_prefills_missing(self):
220225
self.assertNotIn("a-sys", r["changeset_content"])
221226

222227

228+
class TestReconcile(unittest.TestCase):
229+
def test_publishable_crate_missing_from_knope_is_unmanaged(self):
230+
# Both crates are publishable (no `publish` key); only `a` is in knope.
231+
meta = make_meta({"a": "a", "b": "b"}, {})
232+
unmanaged, stale = cd.reconcile_knope_config(meta, {"a"})
233+
self.assertEqual(unmanaged, ["b"])
234+
self.assertEqual(stale, [])
235+
236+
def test_publish_false_crate_not_required_in_knope(self):
237+
# An example crate (publish = false) need not be knope-managed.
238+
meta = make_meta({"a": "a", "ex": "examples/ex"}, {}, publish={"ex": []})
239+
unmanaged, stale = cd.reconcile_knope_config(meta, {"a"})
240+
self.assertEqual(unmanaged, [])
241+
self.assertEqual(stale, [])
242+
243+
def test_publish_false_crate_may_still_be_knope_managed(self):
244+
# e.g. livekit-ffi: publish = false but released via CI, so it's in
245+
# knope. This must not be flagged as stale.
246+
meta = make_meta({"ffi": "ffi"}, {}, publish={"ffi": []})
247+
unmanaged, stale = cd.reconcile_knope_config(meta, {"ffi"})
248+
self.assertEqual(unmanaged, [])
249+
self.assertEqual(stale, [])
250+
251+
def test_restricted_registry_is_still_publishable(self):
252+
meta = make_meta({"a": "a"}, {}, publish={"a": ["crates-io"]})
253+
unmanaged, stale = cd.reconcile_knope_config(meta, set())
254+
self.assertEqual(unmanaged, ["a"])
255+
self.assertEqual(stale, [])
256+
257+
def test_stale_knope_entry_with_no_matching_crate(self):
258+
meta = make_meta({"a": "a"}, {})
259+
unmanaged, stale = cd.reconcile_knope_config(meta, {"a", "ghost"})
260+
self.assertEqual(unmanaged, [])
261+
self.assertEqual(stale, ["ghost"])
262+
263+
def test_both_directions_at_once(self):
264+
meta = make_meta({"a": "a", "b": "b"}, {})
265+
unmanaged, stale = cd.reconcile_knope_config(meta, {"a", "ghost"})
266+
self.assertEqual(unmanaged, ["b"])
267+
self.assertEqual(stale, ["ghost"])
268+
269+
223270
class TestBuildChangesetContent(unittest.TestCase):
224271
def test_deterministic_with_explicit_metadata(self):
225272
content = cd.build_changeset_content(

‎.github/workflows/changeset-check.yml‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,46 @@ jobs:
126126
exit 1
127127
fi
128128
129+
# --- Gate: knope.toml must stay in sync with the workspace ---
130+
# A publishable crate missing from knope.toml silently escapes changeset
131+
# enforcement and is never released; a knope entry with no matching crate
132+
# is dangling config. Either is a release-pipeline bug, so fail hard until
133+
# the config is reconciled. This runs before the "no versioned packages"
134+
# early-out below because a brand-new unmanaged crate maps to no knope
135+
# package and would otherwise slip through as "nothing affected".
136+
NUM_UNMANAGED=$(echo "$DETECTION" | jq '.unmanaged | length')
137+
NUM_STALE=$(echo "$DETECTION" | jq '.stale | length')
138+
if [ "$NUM_UNMANAGED" != "0" ] || [ "$NUM_STALE" != "0" ]; then
139+
COMMENT_BODY=$(
140+
echo "$COMMENT_MARKER"
141+
echo "### knope.toml is out of sync with the workspace"
142+
echo ""
143+
if [ "$NUM_UNMANAGED" != "0" ]; then
144+
UNMANAGED_LIST=$(echo "$DETECTION" | jq -r '.unmanaged[] | "- `\(.)`"')
145+
echo "These publishable crates are **not** managed by knope, so they would never get a changeset requirement or a release:"
146+
echo ""
147+
echo "$UNMANAGED_LIST"
148+
echo ""
149+
echo "Fix each one by **either**:"
150+
echo "- adding a \`[packages.<name>]\` block to \`knope.toml\` (if it should be released), **or**"
151+
echo "- setting \`publish = false\` in its \`Cargo.toml\` (if it should not)."
152+
echo ""
153+
fi
154+
if [ "$NUM_STALE" != "0" ]; then
155+
STALE_LIST=$(echo "$DETECTION" | jq -r '.stale[] | "- `\(.)`"')
156+
echo "These \`knope.toml\` packages have no matching workspace crate (renamed or removed?):"
157+
echo ""
158+
echo "$STALE_LIST"
159+
echo ""
160+
echo "Remove the stale \`[packages.<name>]\` block(s) from \`knope.toml\`."
161+
echo ""
162+
fi
163+
)
164+
upsert_comment "$COMMENT_BODY" || echo "::warning::Could not post PR comment (this can happen for fork PRs with limited permissions)"
165+
echo "::error::knope.toml is out of sync with the workspace (unmanaged: ${NUM_UNMANAGED}, stale: ${NUM_STALE}). See the PR comment for details."
166+
exit 1
167+
fi
168+
129169
# --- If no versioned packages are affected, no changeset is required ---
130170
NUM_REQUIRED=$(echo "$DETECTION" | jq '.required | length')
131171
if [ "$NUM_REQUIRED" = "0" ]; then

‎knope.toml‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,4 +172,12 @@ versioned_files = [
172172
"Cargo.lock",
173173
{ path = "Cargo.toml", dependency = "device-info" },
174174
]
175-
changelog = "device-info/CHANGELOG.md"
175+
changelog = "device-info/CHANGELOG.md"
176+
177+
[packages.livekit-runtime]
178+
versioned_files = [
179+
"livekit-runtime/Cargo.toml",
180+
"Cargo.lock",
181+
{ path = "Cargo.toml", dependency = "livekit-runtime" },
182+
]
183+
changelog = "livekit-runtime/CHANGELOG.md"

0 commit comments

Comments
 (0)