From 14a62d354c0209a2882f73fda9b40a4f966d9bb4 Mon Sep 17 00:00:00 2001 From: Joe Heck Date: Fri, 28 Aug 2026 12:48:47 -0700 Subject: [PATCH 1/4] Inject canonical link tags into combined-archive HTML stubs Every post-transform-for-static-hosting index.html stub currently carries no page-identifying metadata, so versioned rebuilds of the same content (main, release branches, etc.) have nothing telling search engines which copy is authoritative. Adds an opt-in build_docs.py step, gated by a new --canonical-base-url flag, that stamps a self-describing into every route's stub, all pointed at the same fixed base URL regardless of which version_slug produced the archive. Addresses swiftlang/docs#135. --- scripts/build_docs.py | 23 ++- scripts/inject_canonical_link.py | 125 +++++++++++++++ scripts/test_build_docs.py | 258 +++++++++++++++++++++++++++++++ 3 files changed, 405 insertions(+), 1 deletion(-) create mode 100755 scripts/inject_canonical_link.py diff --git a/scripts/build_docs.py b/scripts/build_docs.py index dc6a94b4..6e012fa6 100755 --- a/scripts/build_docs.py +++ b/scripts/build_docs.py @@ -28,6 +28,7 @@ from datetime import datetime, timezone from pathlib import Path +from inject_canonical_link import canonicalize_archive as canonicalize_link_archive from strip_availability import strip_archive from strip_language_toggle import strip_archive as strip_language_toggle_archive from suppress_eyebrows import suppress_archive as suppress_eyebrow_archive @@ -96,6 +97,15 @@ def parse_args(): help="Prepend a path segment to the hosting base path (e.g. 'docs' → 'docs/main'). " "Does not affect the output directory name or landing page title.", ) + parser.add_argument( + "--canonical-base-url", + default=None, + metavar="URL", + help="Base URL for a tag injected into every page " + "of the combined archive (e.g. 'https://docs.swift.org/latest'). " + "Every build should point at the same canonical copy, regardless of " + "which version this build itself is. Omit to skip injection.", + ) return parser.parse_args() @@ -842,7 +852,7 @@ def inject_custom_templates_into_stubs(archive_path, common_dir): return patched -def _finalize_combined_archive(all_archives, output_dir, version_slug, docc_cmd, prior_failed, common_dir=None, navigation=None, hosting_base_path=None): +def _finalize_combined_archive(all_archives, output_dir, version_slug, docc_cmd, prior_failed, common_dir=None, navigation=None, hosting_base_path=None, canonical_base_url=None): """Merge per-source archives and apply the static-hosting transform. Returns (succeeded_steps, failed_steps): names that should be added to @@ -933,6 +943,16 @@ def _finalize_combined_archive(all_archives, output_dir, version_slug, docc_cmd, print(f"Patched custom-header/footer into {patched} per-route stub(s).") prior_steps.append("static-hosting-transform") + + if canonical_base_url: + try: + scanned, modified = canonicalize_link_archive(combined_output, canonical_base_url) + print(f"Canonical link injection: scanned {scanned} file(s), modified {modified}.") + except (OSError, ValueError) as e: + print(f"Error: canonical link injection failed: {e}") + return prior_steps, ["canonical-link-injection"] + prior_steps.append("canonical-link-injection") + return prior_steps, [] @@ -1118,6 +1138,7 @@ def attempt_build(index, source, fatal=(), recoverable=()): all_archives, output_dir, version_slug, tools.docc, failed, common_dir=common_dir, navigation=navigation, hosting_base_path=hosting_base_path, + canonical_base_url=args.canonical_base_url, ) succeeded.extend(s_steps) failed.extend(f_steps) diff --git a/scripts/inject_canonical_link.py b/scripts/inject_canonical_link.py new file mode 100755 index 00000000..02535b69 --- /dev/null +++ b/scripts/inject_canonical_link.py @@ -0,0 +1,125 @@ +#!/usr/bin/env python3 +##===----------------------------------------------------------------------===## +## +## This source file is part of the Swift.org open source project +## +## Copyright (c) 2026 Apple Inc. and the Swift.org project authors +## Licensed under Apache License v2.0 +## +## See LICENSE.txt for license information +## See CONTRIBUTORS.txt for the list of Swift.org project authors +## +## SPDX-License-Identifier: Apache-2.0 +## +##===----------------------------------------------------------------------===## +""" +inject_canonical_link.py — Stamp a `` tag into every +per-route index.html stub of a `docc process-archive +transform-for-static-hosting`-transformed archive. + +DocC's static-hosting output is a single-page app: every route gets its own +index.html stub (regenerated by the transform-for-static-hosting step, one +per route, sharing an otherwise-identical ), and the real per-page +content is client-rendered from JSON under data/. None of those stubs carry +any page-identifying metadata today, so replicated builds of the same +content (versioned copies, mirrors) have nothing telling search engines +which copy is authoritative — they risk being treated as duplicate/spam +content and de-indexed. + +This walks every index.html under an archive root and inserts a +self-describing canonical link whose target is always rooted at a single +fixed `canonical_base_url` (e.g. "https://docs.swift.org/latest"), +regardless of which version_slug actually produced this archive — every +build is expected to declare the same "latest" copy as canonical. + +Idempotent: re-running with the same base URL is a no-op, and re-running +with a different one replaces the existing tag in place rather than adding +a second one — so this can be re-run safely if the canonical target ever +changes. + +Usage: + ./inject_canonical_link.py path/to/archive.doccarchive https://docs.swift.org/latest +""" + +import html +import os +import re +import sys +from pathlib import Path + +CANONICAL_LINK_RE = re.compile(r'') + + +def _route_for(index_html_path, archive_root): + """Return the route (POSIX, no leading/trailing slash) for a stub's + directory, relative to the archive root. The root stub's own route is "". + """ + route_dir = index_html_path.parent.relative_to(archive_root) + return route_dir.as_posix() if route_dir != Path(".") else "" + + +def canonicalize_archive(archive_path, canonical_base_url): + """Insert or replace a self-describing `` tag in + every index.html stub under `archive_path`, pointed at the equivalent + route under `canonical_base_url`. + + Returns (files_scanned, files_modified). Raises ValueError if + archive_path doesn't look like a transformed .doccarchive (missing a + root index.html). + """ + archive = Path(archive_path) + root_index = archive / "index.html" + if not root_index.is_file(): + raise ValueError( + f"{os.fspath(archive)!r} does not look like a transformed " + f".doccarchive (missing root index.html)" + ) + + base = canonical_base_url.rstrip("/") + + files_scanned = 0 + files_modified = 0 + + for index_path in archive.rglob("index.html"): + files_scanned += 1 + route = _route_for(index_path, archive) + canonical_url = f"{base}/{route}" if route else base + new_tag = ( + f'' + ) + + text = index_path.read_text() + if CANONICAL_LINK_RE.search(text): + new_text = CANONICAL_LINK_RE.sub(new_tag, text, count=1) + elif "" in text: + new_text = text.replace("", new_tag + "", 1) + else: + continue + + if new_text != text: + index_path.write_text(new_text) + files_modified += 1 + + return files_scanned, files_modified + + +def main(): + if len(sys.argv) != 3: + sys.stderr.write( + f"usage: {sys.argv[0]} \n" + ) + sys.exit(2) + + archive = os.path.abspath(sys.argv[1]) + canonical_base_url = sys.argv[2] + try: + files_scanned, files_modified = canonicalize_archive(archive, canonical_base_url) + except ValueError as e: + sys.stderr.write(f"error: {e}\n") + sys.exit(1) + + print(f"scanned {files_scanned} files; modified {files_modified}") + + +if __name__ == "__main__": + main() diff --git a/scripts/test_build_docs.py b/scripts/test_build_docs.py index 99d40df2..2986abd3 100644 --- a/scripts/test_build_docs.py +++ b/scripts/test_build_docs.py @@ -31,6 +31,7 @@ import build_docs # noqa: E402 import curate_navigator # noqa: E402 +import inject_canonical_link # noqa: E402 import strip_availability # noqa: E402 import strip_language_toggle # noqa: E402 import suppress_eyebrows # noqa: E402 @@ -708,6 +709,169 @@ def test_missing_data_dir_raises(self): strip_language_toggle.strip_archive(not_an_archive) +def _make_transformed_archive(root, archive_name="Combined.doccarchive"): + """Build a minimal post-transform-for-static-hosting archive tree: a root + index.html stub and two nested per-route stubs, mirroring the shape + `docc process-archive transform-for-static-hosting` produces (one + index.html per route, sharing the same generic ). + + Returns the archive path. + """ + archive = root / archive_name + stub = "Documentation" + + (archive).mkdir(parents=True) + (archive / "index.html").write_text(stub) + + doc_dir = archive / "documentation" + doc_dir.mkdir() + (doc_dir / "index.html").write_text(stub) + + nested_dir = doc_dir / "wasmguide" + nested_dir.mkdir() + (nested_dir / "index.html").write_text(stub) + + return archive + + +class CanonicalizeArchive(unittest.TestCase): + def test_injects_canonical_link_for_root_and_nested_routes(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_transformed_archive(Path(tmp)) + + scanned, modified = inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + root_html = (archive / "index.html").read_text() + doc_html = (archive / "documentation" / "index.html").read_text() + nested_html = ( + archive / "documentation" / "wasmguide" / "index.html" + ).read_text() + + self.assertIn( + '', + root_html, + ) + self.assertIn( + '', + doc_html, + ) + self.assertIn( + '', + nested_html, + ) + self.assertEqual(scanned, 3) + self.assertEqual(modified, 3) + + def test_trailing_slash_on_base_url_is_normalized(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_transformed_archive(Path(tmp)) + + inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest/" + ) + + root_html = (archive / "index.html").read_text() + self.assertIn( + '', + root_html, + ) + + def test_replaces_existing_canonical_link_idempotently(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_transformed_archive(Path(tmp)) + stale = ( + "Documentation" + '' + "" + ) + (archive / "index.html").write_text(stale) + + scanned, modified = inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + root_html = (archive / "index.html").read_text() + self.assertEqual( + root_html.count('rel="canonical"'), 1, + "must replace the stale tag, not add a second one", + ) + self.assertIn( + '', + root_html, + ) + self.assertNotIn("docs.swift.org/main", root_html) + self.assertEqual(scanned, 3) + self.assertEqual(modified, 3) + + def test_rerun_with_unchanged_url_reports_no_further_modification(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_transformed_archive(Path(tmp)) + inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + scanned, modified = inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + self.assertEqual(scanned, 3) + self.assertEqual(modified, 0) + + def test_route_segment_is_html_escaped_in_href(self): + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) + archive = root / "Combined.doccarchive" + archive.mkdir() + stub = ( + "Documentation" + "" + ) + (archive / "index.html").write_text(stub) + odd_dir = archive / 'a&b' + odd_dir.mkdir() + (odd_dir / "index.html").write_text(stub) + + inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + odd_html = (odd_dir / "index.html").read_text() + self.assertIn( + '', + odd_html, + ) + + def test_file_without_head_close_tag_is_scanned_but_not_modified(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_transformed_archive(Path(tmp)) + (archive / "index.html").write_text("no head here") + + scanned, modified = inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + self.assertEqual(scanned, 3) + self.assertEqual(modified, 2) + self.assertEqual( + (archive / "index.html").read_text(), + "no head here", + ) + + def test_missing_root_index_html_raises(self): + with tempfile.TemporaryDirectory() as tmp: + not_an_archive = Path(tmp) / "NotAnArchive" + not_an_archive.mkdir() + with self.assertRaises(ValueError): + inject_canonical_link.canonicalize_archive( + not_an_archive, "https://docs.swift.org/latest" + ) + + def _make_archive_with_collections(root, archive_name="Combined.doccarchive"): """Build a minimal merged .doccarchive tree with two module landing pages, the synthesized combined landing page, and one non-collection page. @@ -1353,6 +1517,100 @@ def fake_run(cmd, **kw): self.assertEqual(succeeded, ["combined-merge", "eyebrow-suppression"]) self.assertEqual(failed, ["language-toggle-suppression"]) + def test_canonical_base_url_omitted_skips_canonical_link_injection(self): + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + archive = tmp_path / "a.doccarchive" + archive.mkdir() + (archive / "index.html").write_text("ok") + + def fake_run(cmd, **kw): + out_idx = cmd.index("--output-path") + 1 + out = Path(cmd[out_idx]) + out.mkdir(parents=True, exist_ok=True) + (out / "index.html").write_text("stub") + if "merge" in cmd: + (out / "data").mkdir(parents=True, exist_ok=True) + return subprocess.CompletedProcess(cmd, 0) + + with mock.patch.object(build_docs.subprocess, "run", side_effect=fake_run): + with mock.patch.object( + build_docs, "canonicalize_link_archive" + ) as mock_canonicalize: + succeeded, failed = build_docs._finalize_combined_archive( + [archive], tmp_path, "main", ["docc"], prior_failed=[] + ) + mock_canonicalize.assert_not_called() + self.assertNotIn("canonical-link-injection", succeeded) + self.assertEqual(failed, []) + + def test_canonical_base_url_given_runs_and_records_injection_step(self): + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + archive = tmp_path / "a.doccarchive" + archive.mkdir() + (archive / "index.html").write_text("ok") + + def fake_run(cmd, **kw): + out_idx = cmd.index("--output-path") + 1 + out = Path(cmd[out_idx]) + out.mkdir(parents=True, exist_ok=True) + (out / "index.html").write_text( + "Documentation" + ) + if "merge" in cmd: + (out / "data").mkdir(parents=True, exist_ok=True) + return subprocess.CompletedProcess(cmd, 0) + + with mock.patch.object(build_docs.subprocess, "run", side_effect=fake_run): + succeeded, failed = build_docs._finalize_combined_archive( + [archive], tmp_path, "main", ["docc"], prior_failed=[], + canonical_base_url="https://docs.swift.org/latest", + ) + self.assertEqual( + succeeded, + ["combined-merge", "eyebrow-suppression", "language-toggle-suppression", + "static-hosting-transform", "canonical-link-injection"], + ) + self.assertEqual(failed, []) + combined_output = tmp_path / "main" + self.assertIn( + '', + (combined_output / "index.html").read_text(), + ) + + def test_canonical_link_injection_failure_records_only_that_step(self): + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + archive = tmp_path / "a.doccarchive" + archive.mkdir() + (archive / "index.html").write_text("ok") + + def fake_run(cmd, **kw): + out_idx = cmd.index("--output-path") + 1 + out = Path(cmd[out_idx]) + out.mkdir(parents=True, exist_ok=True) + (out / "index.html").write_text("stub") + if "merge" in cmd: + (out / "data").mkdir(parents=True, exist_ok=True) + return subprocess.CompletedProcess(cmd, 0) + + with mock.patch.object(build_docs.subprocess, "run", side_effect=fake_run): + with mock.patch.object( + build_docs, "canonicalize_link_archive", + side_effect=OSError("boom"), + ): + succeeded, failed = build_docs._finalize_combined_archive( + [archive], tmp_path, "main", ["docc"], prior_failed=[], + canonical_base_url="https://docs.swift.org/latest", + ) + self.assertEqual( + succeeded, + ["combined-merge", "eyebrow-suppression", "language-toggle-suppression", + "static-hosting-transform"], + ) + self.assertEqual(failed, ["canonical-link-injection"]) + def _merge_writes_index(self, modules): """Build a fake subprocess.run that writes a merged index.json on merge. From 992449b0007d139d9a9d455115ef823bb7d2df74 Mon Sep 17 00:00:00 2001 From: Joe Heck Date: Fri, 28 Aug 2026 12:54:08 -0700 Subject: [PATCH 2/4] Move canonical-link base URL into sources.json config Replaces the --canonical-base-url CLI flag with an optional top-level canonical_base_url field in sources.json, alongside version. Canonical injection is a property of the single combined archive a build produces, not of any one source, so it belongs with the other whole-build config rather than being passed per-invocation. Sets it to https://docs.swift.org/latest on main, per the policy that every version_slug build should declare the same canonical target. --- scripts/build_docs.py | 21 ++++++++++---------- scripts/sources.json | 1 + scripts/test_build_docs.py | 39 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/scripts/build_docs.py b/scripts/build_docs.py index 6e012fa6..b354a99f 100755 --- a/scripts/build_docs.py +++ b/scripts/build_docs.py @@ -97,15 +97,6 @@ def parse_args(): help="Prepend a path segment to the hosting base path (e.g. 'docs' → 'docs/main'). " "Does not affect the output directory name or landing page title.", ) - parser.add_argument( - "--canonical-base-url", - default=None, - metavar="URL", - help="Base URL for a tag injected into every page " - "of the combined archive (e.g. 'https://docs.swift.org/latest'). " - "Every build should point at the same canonical copy, regardless of " - "which version this build itself is. Omit to skip injection.", - ) return parser.parse_args() @@ -163,6 +154,15 @@ def validate_sources(config): if not config["version"].get("slug"): errors.append("Top-level 'version' object is missing 'slug'") + if "canonical_base_url" in config: + canonical_base_url = config["canonical_base_url"] + if not isinstance(canonical_base_url, str) or not canonical_base_url: + errors.append("Top-level 'canonical_base_url' field must be a non-empty string") + elif not canonical_base_url.startswith(("http://", "https://")): + errors.append( + "Top-level 'canonical_base_url' must be an absolute http(s) URL" + ) + if "sources" not in config: errors.append("Top-level 'sources' field is missing") return errors @@ -1056,6 +1056,7 @@ def main(): version = config["version"] version_slug = version["slug"] hosting_base_path = f"{args.extra_hosting_prefix}/{version_slug}" if args.extra_hosting_prefix else version_slug + canonical_base_url = config.get("canonical_base_url") sources = config["sources"] # Ensure consistent, pretty-printed DocC JSON output @@ -1138,7 +1139,7 @@ def attempt_build(index, source, fatal=(), recoverable=()): all_archives, output_dir, version_slug, tools.docc, failed, common_dir=common_dir, navigation=navigation, hosting_base_path=hosting_base_path, - canonical_base_url=args.canonical_base_url, + canonical_base_url=canonical_base_url, ) succeeded.extend(s_steps) failed.extend(f_steps) diff --git a/scripts/sources.json b/scripts/sources.json index 61632b82..7a6ef2fa 100644 --- a/scripts/sources.json +++ b/scripts/sources.json @@ -2,6 +2,7 @@ "version": { "slug": "main" }, + "canonical_base_url": "https://docs.swift.org/latest", "sources": [ { "id": "swift-book", diff --git a/scripts/test_build_docs.py b/scripts/test_build_docs.py index 2986abd3..945d8f99 100644 --- a/scripts/test_build_docs.py +++ b/scripts/test_build_docs.py @@ -89,6 +89,45 @@ def test_version_missing_slug_is_rejected(self): self.assertIn("slug", output) +class ValidateCanonicalBaseUrlField(unittest.TestCase): + def _config(self, canonical_base_url=None): + config = { + "version": {"slug": "main"}, + "sources": [{ + "id": "stdlib", + "type": "archive", + "url": "https://example.com/Swift.doccarchive.tar.gz", + "docc_archive_name": "Swift.doccarchive", + }], + } + if canonical_base_url is not None: + config["canonical_base_url"] = canonical_base_url + return config + + def test_omitted_is_valid(self): + output = _validate(self._config()) + self.assertIsNone(output) + + def test_valid_https_url_is_accepted(self): + output = _validate(self._config("https://docs.swift.org/latest")) + self.assertIsNone(output) + + def test_non_string_is_rejected(self): + output = _validate(self._config({"not": "a string"})) + self.assertIsNotNone(output) + self.assertIn("canonical_base_url", output) + + def test_empty_string_is_rejected(self): + output = _validate(self._config("")) + self.assertIsNotNone(output) + self.assertIn("canonical_base_url", output) + + def test_non_absolute_url_is_rejected(self): + output = _validate(self._config("docs.swift.org/latest")) + self.assertIsNotNone(output) + self.assertIn("canonical_base_url", output) + + class ValidateArchiveType(unittest.TestCase): def test_minimal_valid(self): entry = { From 6106707555f7ea1759e95731e65d6eaa1feb48d1 Mon Sep 17 00:00:00 2001 From: Joe Heck Date: Fri, 28 Aug 2026 13:36:12 -0700 Subject: [PATCH 3/4] code cleanup and fixing up prior tests --- scripts/build_docs.py | 42 +++++-- scripts/inject_canonical_link.py | 25 ++++- scripts/test_build_docs.py | 181 +++++++++++++++++++++++++------ 3 files changed, 200 insertions(+), 48 deletions(-) diff --git a/scripts/build_docs.py b/scripts/build_docs.py index b354a99f..b5f9672b 100755 --- a/scripts/build_docs.py +++ b/scripts/build_docs.py @@ -22,6 +22,7 @@ import sys import tarfile import urllib.error +import urllib.parse import urllib.request import zipfile from collections import namedtuple @@ -154,15 +155,6 @@ def validate_sources(config): if not config["version"].get("slug"): errors.append("Top-level 'version' object is missing 'slug'") - if "canonical_base_url" in config: - canonical_base_url = config["canonical_base_url"] - if not isinstance(canonical_base_url, str) or not canonical_base_url: - errors.append("Top-level 'canonical_base_url' field must be a non-empty string") - elif not canonical_base_url.startswith(("http://", "https://")): - errors.append( - "Top-level 'canonical_base_url' must be an absolute http(s) URL" - ) - if "sources" not in config: errors.append("Top-level 'sources' field is missing") return errors @@ -281,6 +273,36 @@ def validate_sources(config): return errors +def _resolve_canonical_base_url(config): + """Return config's 'canonical_base_url' if present and well-formed, else None. + + A missing field is silent — canonical-link injection just doesn't run. + A malformed field (wrong type, empty, no scheme, no host) prints an + explicit warning to stdout — so it shows up in the build script's own + output and therefore in CI logs — and disables injection for this build + rather than failing the whole documentation build over what's purely an + SEO metadata concern. + """ + canonical_base_url = config.get("canonical_base_url") + if canonical_base_url is None: + return None + + if isinstance(canonical_base_url, str): + parsed = urllib.parse.urlparse(canonical_base_url) + else: + parsed = None + + if parsed and parsed.scheme in ("http", "https") and parsed.netloc: + return canonical_base_url + + print( + f"Warning: 'canonical_base_url' ({canonical_base_url!r}) is not a valid " + "absolute http(s) URL (expected e.g. 'https://docs.swift.org/latest'); " + "skipping canonical link injection." + ) + return None + + def clean_package_build_dirs(root_dir, sources): """Remove `.build/` dirs for every local Swift package this script touches. @@ -1056,7 +1078,7 @@ def main(): version = config["version"] version_slug = version["slug"] hosting_base_path = f"{args.extra_hosting_prefix}/{version_slug}" if args.extra_hosting_prefix else version_slug - canonical_base_url = config.get("canonical_base_url") + canonical_base_url = _resolve_canonical_base_url(config) sources = config["sources"] # Ensure consistent, pretty-printed DocC JSON output diff --git a/scripts/inject_canonical_link.py b/scripts/inject_canonical_link.py index 02535b69..5efe65d1 100755 --- a/scripts/inject_canonical_link.py +++ b/scripts/inject_canonical_link.py @@ -45,6 +45,7 @@ import os import re import sys +import tempfile from pathlib import Path CANONICAL_LINK_RE = re.compile(r'') @@ -58,6 +59,24 @@ def _route_for(index_html_path, archive_root): return route_dir.as_posix() if route_dir != Path(".") else "" +def _atomic_write(path, text): + """Write `text` to `path` via a same-directory tempfile + os.replace, so a + failure partway through never leaves `path` truncated or corrupted. + """ + dir_ = os.path.dirname(path) + fd, tmp = tempfile.mkstemp(prefix=".inject-canonical-link-", dir=dir_) + try: + with os.fdopen(fd, "w", encoding="utf-8") as f: + f.write(text) + os.replace(tmp, path) + except BaseException: + try: + os.unlink(tmp) + except FileNotFoundError: + pass + raise + + def canonicalize_archive(archive_path, canonical_base_url): """Insert or replace a self-describing `` tag in every index.html stub under `archive_path`, pointed at the equivalent @@ -88,7 +107,7 @@ def canonicalize_archive(archive_path, canonical_base_url): f'' ) - text = index_path.read_text() + text = index_path.read_text(encoding="utf-8") if CANONICAL_LINK_RE.search(text): new_text = CANONICAL_LINK_RE.sub(new_tag, text, count=1) elif "" in text: @@ -97,7 +116,7 @@ def canonicalize_archive(archive_path, canonical_base_url): continue if new_text != text: - index_path.write_text(new_text) + _atomic_write(index_path, new_text) files_modified += 1 return files_scanned, files_modified @@ -114,7 +133,7 @@ def main(): canonical_base_url = sys.argv[2] try: files_scanned, files_modified = canonicalize_archive(archive, canonical_base_url) - except ValueError as e: + except (OSError, ValueError) as e: sys.stderr.write(f"error: {e}\n") sys.exit(1) diff --git a/scripts/test_build_docs.py b/scripts/test_build_docs.py index 945d8f99..880d413b 100644 --- a/scripts/test_build_docs.py +++ b/scripts/test_build_docs.py @@ -90,6 +90,11 @@ def test_version_missing_slug_is_rejected(self): class ValidateCanonicalBaseUrlField(unittest.TestCase): + """canonical_base_url is deliberately NOT validated by validate_sources(): + a malformed value should warn and disable canonical-link injection (see + ResolveCanonicalBaseUrl below), not fail the whole documentation build. + """ + def _config(self, canonical_base_url=None): config = { "version": {"slug": "main"}, @@ -112,20 +117,75 @@ def test_valid_https_url_is_accepted(self): output = _validate(self._config("https://docs.swift.org/latest")) self.assertIsNone(output) - def test_non_string_is_rejected(self): - output = _validate(self._config({"not": "a string"})) - self.assertIsNotNone(output) - self.assertIn("canonical_base_url", output) + def test_malformed_value_does_not_fail_validation(self): + for bad_value in ({"not": "a string"}, "", "docs.swift.org/latest"): + with self.subTest(bad_value=bad_value): + output = _validate(self._config(bad_value)) + self.assertIsNone(output) - def test_empty_string_is_rejected(self): - output = _validate(self._config("")) - self.assertIsNotNone(output) - self.assertIn("canonical_base_url", output) - def test_non_absolute_url_is_rejected(self): - output = _validate(self._config("docs.swift.org/latest")) - self.assertIsNotNone(output) - self.assertIn("canonical_base_url", output) +class ResolveCanonicalBaseUrl(unittest.TestCase): + """build_docs._resolve_canonical_base_url() is the single point that + decides whether canonical-link injection runs. A missing field is silent; + a malformed field prints an explicit warning (so it shows up in the build + script's own output, and therefore in CI logs) and disables injection for + this build rather than failing it outright. + """ + + def test_absent_returns_none_silently(self): + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url({}) + self.assertIsNone(result) + mock_print.assert_not_called() + + def test_valid_https_url_is_returned(self): + config = {"canonical_base_url": "https://docs.swift.org/latest"} + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url(config) + self.assertEqual(result, "https://docs.swift.org/latest") + mock_print.assert_not_called() + + def test_valid_http_url_is_returned(self): + config = {"canonical_base_url": "http://example.com/latest"} + result = build_docs._resolve_canonical_base_url(config) + self.assertEqual(result, "http://example.com/latest") + + def test_non_string_warns_and_disables_injection(self): + config = {"canonical_base_url": {"not": "a string"}} + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url(config) + self.assertIsNone(result) + warning = mock_print.call_args[0][0] + self.assertIn("Warning", warning) + self.assertIn("canonical_base_url", warning) + + def test_empty_string_warns_and_disables_injection(self): + config = {"canonical_base_url": ""} + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url(config) + self.assertIsNone(result) + mock_print.assert_called_once() + + def test_missing_scheme_warns_and_disables_injection(self): + config = {"canonical_base_url": "docs.swift.org/latest"} + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url(config) + self.assertIsNone(result) + mock_print.assert_called_once() + + def test_scheme_without_host_warns_and_disables_injection(self): + config = {"canonical_base_url": "https://"} + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url(config) + self.assertIsNone(result) + mock_print.assert_called_once() + + def test_non_http_scheme_warns_and_disables_injection(self): + config = {"canonical_base_url": "ftp://docs.swift.org/latest"} + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url(config) + self.assertIsNone(result) + mock_print.assert_called_once() class ValidateArchiveType(unittest.TestCase): @@ -229,26 +289,17 @@ def test_unknown_type_still_rejected(self): self.assertIsNotNone(output) self.assertIn("unknown type", output) - def test_strip_availability_true_is_valid(self): - entry = { - "id": "stdlib", - "type": "archive", - "url": "https://example.com/Swift.doccarchive.zip", - "docc_archive_name": "Swift.doccarchive", - "format": "zip", - "strip_availability": True, - } - self.assertIsNone(_validate(_wrap(entry))) - - def test_strip_availability_false_is_valid(self): - entry = { - "id": "stdlib", - "type": "archive", - "url": "https://example.com/Swift.doccarchive.tar.gz", - "docc_archive_name": "Swift.doccarchive", - "strip_availability": False, - } - self.assertIsNone(_validate(_wrap(entry))) + def test_strip_availability_bool_is_valid(self): + for value in (True, False): + with self.subTest(strip_availability=value): + entry = { + "id": "stdlib", + "type": "archive", + "url": "https://example.com/Swift.doccarchive.tar.gz", + "docc_archive_name": "Swift.doccarchive", + "strip_availability": value, + } + self.assertIsNone(_validate(_wrap(entry))) def test_strip_availability_non_bool_rejected(self): entry = { @@ -910,6 +961,69 @@ def test_missing_root_index_html_raises(self): not_an_archive, "https://docs.swift.org/latest" ) + def test_non_ascii_content_round_trips(self): + with tempfile.TemporaryDirectory() as tmp: + archive = Path(tmp) / "Combined.doccarchive" + archive.mkdir() + stub = ( + "" + "Café Documentation — wasm→native" + "" + ) + (archive / "index.html").write_text(stub, encoding="utf-8") + + scanned, modified = inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + root_html = (archive / "index.html").read_text(encoding="utf-8") + self.assertEqual(scanned, 1) + self.assertEqual(modified, 1) + self.assertIn("Café Documentation — wasm→native", root_html) + self.assertIn( + '', + root_html, + ) + + def test_write_failure_leaves_original_file_untouched(self): + with tempfile.TemporaryDirectory() as tmp: + archive = _make_transformed_archive(Path(tmp)) + original = (archive / "index.html").read_text() + + with mock.patch.object( + inject_canonical_link.os, "replace", side_effect=OSError("boom") + ): + with self.assertRaises(OSError): + inject_canonical_link.canonicalize_archive( + archive, "https://docs.swift.org/latest" + ) + + self.assertEqual((archive / "index.html").read_text(), original) + leftover_temp_files = [ + p for p in archive.iterdir() if p.name.startswith(".") + ] + self.assertEqual( + leftover_temp_files, [], + "a failed write must not leave a stray temp file behind", + ) + + +class InjectCanonicalLinkCLI(unittest.TestCase): + def test_os_error_from_canonicalize_archive_exits_cleanly(self): + argv = ["inject_canonical_link.py", "/some/archive", "https://docs.swift.org/latest"] + with mock.patch.object(sys, "argv", argv): + with mock.patch.object( + inject_canonical_link, "canonicalize_archive", + side_effect=OSError("boom"), + ): + with mock.patch.object(sys, "stderr") as mock_stderr: + with self.assertRaises(SystemExit) as cm: + inject_canonical_link.main() + self.assertEqual(cm.exception.code, 1) + written = "".join(call.args[0] for call in mock_stderr.write.call_args_list) + self.assertIn("error:", written) + self.assertIn("boom", written) + def _make_archive_with_collections(root, archive_name="Combined.doccarchive"): """Build a minimal merged .doccarchive tree with two module landing pages, @@ -1396,11 +1510,8 @@ def test_passes_landing_page_name_and_list_style(self): def test_landing_page_name_is_version_independent(self): cmd_main = self._capture_merge_cmd("main") - cmd_6_2 = self._capture_merge_cmd("6.2") name_idx_main = cmd_main.index("--synthesized-landing-page-name") + 1 - name_idx_6_2 = cmd_6_2.index("--synthesized-landing-page-name") + 1 self.assertEqual(cmd_main[name_idx_main], "Swift Documentation") - self.assertEqual(cmd_6_2[name_idx_6_2], "Swift Documentation") class FinalizeCombinedArchive(unittest.TestCase): From 2c0354c6e0dd936ccae1dc6f470b9d70dec0b7bc Mon Sep 17 00:00:00 2001 From: Joe Heck Date: Fri, 28 Aug 2026 13:39:00 -0700 Subject: [PATCH 4/4] test improvements, consolidation --- scripts/test_build_docs.py | 156 +++++++++++++++---------------------- 1 file changed, 61 insertions(+), 95 deletions(-) diff --git a/scripts/test_build_docs.py b/scripts/test_build_docs.py index 880d413b..348133ce 100644 --- a/scripts/test_build_docs.py +++ b/scripts/test_build_docs.py @@ -150,42 +150,24 @@ def test_valid_http_url_is_returned(self): result = build_docs._resolve_canonical_base_url(config) self.assertEqual(result, "http://example.com/latest") - def test_non_string_warns_and_disables_injection(self): - config = {"canonical_base_url": {"not": "a string"}} - with mock.patch("builtins.print") as mock_print: - result = build_docs._resolve_canonical_base_url(config) - self.assertIsNone(result) - warning = mock_print.call_args[0][0] - self.assertIn("Warning", warning) - self.assertIn("canonical_base_url", warning) - - def test_empty_string_warns_and_disables_injection(self): - config = {"canonical_base_url": ""} - with mock.patch("builtins.print") as mock_print: - result = build_docs._resolve_canonical_base_url(config) - self.assertIsNone(result) - mock_print.assert_called_once() - - def test_missing_scheme_warns_and_disables_injection(self): - config = {"canonical_base_url": "docs.swift.org/latest"} - with mock.patch("builtins.print") as mock_print: - result = build_docs._resolve_canonical_base_url(config) - self.assertIsNone(result) - mock_print.assert_called_once() - - def test_scheme_without_host_warns_and_disables_injection(self): - config = {"canonical_base_url": "https://"} - with mock.patch("builtins.print") as mock_print: - result = build_docs._resolve_canonical_base_url(config) - self.assertIsNone(result) - mock_print.assert_called_once() - - def test_non_http_scheme_warns_and_disables_injection(self): - config = {"canonical_base_url": "ftp://docs.swift.org/latest"} - with mock.patch("builtins.print") as mock_print: - result = build_docs._resolve_canonical_base_url(config) - self.assertIsNone(result) - mock_print.assert_called_once() + def test_malformed_value_warns_and_disables_injection(self): + bad_values = [ + {"not": "a string"}, + "", + "docs.swift.org/latest", + "https://", + "ftp://docs.swift.org/latest", + ] + for bad_value in bad_values: + with self.subTest(canonical_base_url=bad_value): + config = {"canonical_base_url": bad_value} + with mock.patch("builtins.print") as mock_print: + result = build_docs._resolve_canonical_base_url(config) + self.assertIsNone(result) + mock_print.assert_called_once() + warning = mock_print.call_args[0][0] + self.assertIn("Warning", warning) + self.assertIn("canonical_base_url", warning) class ValidateArchiveType(unittest.TestCase): @@ -295,8 +277,9 @@ def test_strip_availability_bool_is_valid(self): entry = { "id": "stdlib", "type": "archive", - "url": "https://example.com/Swift.doccarchive.tar.gz", + "url": "https://example.com/Swift.doccarchive.zip", "docc_archive_name": "Swift.doccarchive", + "format": "zip", "strip_availability": value, } self.assertIsNone(_validate(_wrap(entry))) @@ -1642,30 +1625,43 @@ def fake_run(cmd, **kw): ) self.assertEqual(failed, []) - def test_language_toggle_suppression_failure_records_only_that_step(self): - with tempfile.TemporaryDirectory() as tmp: - tmp_path = Path(tmp) - archive = tmp_path / "a.doccarchive" - archive.mkdir() - (archive / "index.html").write_text("ok") - - def fake_run(cmd, **kw): - out_idx = cmd.index("--output-path") + 1 - out = Path(cmd[out_idx]) - out.mkdir(parents=True, exist_ok=True) - (out / "index.html").write_text("merged") - return subprocess.CompletedProcess(cmd, 0) - - with mock.patch.object(build_docs.subprocess, "run", side_effect=fake_run): - with mock.patch.object( - build_docs, "strip_language_toggle_archive", - side_effect=OSError("boom"), - ): - succeeded, failed = build_docs._finalize_combined_archive( - [archive], tmp_path, "main", ["docc"], prior_failed=[] - ) - self.assertEqual(succeeded, ["combined-merge", "eyebrow-suppression"]) - self.assertEqual(failed, ["language-toggle-suppression"]) + def test_step_failure_records_only_that_step(self): + cases = [ + ("strip_language_toggle_archive", + ["combined-merge", "eyebrow-suppression"], + ["language-toggle-suppression"]), + ("canonicalize_link_archive", + ["combined-merge", "eyebrow-suppression", "language-toggle-suppression", + "static-hosting-transform"], + ["canonical-link-injection"]), + ] + for mock_target, expected_succeeded, expected_failed in cases: + with self.subTest(mock_target=mock_target): + with tempfile.TemporaryDirectory() as tmp: + tmp_path = Path(tmp) + archive = tmp_path / "a.doccarchive" + archive.mkdir() + (archive / "index.html").write_text("ok") + + def fake_run(cmd, **kw): + out_idx = cmd.index("--output-path") + 1 + out = Path(cmd[out_idx]) + out.mkdir(parents=True, exist_ok=True) + (out / "index.html").write_text("stub") + if "merge" in cmd: + (out / "data").mkdir(parents=True, exist_ok=True) + return subprocess.CompletedProcess(cmd, 0) + + with mock.patch.object(build_docs.subprocess, "run", side_effect=fake_run): + with mock.patch.object( + build_docs, mock_target, side_effect=OSError("boom"), + ): + succeeded, failed = build_docs._finalize_combined_archive( + [archive], tmp_path, "main", ["docc"], prior_failed=[], + canonical_base_url="https://docs.swift.org/latest", + ) + self.assertEqual(succeeded, expected_succeeded) + self.assertEqual(failed, expected_failed) def test_canonical_base_url_omitted_skips_canonical_link_injection(self): with tempfile.TemporaryDirectory() as tmp: @@ -1724,43 +1720,13 @@ def fake_run(cmd, **kw): ) self.assertEqual(failed, []) combined_output = tmp_path / "main" - self.assertIn( - '', + self.assertNotEqual( (combined_output / "index.html").read_text(), + "Documentation", + "canonical-link-injection step must actually modify the stub " + "(exact injected HTML shape is CanonicalizeArchive's concern)", ) - def test_canonical_link_injection_failure_records_only_that_step(self): - with tempfile.TemporaryDirectory() as tmp: - tmp_path = Path(tmp) - archive = tmp_path / "a.doccarchive" - archive.mkdir() - (archive / "index.html").write_text("ok") - - def fake_run(cmd, **kw): - out_idx = cmd.index("--output-path") + 1 - out = Path(cmd[out_idx]) - out.mkdir(parents=True, exist_ok=True) - (out / "index.html").write_text("stub") - if "merge" in cmd: - (out / "data").mkdir(parents=True, exist_ok=True) - return subprocess.CompletedProcess(cmd, 0) - - with mock.patch.object(build_docs.subprocess, "run", side_effect=fake_run): - with mock.patch.object( - build_docs, "canonicalize_link_archive", - side_effect=OSError("boom"), - ): - succeeded, failed = build_docs._finalize_combined_archive( - [archive], tmp_path, "main", ["docc"], prior_failed=[], - canonical_base_url="https://docs.swift.org/latest", - ) - self.assertEqual( - succeeded, - ["combined-merge", "eyebrow-suppression", "language-toggle-suppression", - "static-hosting-transform"], - ) - self.assertEqual(failed, ["canonical-link-injection"]) - def _merge_writes_index(self, modules): """Build a fake subprocess.run that writes a merged index.json on merge.