From a098cabce7dd467f6e94ae0210628ed865e224e3 Mon Sep 17 00:00:00 2001 From: Joe Heck Date: Thu, 3 Sep 2026 13:30:01 -0700 Subject: [PATCH 1/3] tuning availability data for swift-testing --- .gitignore | 1 + scripts/build_docs.py | 20 +++- scripts/sources.json | 3 +- scripts/strip_availability.py | 61 +++++++++- scripts/test_build_docs.py | 217 ++++++++++++++++++++++++++++++++++ 5 files changed, 295 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 184eff813..ed340052f 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ Package.resolved **/*.docc/header.html **/*.docc/footer.html scripts/__pycache__/ +scripts/.coverage diff --git a/scripts/build_docs.py b/scripts/build_docs.py index b5f9672ba..dc3afabb8 100755 --- a/scripts/build_docs.py +++ b/scripts/build_docs.py @@ -30,7 +30,7 @@ from pathlib import Path from inject_canonical_link import canonicalize_archive as canonicalize_link_archive -from strip_availability import strip_archive +from strip_availability import strip_archive, strip_linux_availability from strip_language_toggle import strip_archive as strip_language_toggle_archive from suppress_eyebrows import suppress_archive as suppress_eyebrow_archive from curate_navigator import ( @@ -218,6 +218,7 @@ def validate_sources(config): disallowed_for_archive = ( "targets", "docc_catalog", "path", "repo", "ref", "preflight", "add_docc_plugin", "extra_flags", "env", + "strip_linux_availability", ) for field in disallowed_for_archive: if field in entry: @@ -247,6 +248,14 @@ def validate_sources(config): "'archive' (only allowed on archive sources)" ) + if "strip_linux_availability" in entry and not isinstance( + entry["strip_linux_availability"], bool + ): + errors.append( + f"{label} 'strip_linux_availability' must be a boolean " + f"(got {type(entry['strip_linux_availability']).__name__})" + ) + if entry.get("add_docc_plugin") and entry_type != "git": errors.append(f"{label} has 'add_docc_plugin' but is not type 'git'") @@ -766,6 +775,15 @@ def build_source(source, root_dir, workspace, common_dir, temp_archive_dir, docc source, source_dir, common_dir, temp_archive_dir, docc_cmd, env ) + if source.get("strip_linux_availability"): + for archive in archives: + print(f"Stripping Linux availability from {archive}...") + scanned, modified, removed = strip_linux_availability(archive) + print( + f" scanned {scanned} files; modified {modified}; " + f"removed {removed} Linux platform entries" + ) + configured_ref = source["ref"] if source_type == "git" else None actual_ref, commit_sha = _collect_git_metadata(source_dir, configured_ref) diff --git a/scripts/sources.json b/scripts/sources.json index 7a6ef2fad..38a2e97aa 100644 --- a/scripts/sources.json +++ b/scripts/sources.json @@ -48,7 +48,8 @@ "repo": "https://github.com/swiftlang/swift-testing.git", "ref": "main", "targets": ["Testing"], - "add_docc_plugin": true + "add_docc_plugin": true, + "strip_linux_availability": true }, { "id": "docc-documentation", diff --git a/scripts/strip_availability.py b/scripts/strip_availability.py index f0f30841e..73fd2066c 100644 --- a/scripts/strip_availability.py +++ b/scripts/strip_availability.py @@ -38,6 +38,7 @@ import tempfile TARGET_KEY = "platforms" +LINUX_PLATFORM_NAME = "Linux" def strip(node): @@ -55,11 +56,48 @@ def strip(node): return removed -def process_file(path): +def _is_linux_entry(entry): + if isinstance(entry, str): + return entry == LINUX_PLATFORM_NAME + if isinstance(entry, dict): + return entry.get("name") == LINUX_PLATFORM_NAME + return False + + +def strip_linux(node): + """Recursively remove only the Linux entry from every TARGET_KEY list. + + Unlike strip(), this leaves "platforms" and its other entries (Swift, + Xcode, real Apple-platform badges) intact -- it only removes the Linux + entry DocC synthesizes from the build host's target triple when a + package target's symbol graph is extracted on a Linux toolchain, as + swift-testing's is, since the combined build runs inside a Linux + container. + """ + removed = 0 + if isinstance(node, dict): + if TARGET_KEY in node and isinstance(node[TARGET_KEY], list): + before = node[TARGET_KEY] + after = [entry for entry in before if not _is_linux_entry(entry)] + removed += len(before) - len(after) + if len(after) != len(before): + if after: + node[TARGET_KEY] = after + else: + del node[TARGET_KEY] + for v in node.values(): + removed += strip_linux(v) + elif isinstance(node, list): + for v in node: + removed += strip_linux(v) + return removed + + +def process_file(path, strip_fn=strip): with open(path, "rb") as f: data = json.load(f) - removed = strip(data) + removed = strip_fn(data) if removed == 0: return 0 @@ -97,8 +135,12 @@ def main(): ) -def strip_archive(archive_path): - """Strip every 'platforms' key from JSON files under /data/. +def strip_archive(archive_path, strip_fn=strip): + """Strip 'platforms' data from JSON files under /data/. + + By default deletes every 'platforms' key outright (strip_fn=strip). Pass + strip_fn=strip_linux to remove only the Linux entry from each list + instead, leaving the rest of the platform data in place. Returns (files_scanned, files_modified, keys_removed). Raises ValueError if archive_path doesn't look like a .doccarchive @@ -123,7 +165,7 @@ def strip_archive(archive_path): path = os.path.join(root, name) files_scanned += 1 try: - removed = process_file(path) + removed = process_file(path, strip_fn) except json.JSONDecodeError as e: sys.stderr.write(f"skip (invalid JSON): {path}: {e}\n") continue @@ -134,5 +176,14 @@ def strip_archive(archive_path): return files_scanned, files_modified, keys_removed +def strip_linux_availability(archive_path): + """Remove only the Linux entry from every 'platforms' list in an archive. + + Thin wrapper around strip_archive() using strip_linux() -- see its + docstring for why this is narrower than the default full wipe. + """ + return strip_archive(archive_path, strip_fn=strip_linux) + + if __name__ == "__main__": main() \ No newline at end of file diff --git a/scripts/test_build_docs.py b/scripts/test_build_docs.py index 348133ce1..ea2614348 100644 --- a/scripts/test_build_docs.py +++ b/scripts/test_build_docs.py @@ -324,6 +324,56 @@ def test_rejected_on_local_source(self): self.assertIn("strip_availability", output) +class ValidateStripLinuxAvailability(unittest.TestCase): + def test_bool_is_valid_on_git(self): + for value in (True, False): + with self.subTest(strip_linux_availability=value): + entry = { + "id": "swift-testing", + "type": "git", + "repo": "https://github.com/swiftlang/swift-testing.git", + "ref": "main", + "targets": ["Testing"], + "strip_linux_availability": value, + } + self.assertIsNone(_validate(_wrap(entry))) + + def test_bool_is_valid_on_local(self): + entry = { + "id": "api-guidelines", + "type": "local", + "path": "api-guidelines", + "targets": ["APIGuidelines"], + "strip_linux_availability": True, + } + self.assertIsNone(_validate(_wrap(entry))) + + def test_non_bool_rejected(self): + entry = { + "id": "swift-testing", + "type": "git", + "repo": "https://github.com/swiftlang/swift-testing.git", + "ref": "main", + "targets": ["Testing"], + "strip_linux_availability": "yes", + } + output = _validate(_wrap(entry)) + self.assertIsNotNone(output) + self.assertIn("strip_linux_availability", output) + + def test_rejected_on_archive_source(self): + entry = { + "id": "stdlib", + "type": "archive", + "url": "https://example.com/Swift.doccarchive.tar.gz", + "docc_archive_name": "Swift.doccarchive", + "strip_linux_availability": True, + } + output = _validate(_wrap(entry)) + self.assertIsNotNone(output) + self.assertIn("strip_linux_availability", output) + + class ValidateExistingTypesStillWork(unittest.TestCase): """Regression: make sure local and git validation is unchanged.""" @@ -609,6 +659,60 @@ def test_strip_availability_absent_leaves_platforms(self): self.assertIn("platforms", payload["metadata"]) +class BuildSourceGitBranchStripLinuxAvailability(unittest.TestCase): + def _build(self, tmp_path, archive, strip_linux_availability): + source_dir = tmp_path / "checkout" + source_dir.mkdir() + source = { + "id": "swift-testing", + "type": "git", + "repo": "https://github.com/swiftlang/swift-testing.git", + "ref": "main", + "targets": ["Testing"], + } + if strip_linux_availability is not None: + source["strip_linux_availability"] = strip_linux_availability + + with mock.patch("build_docs.clone_or_update", return_value=source_dir), \ + mock.patch("build_docs._build_package_targets", return_value=[archive]): + return build_docs.build_source( + source, + root_dir=tmp_path, + workspace=tmp_path / "workspace", + common_dir=tmp_path, + temp_archive_dir=tmp_path / "_archives", + docc_cmd=[], + env={}, + ) + + def test_strips_only_linux_entries_from_built_archive(self): + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + archive = _make_archive_with_linux_platform(tmp_path, "Testing.doccarchive") + + archives, _ = self._build(tmp_path, archive, True) + + payload = json.loads( + (archives[0] / "data" / "documentation" / "foosymbol.json").read_text() + ) + platform_names = {p["name"] for p in payload["metadata"]["platforms"]} + self.assertNotIn("Linux", platform_names) + self.assertEqual(platform_names, {"Swift", "Xcode"}) + + def test_absent_leaves_linux_entry(self): + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + archive = _make_archive_with_linux_platform(tmp_path, "Testing.doccarchive") + + archives, _ = self._build(tmp_path, archive, None) + + payload = json.loads( + (archives[0] / "data" / "documentation" / "foosymbol.json").read_text() + ) + platform_names = {p["name"] for p in payload["metadata"]["platforms"]} + self.assertIn("Linux", platform_names) + + def _make_archive_with_platforms(root, archive_name="Swift.doccarchive"): """Build a minimal .doccarchive tree on disk with platforms data sprinkled in. @@ -697,6 +801,119 @@ def test_missing_data_dir_raises(self): strip_availability.strip_archive(not_an_archive) +def _make_archive_with_linux_platform(root, archive_name="Testing.doccarchive"): + """Build a minimal .doccarchive tree with a synthesized Linux platform entry. + + Mirrors what DocC produces for a package target whose symbol graph was + extracted on a Linux toolchain: a "Linux" entry sits alongside the real + "Swift"/"Xcode" version markers in metadata.platforms, and a plain-string + "Linux" entry in a declaration's platforms list. strip_linux_availability() + should remove only those Linux entries and leave the rest untouched. + """ + archive = root / archive_name + data_dir = archive / "data" / "documentation" + data_dir.mkdir(parents=True) + + symbol = { + "metadata": { + "title": "FooSymbol", + "platforms": [ + {"name": "Linux", "introducedAt": "6.2"}, + {"name": "Swift", "introducedAt": "6.2"}, + {"name": "Xcode", "introducedAt": "26.0"}, + ], + }, + "primaryContentSections": [ + { + "kind": "declarations", + "declarations": [ + { + "tokens": [{"text": "func foo()"}], + "platforms": ["Linux"], + } + ], + } + ], + "kind": "symbol", + } + (data_dir / "foosymbol.json").write_text(json.dumps(symbol)) + + article = { + "metadata": {"title": "Article"}, + "kind": "article", + } + (data_dir / "article.json").write_text(json.dumps(article)) + + return archive + + +class StripLinuxAvailability(unittest.TestCase): + def test_removes_only_linux_entries_and_returns_counts(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_archive_with_linux_platform(Path(tmp)) + scanned, modified, removed = strip_availability.strip_linux_availability( + archive + ) + + symbol = json.loads( + (archive / "data" / "documentation" / "foosymbol.json").read_text() + ) + platform_names = {p["name"] for p in symbol["metadata"]["platforms"]} + self.assertNotIn("Linux", platform_names) + self.assertEqual(platform_names, {"Swift", "Xcode"}) + self.assertNotIn( + "platforms", symbol["primaryContentSections"][0]["declarations"][0] + ) + # Unrelated keys preserved. + self.assertEqual(symbol["metadata"]["title"], "FooSymbol") + self.assertEqual( + symbol["primaryContentSections"][0]["declarations"][0]["tokens"], + [{"text": "func foo()"}], + ) + + self.assertEqual(scanned, 2) + self.assertEqual(modified, 1) + self.assertEqual(removed, 2) + + def test_leaves_non_linux_declaration_platforms_alone(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_archive_with_platforms(Path(tmp)) + scanned, modified, removed = strip_availability.strip_linux_availability( + archive + ) + + symbol = json.loads( + (archive / "data" / "documentation" / "foosymbol.json").read_text() + ) + self.assertEqual( + symbol["metadata"]["platforms"], [{"name": "iOS", "introducedAt": "13.0"}] + ) + self.assertEqual( + symbol["primaryContentSections"][0]["declarations"][0]["platforms"], + ["macOS"], + ) + self.assertEqual(modified, 0) + self.assertEqual(removed, 0) + + def test_archive_without_platforms_is_unchanged(self): + with tempfile.TemporaryDirectory() as tmp: + archive = Path(tmp) / "Testing.doccarchive" + data_dir = archive / "data" + data_dir.mkdir(parents=True) + payload = {"metadata": {"title": "X"}, "kind": "article"} + (data_dir / "x.json").write_text(json.dumps(payload)) + + scanned, modified, removed = strip_availability.strip_linux_availability( + archive + ) + self.assertEqual(scanned, 1) + self.assertEqual(modified, 0) + self.assertEqual(removed, 0) + self.assertEqual( + json.loads((data_dir / "x.json").read_text()), payload + ) + + def _make_archive_with_language_variants(root, archive_name="Swift.doccarchive"): """Build a minimal .doccarchive tree with a module page carrying the top-level `variants` array that drives the "Language: Swift" nav pill, From cabcbed44a3b6d67fc0b04bcacacffc0653a4def Mon Sep 17 00:00:00 2001 From: Joe Heck Date: Thu, 3 Sep 2026 13:57:01 -0700 Subject: [PATCH 2/3] cleaning up doc comments about strip functions --- scripts/strip_availability.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/scripts/strip_availability.py b/scripts/strip_availability.py index 73fd2066c..55ef406ba 100644 --- a/scripts/strip_availability.py +++ b/scripts/strip_availability.py @@ -15,14 +15,13 @@ """ strip-availability.py — Remove platform availability data from a DocC archive. -Walks every JSON file under /data/ and deletes any "platforms" key it -finds. In a Swift.doccarchive these only appear in two places: +Walks every JSON file under /data/ and deletes or modifies any "platforms" +key it finds. In a Swift.doccarchive these only appear in two places: - - metadata.platforms (the iOS/macOS/... badge table - on each symbol page) + - metadata.platforms + (the iOS/macOS/... badge table on each symbol page) - primaryContentSections[*].declarations[*].platforms - (per-declaration variant tag, - e.g. ["macOS"]) + (per-declaration variant tag, e.g. ["macOS"]) Both are populated by DocC at convert-time from the bundle's CDAppleDefaultAvailability Info.plist key plus any compiler-provided @@ -68,11 +67,8 @@ def strip_linux(node): """Recursively remove only the Linux entry from every TARGET_KEY list. Unlike strip(), this leaves "platforms" and its other entries (Swift, - Xcode, real Apple-platform badges) intact -- it only removes the Linux - entry DocC synthesizes from the build host's target triple when a - package target's symbol graph is extracted on a Linux toolchain, as - swift-testing's is, since the combined build runs inside a Linux - container. + Xcode) intact -- it only removes the Linux entry DocC synthesizes from + the build host's target triple. """ removed = 0 if isinstance(node, dict): @@ -138,9 +134,7 @@ def main(): def strip_archive(archive_path, strip_fn=strip): """Strip 'platforms' data from JSON files under /data/. - By default deletes every 'platforms' key outright (strip_fn=strip). Pass - strip_fn=strip_linux to remove only the Linux entry from each list - instead, leaving the rest of the platform data in place. + By default deletes every 'platforms' key outright (strip_fn=strip). Returns (files_scanned, files_modified, keys_removed). Raises ValueError if archive_path doesn't look like a .doccarchive From 460a41203d38fa426d35ac8ff3c5ab100b72b83e Mon Sep 17 00:00:00 2001 From: Joe Heck Date: Thu, 3 Sep 2026 16:47:46 -0700 Subject: [PATCH 3/3] build docs workflow: check out PR head for matching-branch matrix job The main-slug matrix entry hardcoded ref: main, so PR builds always rebuilt the current main branch instead of the PR's own commits. --- .github/workflows/build_documentation.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build_documentation.yml b/.github/workflows/build_documentation.yml index 7a86ae2c8..102850c61 100644 --- a/.github/workflows/build_documentation.yml +++ b/.github/workflows/build_documentation.yml @@ -31,7 +31,10 @@ jobs: steps: - uses: actions/checkout@v7 with: - ref: ${{ matrix.ref }} + ref: >- + ${{ (github.event_name == 'pull_request' && matrix.ref == github.event.pull_request.base.ref) + && github.event.pull_request.head.sha + || matrix.ref }} # - uses: actions/cache@v6 # with: