Skip to content

Probe for the actual Danger.swiftmodule instead of trusting the compiler that built danger-swift - #663

Draft
DylanBettermannDD wants to merge 5 commits into
danger:masterfrom
DylanBettermannDD:db/mobex-5198-modulefolder-probe
Draft

Probe for the actual Danger.swiftmodule instead of trusting the compiler that built danger-swift#663
DylanBettermannDD wants to merge 5 commits into
danger:masterfrom
DylanBettermannDD:db/mobex-5198-modulefolder-probe

Conversation

@DylanBettermannDD

Copy link
Copy Markdown

Problem

SPMDanger.moduleFolder (used to build the -I flag when compiling a Dangerfile against a DangerDeps SwiftPM product) picks between .build/debug and .build/debug/Modules using a compile-time check on the Swift version that built the danger-swift binary itself:

public var moduleFolder: String {
    #if compiler(<6.0)
        buildFolder
    #else
        buildFolder + "/Modules"
    #endif
}

That's a proxy for a runtime property — the toolchain and build system used to build the target package — and it's wrong whenever those two differ, which is the normal case for a distributed danger-swift binary (Homebrew, Docker, or the prebuilt universal binary from #660): the binary's own compile-time Swift version has nothing to do with which build system/toolchain later builds the Dangerfile's dependencies.

Concretely, this breaks on a toolchain that defaults swift build to the newer swiftbuild build system (observed on Xcode 27 Beta 4 / Swift 6.4; not claiming a specific version threshold here, just that this is a real, shipping build system that behaves differently from native). Under swiftbuild, SwiftPM produces flat *.swiftmodule files with no Modules/ subdirectory at all — the inverse of what native+Swift 6 produces. A danger-swift binary built with Swift ≥6.0 (true of every distributed binary today) then looks in Modules/, finds nothing, and fails:

error: no such module 'Danger'

Measured layouts (building only --product DangerDeps<X>, exactly what buildDependencies runs):

native (Xcode 26.6, Swift 6.3.3) swiftbuild (Xcode 27 Beta 4, Swift 6.4)
.build/debug symlink → arm64-apple-macosx/debug symlink → out/Products/Debug
flat *.swiftmodule in bin path 0 6
<bin>/Modules/ exists, 6 modules absent

The two layouts are disjoint. .build/debug itself stays a valid symlink under both — SwiftPM repoints it on every build — so only the module-search path (-I) is wrong, never -L.

Sources/DangerDependenciesResolver/Script.swift's artifactsPath has the identical #if compiler(<6.0) pattern for the Marathon-based inline-dependency path (import ... package: Dangerfiles with no DangerDeps library).

Fix

Replace the compile-time check with a runtime probe for the actual Danger.swiftmodule artifact at both candidate locations, falling back to today's compiled-in default whenever the probe is ambiguous (both or neither candidate present) so no currently-working configuration changes behavior:

public var moduleFolder: String {
    let flatModule = buildFolder + "/Danger.swiftmodule"
    let nestedModule = buildFolder + "/Modules/Danger.swiftmodule"

    switch (fileManager.fileExists(atPath: flatModule), fileManager.fileExists(atPath: nestedModule)) {
    case (true, false):
        return buildFolder
    case (false, true):
        return buildFolder + "/Modules"
    default:
        #if compiler(<6.0)
            return buildFolder
        #else
            return buildFolder + "/Modules"
        #endif
    }
}

Probing for the exact artifact (not mere Modules/ directory existence) avoids a false positive from an empty/partial Modules/ left over from a prior build under a different toolchain.

Backwards compatibility is testable, not just asserted: both unambiguous branches are byte-identical to today's two #if compiler branches, and the only behavior change is in states that are broken today (native <6.0 binary vs. ≥6.0 package or vice versa, and swiftbuild). The ambiguous fallback keeps every currently-working binary — including the official Docker image, which defaults to Swift 5.9 — on its existing compiled-in behavior.

The same fix is applied to Script.artifactsPath, resolved against the script's own folder (not the process's working directory, since the build for that path runs in a separate scratch folder).

Testing

  • swift test --filter "SPMDangerTests|ScriptTests" — 20/20 passing, including new coverage for all four probe states (flat-only, nested-only, both-present, neither-present) on both call sites, plus a regression test pinning that Script.artifactsPath probes under its own folder, not the process cwd.
  • Verified end-to-end against real Dangerfiles from a large iOS monorepo, compiling with the runner's actual swiftc flags:
    • Under the newer swiftbuild build system (Xcode 27 Beta 4, Swift 6.4): the unpatched behavior (-I <bin>/Modules) fails with no such module 'Danger'; the patched behavior (-I <bin>) compiles and runs.
    • Under native (Xcode 26.6, Swift 6.3.3): unchanged — the probe unambiguously selects the nested Modules/ path, byte-identical to today, and the flat path correctly fails (proving the two layouts are genuinely disjoint, not just "either works").
  • swift build -c release --product danger-swift succeeds under both toolchains.

Related

…ler that built danger-swift

moduleFolder (and the equivalent Script.artifactsPath) picked between
.build/debug and .build/debug/Modules using a compile-time #if compiler(<6.0)
check on whichever Swift built the danger-swift binary itself. That's a proxy
for a runtime property of the toolchain/build system building the target
package, and it's wrong whenever those two differ — which is the normal case
for a distributed binary (Homebrew, Docker, the new prebuilt universal
binary), and is why the swiftbuild build system (Xcode 16.3+'s new default)
breaks it.

Replace the compile-time check with a runtime probe for the exact
Danger.swiftmodule artifact at both candidate locations, falling back to
today's compiled-in default whenever the probe is ambiguous (both or neither
present) so no currently-working configuration changes behavior.
The Marathon inline-dependency path's Script.artifactsPath had the same
compile-time #if compiler(<6.0) defect as SPMDanger.moduleFolder, but an
initial fix probed FileManager.default against paths relative to the
process's current directory. Runner.swift's only caller resolves the
returned paths relative to the script's own folder (a separate directory
where the build actually ran), not the process cwd, so the probe could pick
the wrong candidate based on unrelated filesystem state.

Resolve the probe against the script's own folder instead, with an
injectable FileManager so this is actually testable, and add coverage
mirroring the SPMDangerTests cases plus a regression test pinning that the
probe ignores the process cwd.
@DylanBettermannDD

Copy link
Copy Markdown
Author

The macOS CI failures here are unrelated to this diff. Every macOS job fails identically before Danger ever gets to run the compiled Swift:

Failed to fetch GitHub pull request files: TypeError: terminated
  ... ERR_INVALID_ARG_TYPE: The "stream" argument must be an instance of Stream. Received an instance of ReadableStream

This happens inside the danger-js binary's own fetch() call to GitHub, after our build already succeeded (Build complete!, danger-swift links and launches fine) — the compiled runner never gets a chance to run.

Root cause looks environmental: the macOS jobs install danger-js unpinned (brew install danger/tap/danger-js), while the Linux jobs use a pinned Node 20.x (actions/setup-node@v4) via yarn global add danger — and Linux passes on both this PR and #662. A recent danger-js release appears to bundle an undici version whose fetch() is incompatible with the current macOS runners.

This is reproducible on #662 as well (identical stack trace, identical point of failure), so it predates this PR and isn't something this diff can fix — flagging in case it's useful context, but not blocking review of the actual change.

@DylanBettermannDD

DylanBettermannDD commented Aug 1, 2026

Copy link
Copy Markdown
Author

Correction to my earlier note here about the macOS CI failure: I'd said "a recent danger-js release appears to bundle an undici version whose fetch() is incompatible with the current macOS runners" — that's not quite right. The runners' own Node (22/24) is fine. The actual cause is that macOS CI installs danger-js via the Homebrew tap, which ships a pkg-built standalone binary with its own embedded Node 18 older than 18.17 — and undici 6 (which danger-js 13.0.10 pins) requires >=18.17. Below that, every fetch() call throws ERR_INVALID_ARG_TYPE.

Opened a fix at #664: install danger-js on macOS via setup-node + yarn, matching how the (already-passing) Linux jobs install it, instead of the brew binary. CI on that PR is green, including a full run of the Test dependencies resolver and Test on macOS matrices (Danger: ✓ passed review). I haven't re-run this branch's own CI against that fix yet, but the mechanism is now confirmed rather than just theorized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant