release: ship dx-windows-*-pdb.zip debug symbols with each Windows cell (part of #8428)

.github/workflows/release.yml

@@ -103,10 +103,20 @@ jobs:
             printf '%s  %s\n' "$(sha256sum "out/$archive.spdx.json" | cut -d' ' -f1)" "$archive.spdx.json" >> sums.tmp
             cat "out/$asset.sha256" "out/$archive.sha256" >> sums.tmp
             files="$files $asset $asset.sha256 $archive $archive.sha256 $archive.spdx.json"
+            case "$asset" in dx-windows-*)
+              pdb="$asset-pdb.zip"
+              for file in "$pdb" "$pdb.sha256" "$pdb.spdx.json"; do
+                test -s "out/$file" || { echo "want a non-empty out/$file" >&2; exit 1; }
+              done
+              printf '%s  %s\n' "$(sha256sum "out/$pdb.spdx.json" | cut -d' ' -f1)" "$pdb.spdx.json" >> sums.tmp
+              cat "out/$pdb.sha256" >> sums.tmp
+              files="$files $pdb $pdb.sha256 $pdb.spdx.json"
+              ;;
+            esac
           done
           printf '%s\n' "$files" > out/files.list
           LC_ALL=C sort -k2 sums.tmp > out/dx-sha256sums.txt
-          test "$(wc -l < out/dx-sha256sums.txt)" -eq 15 || { echo "want 15 manifest lines, got $(wc -l < out/dx-sha256sums.txt)" >&2; exit 1; }
+          test "$(wc -l < out/dx-sha256sums.txt)" -eq 19 || { echo "want 19 manifest lines, got $(wc -l < out/dx-sha256sums.txt)" >&2; exit 1; }
           (cd out && sha256sum -c dx-sha256sums.txt)
       - name: Build and verify the source archive
         env:
@@ -363,7 +373,7 @@ jobs:
           GH_TOKEN: ${{ github.token }}
         run: |
           set -euo pipefail
-          for archive in dx-linux-x86_64.tar.gz dx-linux-arm64.tar.gz dx-macos-arm64.tar.gz dx-windows-x86_64.zip dx-windows-arm64.zip; do
+          for archive in dx-linux-x86_64.tar.gz dx-linux-arm64.tar.gz dx-macos-arm64.tar.gz dx-windows-x86_64.zip dx-windows-arm64.zip dx-windows-x86_64-pdb.zip dx-windows-arm64-pdb.zip; do
             for file in "$archive" "$archive.spdx.json"; do
             gh attestation verify "out/$file" \
               --bundle out/dx-sha256sums.txt.sigstore.json \

cli/adopt/src/launcher.rs

@@ -48,11 +48,11 @@ impl LauncherPins {
             };
             let archive = RELEASE_ASSETS
                 .iter()
-                .any(|known| asset == archive_name(known));
+                .any(|known| asset == archive_name(known) || asset == symbols_name(known));
             let sbom = asset.strip_suffix(".spdx.json").is_some_and(|base| {
                 RELEASE_ASSETS
                     .iter()
-                    .any(|known| base == archive_name(known))
+                    .any(|known| base == archive_name(known) || base == symbols_name(known))
             });
             if archive || sbom {
                 if !seen_archives.insert(asset.to_owned()) {
@@ -228,6 +228,15 @@ fn archive_name(raw: &str) -> String {
     format!("{raw}{suffix}")
 }
 
+/// The debug symbols asset for a raw release asset: `<raw>-pdb.zip` on Windows, empty elsewhere.
+fn symbols_name(raw: &str) -> String {
+    if raw.contains("windows") {
+        format!("{raw}-pdb.zip")
+    } else {
+        String::new()
+    }
+}
+
 fn valid_pin(pin: &str) -> bool {
     pin.is_empty()
         || (pin.len() == 64 && pin.bytes().all(|b| matches!(b, b'0'..=b'9' | b'a'..=b'f')))
@@ -904,6 +913,20 @@ mod tests {
         assert!(LauncherPins::from_sha256sums(&unknown).is_err());
     }
 
+    #[test]
+    fn from_sha256sums_skips_symbols_lines() {
+        let a = "a".repeat(64);
+        let symbols = format!(
+            "{a}  dx-windows-x86_64-pdb.zip\n{a}  dx-windows-arm64-pdb.zip.spdx.json\n"
+        );
+        assert_eq!(
+            LauncherPins::from_sha256sums(&format!("{MANIFEST}{symbols}")).expect("combined"),
+            LauncherPins::from_sha256sums(MANIFEST).expect("manifest")
+        );
+        let linux = format!("{MANIFEST}{a}  dx-linux-x86_64-pdb.zip\n");
+        assert!(LauncherPins::from_sha256sums(&linux).is_err());
+    }
+
     #[test]
     fn from_sha256sums_skips_sbom_lines() {
         let a = "a".repeat(64);

cli/cli/BUILD.bazel

@@ -5,6 +5,7 @@ load("@rules_rust_prost//:defs.bzl", "rust_prost_library")
 load("//deploy/rules:github.bzl", "github_deploy")
 load("//quality:fixtures.bzl", "real_source_target")
 load("//rust/rules:defs.bzl", "rust_binary", "rust_clippy_test", "rust_library", "rust_test", "rustfmt_test")
+load("//rust/rules:wrapper_tests.bzl", "dx_msvc_opt_pdb_tests")
 load("//tools/visibility:visibility.bzl", "VIS_CLI")
 
 package(default_visibility = VIS_CLI)
@@ -188,6 +189,13 @@ filegroup(
     name = "dx_pdb",
     srcs = [":dx"],
     output_group = "pdb_file",
+    visibility = ["//visibility:public"],
+)
+
+dx_msvc_opt_pdb_tests(
+    name = "dx_opt_pdb_test",
+    expected = "dx_upstream.pdb",
+    target = ":dx",
 )
 
 github_deploy(

deploy/release/BUILD.bazel

@@ -216,12 +216,18 @@ rust_test(
         ":consumer_template",
         ":dx_host_cell",
         ":release_matrix",
-    ],
+    ] + select({
+        "@platforms//os:windows": [":dx_host_cell_pdb"],
+        "//conditions:default": [],
+    }),
     env = {
         "DX_CONSUMER_TEMPLATE": "$(rootpath consumer/consumer.BUILD.bazel)",
         "DX_HOST_CELL": "$(rootpath :dx_host_cell)",
         "DX_RELEASE_MATRIX": "$(rootpath :release_matrix)",
-    },
+    } | select({
+        "@platforms//os:windows": {"DX_HOST_SYMBOLS": "$(rootpath :dx_host_cell_pdb)"},
+        "//conditions:default": {},
+    }),
     deps = [
         ":dx_release_tools",
         "//deploy/rules:dx_deploy_tools",

deploy/release/host_cell_test.rs

@@ -43,6 +43,23 @@ fn host_cell_ships_exactly_its_members() {
     }
 }
 
+#[test]
+fn windows_host_cell_ships_its_symbols_zip() {
+    let Ok(rel) = std::env::var("DX_HOST_SYMBOLS") else {
+        return;
+    };
+    let (cell, _) = host_cell();
+    let symbols = dx_testing::resolve_runfiles(&rel);
+    assert_eq!(
+        symbols.file_name().and_then(|name| name.to_str()),
+        Some(format!("{}-pdb.zip", cell.name).as_str())
+    );
+    let listing = dx_deploy_tools::archive_listing(&symbols).expect("symbols listing");
+    let names: Vec<&str> = listing.iter().map(|(name, _, _)| name.as_str()).collect();
+    // dx.exe names the PDB of the upstream rust_binary (/pdbaltpath:%_PDB%).
+    assert_eq!(names, ["dx_upstream.pdb"]);
+}
+
 #[test]
 fn host_cell_notice_lists_shipped_crates_only() {
     let (_, archive) = host_cell();

deploy/release/matrix.bzl

@@ -54,6 +54,12 @@ def release_cell_archive(cell):
     """Returns the archive file name of one release cell."""
     return archive_filenames(cell["name"], cell["format"])[0]
 
+def release_cell_symbols(cell):
+    """Returns the debug symbols archive file name of one release cell, or None when it ships none."""
+    if cell["os"] != "windows":
+        return None
+    return cell["name"] + "-pdb.zip"
+
 def release_matrix_json():
     """Returns the release-matrix/v1 document for every cell."""
     cells = [
@@ -81,8 +87,8 @@ release_matrix_file = rule(
     doc = "Writes the release-matrix/v1 JSON for every release cell.",
 )
 
-def release_cells(name, cells = RELEASE_MATRIX, app = "//cli/cli:dx"):
-    """Declares one archive per release cell plus the `name` alias to the host cell archive."""
+def release_cells(name, cells = RELEASE_MATRIX, app = "//cli/cli:dx", pdb = "//cli/cli:dx_pdb"):
+    """Declares one archive per release cell, a symbols zip per Windows cell, and the `name` aliases for the host cell."""
     for cell in cells:
         cell_name = cell["name"]
         constraints = cell["constraints"]
@@ -120,6 +126,30 @@ def release_cells(name, cells = RELEASE_MATRIX, app = "//cli/cli:dx"):
             runtimes = (":" + cell_name + "_inventory_runtimes") if cell["runtime"] else None,
             target_compatible_with = constraints,
         )
+        symbols = release_cell_symbols(cell)
+        if symbols:
+            # The entry keeps the PDB basename that /pdbaltpath:%_PDB% writes into the exe.
+            native.genrule(
+                name = cell_name + "_pdb_archive",
+                srcs = [pdb],
+                outs = [symbols],
+                cmd = "$(execpath @bazel_tools//tools/zip:zipper) c $@ $$(basename $(location " + pdb + "))=$(location " + pdb + ")",
+                target_compatible_with = constraints,
+                tools = ["@bazel_tools//tools/zip:zipper"],
+            )
+            native.genrule(
+                name = cell_name + "_pdb_checksum",
+                srcs = [":" + cell_name + "_pdb_archive"],
+                outs = [symbols + ".sha256"],
+                cmd = "$(location //deploy/rules:hasher) $(location :" + cell_name + "_pdb_archive) $(OUTS)",
+                target_compatible_with = constraints,
+                tools = ["//deploy/rules:hasher"],
+            )
+            sbom_release(
+                name = cell_name + "_pdb_sbom",
+                artifact = ":" + cell_name + "_pdb_archive",
+                target_compatible_with = constraints,
+            )
         native.config_setting(
             name = cell_name + "_host",
             constraint_values = constraints,
@@ -142,3 +172,17 @@ def release_cells(name, cells = RELEASE_MATRIX, app = "//cli/cli:dx"):
             **{"//conditions:default": ":" + name + "_unsupported"}
         )),
     )
+    native.alias(
+        name = name + "_pdb",
+        actual = select(dict(
+            [(":" + cell["name"] + "_host", ":" + cell["name"] + "_pdb_archive") for cell in cells if release_cell_symbols(cell)],
+            **{"//conditions:default": ":" + name + "_unsupported"}
+        )),
+    )
+    native.alias(
+        name = name + "_pdb_sbom",
+        actual = select(dict(
+            [(":" + cell["name"] + "_host", ":" + cell["name"] + "_pdb_sbom_spdx") for cell in cells if release_cell_symbols(cell)],
+            **{"//conditions:default": ":" + name + "_unsupported"}
+        )),
+    )

deploy/release/matrix_tests.bzl

@@ -1,7 +1,7 @@
 """Unit tests for the release matrix."""
 
 load("//libs/starlark:defs.bzl", "expect_equal", "starlark_test")
-load(":matrix.bzl", "release_cell_archive", "release_matrix_cell", "release_matrix_error", "release_matrix_json", "release_matrix_names")
+load(":matrix.bzl", "release_cell_archive", "release_cell_symbols", "release_matrix_cell", "release_matrix_error", "release_matrix_json", "release_matrix_names")
 
 def _field(field):
     """Returns one field of every cell in matrix order."""
@@ -42,6 +42,11 @@ def matrix_unit_tests(name):
                 [release_cell_archive(release_matrix_cell(n)) for n in release_matrix_names()],
                 ["dx-linux-x86_64.tar.gz", "dx-linux-arm64.tar.gz", "dx-macos-arm64.tar.gz", "dx-windows-x86_64.zip", "dx-windows-arm64.zip"],
             ),
+            expect_equal(
+                "matrix ships a symbols zip for windows cells only",
+                [release_cell_symbols(release_matrix_cell(n)) for n in release_matrix_names()],
+                [None, None, None, "dx-windows-x86_64-pdb.zip", "dx-windows-arm64-pdb.zip"],
+            ),
             expect_equal(
                 "matrix unknown name has no cell",
                 release_matrix_cell("dx-plan9-mips"),

deploy/release/src/cell.rs

@@ -228,6 +228,13 @@ pub struct Consumer {
     pub checkout: PathBuf,
 }
 
+/// The debug symbols archive of a Windows cell and its independent rebuild.
+#[derive(Debug, Clone)]
+pub struct Symbols {
+    pub archive: PathBuf,
+    pub rebuild: PathBuf,
+}
+
 /// Everything `qualify` needs to measure one cell on its native runner.
 #[derive(Debug, Clone)]
 pub struct QualifyInputs {
@@ -244,6 +251,7 @@ pub struct QualifyInputs {
     pub runner_os: String,
     pub runner_arch: String,
     pub consumer: Option<Consumer>,
+    pub symbols: Option<Symbols>,
 }
 
 struct Verdicts(Vec<Check>);
@@ -667,6 +675,19 @@ fn differing_members(inputs: &QualifyInputs) -> String {
     }
 }
 
+/// Checks that the symbols archive equals its rebuild.
+fn symbols_match(symbols: &Symbols) -> Result<(), String> {
+    let (first, _) = sha256_file(&symbols.archive)?;
+    let (second, _) = sha256_file(&symbols.rebuild)?;
+    if first == second {
+        Ok(())
+    } else {
+        Err(format!(
+            "symbols rebuild is {second}, symbols archive is {first}"
+        ))
+    }
+}
+
 /// Measures one cell archive on its native runner and returns a record with a verdict per check.
 pub fn qualify(inputs: &QualifyInputs, runner: &dyn Runner) -> CellRecord {
     let cell = &inputs.cell;
@@ -719,17 +740,22 @@ pub fn qualify(inputs: &QualifyInputs, runner: &dyn Runner) -> CellRecord {
     );
     verdicts.note(
         "determinism",
-        sha256_file(&inputs.rebuild).and_then(|(rebuilt, _)| {
-            if rebuilt == archive.sha256 {
-                Ok(())
-            } else {
-                Err(format!(
-                    "rebuild is {rebuilt}, archive is {}; {}",
-                    archive.sha256,
-                    differing_members(inputs)
-                ))
-            }
-        }),
+        sha256_file(&inputs.rebuild)
+            .and_then(|(rebuilt, _)| {
+                if rebuilt == archive.sha256 {
+                    Ok(())
+                } else {
+                    Err(format!(
+                        "rebuild is {rebuilt}, archive is {}; {}",
+                        archive.sha256,
+                        differing_members(inputs)
+                    ))
+                }
+            })
+            .and_then(|()| match &inputs.symbols {
+                Some(symbols) => symbols_match(symbols),
+                None => Ok(()),
+            }),
     );
     let listing = dx_deploy_tools::archive_extract(&inputs.archive, &inputs.work.join(INSTALL_DIR))
         .map_err(|error| format!("cannot extract: {error}"));
@@ -1015,7 +1041,7 @@ fn read_matrix(path: &str) -> Result<Vec<Cell>, String> {
     parse_matrix(&text)
 }
 
-pub const QUALIFY_USAGE: &str = "qualify --matrix <release_matrix.json> --cell <name> --archive <path> --rebuild <path> --work <dir> --sha <commit> --version <version> --workflow-ref <ref> --workflow-sha <commit> --run-id <id> --job <name> --out <record.json> [--consumer <template dir> --checkout <rules_dx dir>]";
+pub const QUALIFY_USAGE: &str = "qualify --matrix <release_matrix.json> --cell <name> --archive <path> --rebuild <path> --work <dir> --sha <commit> --version <version> --workflow-ref <ref> --workflow-sha <commit> --run-id <id> --job <name> --out <record.json> [--consumer <template dir> --checkout <rules_dx dir>] [--symbols <zip> --symbols-rebuild <zip>]";
 
 pub const VERIFY_USAGE: &str = "verify --matrix <release_matrix.json> --records <dir> --archives <dir> --sha <commit> --version <version> --workflow-ref <ref>";
 
@@ -1041,8 +1067,17 @@ pub fn parse_qualify_args(
             "job",
             "out",
         ],
-        &["consumer", "checkout"],
+        &["consumer", "checkout", "symbols", "symbols-rebuild"],
     )?;
+    let symbols = match (flags.get("symbols"), flags.get("symbols-rebuild")) {
+        (Some(archive), Some(rebuild)) => Some(Symbols {
+            archive: PathBuf::from(*archive),
+            rebuild: PathBuf::from(*rebuild),
+        }),
+        (Some(_), None) => return Err("--symbols needs --symbols-rebuild".to_owned()),
+        (None, Some(_)) => return Err("--symbols-rebuild needs --symbols".to_owned()),
+        (None, None) => None,
+    };
     let consumer = match (flags.get("consumer"), flags.get("checkout")) {
         (Some(template), Some(checkout)) => Some(Consumer {
             template: PathBuf::from(*template),
@@ -1071,6 +1106,7 @@ pub fn parse_qualify_args(
         runner_os: runner_os.to_owned(),
         runner_arch: runner_arch.to_owned(),
         consumer,
+        symbols,
     };
     Ok((inputs, PathBuf::from(flag(&flags, "out"))))
 }

deploy/release/src/cell_tests.rs

@@ -338,6 +338,7 @@ fn stage(cell: Cell, members: Members) -> Fixture {
             template: dir.path().join("template"),
             checkout: dir.path().join("checkout"),
         }),
+        symbols: None,
     };
     Fixture { _dir: dir, inputs }
 }
@@ -557,6 +558,26 @@ fn qualify_rejects_an_extra_member_bad_modes_and_identity() {
     assert_failed(&record, "identity", "IDENTITY arch is Some(\"aarch64\")");
 }
 
+#[test]
+fn qualify_compares_the_symbols_rebuild() {
+    let fixture = synthetic("zip", |_| {});
+    let dir = fixture.inputs.work.parent().expect("dir").to_path_buf();
+    let archive = dir.join("out").join("dx-pdb.zip");
+    let rebuild = dir.join("rebuild").join("dx-pdb.zip");
+    std::fs::write(&archive, b"pdb").expect("archive");
+    std::fs::write(&rebuild, b"pdb").expect("rebuild");
+    let mut inputs = fixture.inputs.clone();
+    inputs.symbols = Some(Symbols {
+        archive,
+        rebuild: rebuild.clone(),
+    });
+    let record = qualify(&inputs, &FakeRunner::new());
+    assert_eq!(verdict_of(&record, "determinism").0, Verdict::Ok);
+    std::fs::write(&rebuild, b"other pdb").expect("rebuild");
+    let record = qualify(&inputs, &FakeRunner::new());
+    assert_failed(&record, "determinism", "symbols rebuild is");
+}
+
 #[test]
 fn qualify_rejects_format_name_digest_and_rebuild_mismatches() {
     let fixture = synthetic("zip", |_| {});