Skip to content

Commit 3eb353e

Browse files
tausbnCopilot
andcommitted
swift-syntax-rs: Address PR review feedback
- BUILD.bazel: require x86_64 on the Linux branch of the compatibility select. The only registered standalone Linux Swift toolchain is x86_64-only, so without the CPU constraint Linux/aarch64 targets stayed "compatible" and then failed toolchain resolution instead of being skipped cleanly. - build.rs: emitting `rerun-if-changed` disables Cargo's default whole-package scan, so list every input to `swift build` — the package manifests, `Package.resolved`, `.swift-version`, and the whole Sources tree — so stale shim/toolchain output can't linger. (`.build/` is left unwatched, as it is this build's own output.) - src/lib.rs: a null result from the shim is not a parse failure — SwiftParser recovers from invalid syntax and always yields a tree — so it means the shim failed to serialize/allocate the JSON. Fix the error message and the variant doc accordingly. - README.md: `.swift-version` is not honored by the macOS Bazel build (which uses the host Xcode toolchain); document that it pins the Linux and local builds only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1577d82 commit 3eb353e

4 files changed

Lines changed: 44 additions & 17 deletions

File tree

unified/swift-syntax-rs/BUILD.bazel

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,21 @@ exports_files([".swift-version"])
1010
# Targets in this package require a Swift toolchain (Linux or macOS).
1111
# `select()` gives us OR-of-OSes; other platforms get marked incompatible
1212
# so `bazel build/test //...` skips them cleanly.
13+
#
14+
# The Linux branch additionally requires x86_64: the only registered standalone
15+
# Linux Swift toolchain (`swift_toolchain_exec_ubuntu24.04`, see //:MODULE.bazel)
16+
# is x86_64-only. Without the CPU constraint, Linux/aarch64 targets would stay
17+
# "compatible" and then fail toolchain resolution instead of being skipped.
18+
config_setting(
19+
name = "linux_x86_64",
20+
constraint_values = [
21+
"@platforms//os:linux",
22+
"@platforms//cpu:x86_64",
23+
],
24+
)
25+
1326
_SWIFT_SUPPORTED_PLATFORMS = select({
14-
"@platforms//os:linux": [],
27+
":linux_x86_64": [],
1528
"@platforms//os:macos": [],
1629
"//conditions:default": ["@platforms//:incompatible"],
1730
})

unified/swift-syntax-rs/README.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,15 @@ Requirements:
155155
[`xcode_transition.bzl`](xcode_transition.bzl)), so other targets on macOS
156156
keep using Bazel's default CC toolchain.
157157

158-
The Swift compiler version is read from [`.swift-version`](.swift-version) by
159-
both the Bazel toolchain (`swift.toolchain(swift_version_file = …)`) and the
160-
local build, and is kept in sync with the `swift-syntax` release pinned in
161-
`swift/Package.swift`, so local and CI builds behave identically.
158+
The Swift compiler version in [`.swift-version`](.swift-version) is kept in sync
159+
with the `swift-syntax` release pinned in `swift/Package.swift`. It is honored on
160+
**Linux**, where the hermetic swift.org Bazel toolchain
161+
(`swift.toolchain(swift_version_file = …)`) and the local `cargo`/`swift build`
162+
both read it. On **macOS** it is *not* honored by the Bazel build: `rules_swift`
163+
auto-registers the host `xcode_swift_toolchain`, which uses whichever Swift ships
164+
with the installed Xcode and ignores `.swift-version`. So the pinned version
165+
governs Linux (and local) builds, while the macOS compiler version depends on the
166+
host Xcode.
162167

163168
## Usage
164169

unified/swift-syntax-rs/build.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,20 @@ fn main() {
66
let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap());
77
let swift_dir = manifest_dir.join("swift");
88

9-
println!(
10-
"cargo:rerun-if-changed={}",
11-
swift_dir.join("Package.swift").display()
12-
);
13-
println!(
14-
"cargo:rerun-if-changed={}",
15-
swift_dir
16-
.join("Sources/SwiftSyntaxFFI/SwiftSyntaxFFI.swift")
17-
.display()
18-
);
9+
// Emitting any `rerun-if-changed` disables Cargo's default whole-package
10+
// scan, so we must list every input to the `swift build` below. Watch the
11+
// package manifests, the pinned dependency lockfile, the pinned compiler
12+
// version, and the entire Swift sources tree (a directory is scanned
13+
// recursively). `.build/` is intentionally *not* watched (it is this
14+
// build's output; watching it would cause perpetual rebuilds).
15+
for input in [
16+
swift_dir.join("Package.swift"),
17+
swift_dir.join("Package.resolved"),
18+
swift_dir.join("Sources"),
19+
manifest_dir.join(".swift-version"),
20+
] {
21+
println!("cargo:rerun-if-changed={}", input.display());
22+
}
1923
println!("cargo:rerun-if-env-changed=SWIFT");
2024
println!("cargo:rerun-if-env-changed=SWIFTC");
2125

unified/swift-syntax-rs/src/lib.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,20 @@ unsafe extern "C" {
2929
pub enum ParseError {
3030
/// The provided source contained an interior NUL byte.
3131
NulByte,
32-
/// The Swift side failed to produce a result.
32+
/// The Swift shim returned no result. `SwiftParser` recovers from invalid
33+
/// syntax (it always produces a tree, possibly with error nodes), so this
34+
/// does *not* indicate a syntax error in the source — it means the shim
35+
/// failed to serialize the tree to JSON or to allocate the result string.
3336
SwiftFailure,
3437
}
3538

3639
impl std::fmt::Display for ParseError {
3740
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
3841
match self {
3942
ParseError::NulByte => write!(f, "source contained an interior NUL byte"),
40-
ParseError::SwiftFailure => write!(f, "swift-syntax failed to parse the source"),
43+
ParseError::SwiftFailure => {
44+
write!(f, "the swift-syntax shim failed to produce a JSON result")
45+
}
4146
}
4247
}
4348
}

0 commit comments

Comments
 (0)