From 3d23f5a5c65fbf3643274a5f95eecf9040cc8630 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Tue, 18 Aug 2026 12:52:56 +0200 Subject: [PATCH] Sync platform npm binaries via dune promotion The compiler binaries in packages/@rescript//bin - what cli/*.js, the test harnesses, and the runtime build actually run - were copied there by mtime-based Makefile rules that only fired when make itself ran, plus a separate scripts/copyExes.js step in CI. A bare `dune build` left them stale, and because some test suites read _build directly while others resolve through the npm package, the tree could silently disagree with itself about which compiler was under test. Replace both copy mechanisms with dune promotion rules (compiler/sync/dune): one rule per platform, covering bsc, rescript-editor-analysis, and rescript-tools. Promotion runs on every `dune build`, compares content so unchanged binaries are not rewritten (no timestamp churn for make's downstream stamps), strips on Unix like the packaging step always did, and copies unstripped on Windows. The browser profile is excluded so a playground build can never overwrite the native binaries. The promotion rules are now the only producer of these files: - The Makefile copy rules and both mtime-compensation touch loops are deleted. `make compiler` verifies each binary against the dune build output (cmp, with one forced re-promotion retry that also self-heals a damaged copy), restores a lost executable bit, and fails with a pointer to compiler/sync/dune when promotion did not produce it - e.g. when a platform predicate is wrong - rather than silently accepting a missing or stale binary. - scripts/copyExes.js loses its compiler mode (the rewatch mode remains), and the CI step that used it is replaced by scripts/checkCompilerExes.js, which asserts on every platform in the matrix that the promoted binaries match the build instead of quietly re-creating them. The coverage flow still deliberately swaps in unstripped instrumented binaries after its build; the next regular `dune build` restores the clean ones automatically. Signed-Off-By: Cristiano Calcagno Co-Authored-By: Claude Fable 5 --- .github/workflows/ci.yml | 4 +- CHANGELOG.md | 1 + Makefile | 27 ++++---- compiler/dune | 1 + compiler/sync/dune | 118 +++++++++++++++++++++++++++++++++++ scripts/checkCompilerExes.js | 54 ++++++++++++++++ scripts/copyExes.js | 16 ++--- 7 files changed, 194 insertions(+), 27 deletions(-) create mode 100644 compiler/sync/dune create mode 100644 scripts/checkCompilerExes.js diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ea045088fe..0beb1e1e041 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -162,8 +162,8 @@ jobs: _build key: ${{ steps.compiler-build-state-key.outputs.value }} - - name: Copy compiler exes to platform bin dir - run: node scripts/copyExes.js --compiler + - name: Verify promoted compiler exes + run: node scripts/checkCompilerExes.js - name: "Syntax: Run tests" env: diff --git a/CHANGELOG.md b/CHANGELOG.md index e29f818a89e..fdc5ac07138 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ #### :house: Internal +- Sync the platform npm package's compiler binaries (`packages/@rescript//bin`) via dune promotion on every `dune build`, instead of Makefile/CI copy steps that only ran when make did: a plain `dune build` can no longer leave `cli/*.js` and the test harnesses running a stale compiler. https://github.com/rescript-lang/rescript/pull/8560 - Remove unused compiler IR definitions, modules, helpers, error variants, and Typedtree fields. https://github.com/rescript-lang/rescript/pull/8551 https://github.com/rescript-lang/rescript/pull/8555 - Add the `-check-lam` compiler option, enable Lambda invariant checking in compiler tests, and remove build-profile-dependent checking. https://github.com/rescript-lang/rescript/pull/8534 - Replace `-bs-diagnose` with `-debug-ir` and make IR diagnostic artifacts deterministic, compilation-local, and easy to clean. https://github.com/rescript-lang/rescript/pull/8535 diff --git a/Makefile b/Makefile index bec611f7661..eaa5dcfbcdb 100644 --- a/Makefile +++ b/Makefile @@ -102,24 +102,26 @@ COMPILER_SOURCE_DIRS := compiler tests analysis tools COMPILER_SOURCES = $(shell find $(COMPILER_SOURCE_DIRS) -type f \( -name '*.ml' -o -name '*.mli' -o -name '*.dune' -o -name dune -o -name dune-project \)) COMPILER_BIN_NAMES := bsc rescript-editor-analysis rescript-tools COMPILER_EXES := $(addsuffix .exe,$(addprefix $(BIN_DIR)/,$(COMPILER_BIN_NAMES))) -COMPILER_DUNE_BINS := $(addsuffix $(PLATFORM_EXE_EXT),$(addprefix $(DUNE_BIN_DIR)/,$(COMPILER_BIN_NAMES))) compiler: $(COMPILER_EXES) -define MAKE_COMPILER_COPY_RULE -$(BIN_DIR)/$(1).exe: $(DUNE_BIN_DIR)/$(1)$(PLATFORM_EXE_EXT) - $$(call COPY_EXE,$$<,$$@) -endef - -$(foreach bin,$(COMPILER_BIN_NAMES),$(eval $(call MAKE_COMPILER_COPY_RULE,$(bin)))) - -# "touch" after dune build to make sure that the binaries' timestamps are updated -# even if the actual content of the sources hasn't changed. +# The compiler binaries in $(BIN_DIR) are produced by dune itself: the +# promotion rules in compiler/sync/dune copy (and strip) them into the +# platform npm package on every `dune build`, comparing content so unchanged +# binaries are not rewritten. Make only needs to know that running dune +# produces them. $(COMPILER_BUILD_STAMP): $(COMPILER_SOURCES) dune build - @$(foreach bin,$(COMPILER_DUNE_BINS),touch $(bin);) -$(COMPILER_DUNE_BINS): $(COMPILER_BUILD_STAMP) ; +$(COMPILER_EXES): $(COMPILER_BUILD_STAMP) + @cmp -s $@ _build/default/compiler/sync/$(@F) || { \ + rm -f _build/default/compiler/sync/$(@F); dune build; } + @cmp -s $@ _build/default/compiler/sync/$(@F) || { \ + echo "Error: $@ is missing or does not match the dune build output."; \ + echo "Dune promotion did not produce it; check that this platform is"; \ + echo "covered by a rule in compiler/sync/dune."; \ + exit 1; } + @test -x $@ || chmod 755 $@ clean-compiler: dune clean && rm -f $(COMPILER_EXES) $(COMPILER_BUILD_STAMP) @@ -270,7 +272,6 @@ COVERAGE_TEST_ENV := BISECT_FILE=$(COVERAGE_BISECT_PREFIX) BISECT_SILENT=YES .PHONY: coverage-build coverage-build: | $(YARN_INSTALL_STAMP) dune build --instrument-with bisect_ppx - @$(foreach bin,$(COMPILER_DUNE_BINS),touch $(bin);) @$(foreach bin,$(COMPILER_BIN_NAMES), \ cp $(DUNE_BIN_DIR)/$(bin)$(PLATFORM_EXE_EXT) $(BIN_DIR)/$(bin).exe && \ chmod 755 $(BIN_DIR)/$(bin).exe;) diff --git a/compiler/dune b/compiler/dune index dd56eeb2ce5..9c81af6a72a 100644 --- a/compiler/dune +++ b/compiler/dune @@ -9,6 +9,7 @@ gentype jsoo ml + sync syntax) (env diff --git a/compiler/sync/dune b/compiler/sync/dune new file mode 100644 index 00000000000..e11fdfacaae --- /dev/null +++ b/compiler/sync/dune @@ -0,0 +1,118 @@ +; Keep the npm platform package's binaries in sync with every `dune build`. +; +; The packages/@rescript//bin copies are what cli/*.js, the test +; harnesses, and the runtime build actually run; these promotion rules are +; their only producer (the Makefile no longer copies binaries). Promotion +; compares content, so unchanged binaries are not rewritten and no-op builds +; cause no timestamp churn downstream. +; +; One rule per platform; %{system}/%{architecture} come from `ocamlc -config` +; (note: x64 is "amd64" there). Windows copies without stripping, matching +; the historical packaging step. The browser profile is excluded because it +; builds a playground-flavoured compiler that must never overwrite the +; native binaries. + +(rule + (enabled_if + (and + (<> %{profile} browser) + (= %{system} macosx) + (= %{architecture} arm64))) + (targets bsc.exe rescript-editor-analysis.exe rescript-tools.exe) + (deps + ../bsc/rescript_compiler_main.exe + ../../analysis/bin/main.exe + ../../tools/bin/main.exe) + (mode + (promote + (until-clean) + (into ../../packages/@rescript/darwin-arm64/bin))) + (action + (progn + (run strip -o bsc.exe ../bsc/rescript_compiler_main.exe) + (run strip -o rescript-editor-analysis.exe ../../analysis/bin/main.exe) + (run strip -o rescript-tools.exe ../../tools/bin/main.exe)))) + +(rule + (enabled_if + (and + (<> %{profile} browser) + (= %{system} macosx) + (= %{architecture} amd64))) + (targets bsc.exe rescript-editor-analysis.exe rescript-tools.exe) + (deps + ../bsc/rescript_compiler_main.exe + ../../analysis/bin/main.exe + ../../tools/bin/main.exe) + (mode + (promote + (until-clean) + (into ../../packages/@rescript/darwin-x64/bin))) + (action + (progn + (run strip -o bsc.exe ../bsc/rescript_compiler_main.exe) + (run strip -o rescript-editor-analysis.exe ../../analysis/bin/main.exe) + (run strip -o rescript-tools.exe ../../tools/bin/main.exe)))) + +(rule + (enabled_if + (and + (<> %{profile} browser) + (= %{system} linux) + (= %{architecture} arm64))) + (targets bsc.exe rescript-editor-analysis.exe rescript-tools.exe) + (deps + ../bsc/rescript_compiler_main.exe + ../../analysis/bin/main.exe + ../../tools/bin/main.exe) + (mode + (promote + (until-clean) + (into ../../packages/@rescript/linux-arm64/bin))) + (action + (progn + (run strip -o bsc.exe ../bsc/rescript_compiler_main.exe) + (run strip -o rescript-editor-analysis.exe ../../analysis/bin/main.exe) + (run strip -o rescript-tools.exe ../../tools/bin/main.exe)))) + +(rule + (enabled_if + (and + (<> %{profile} browser) + (= %{system} linux) + (= %{architecture} amd64))) + (targets bsc.exe rescript-editor-analysis.exe rescript-tools.exe) + (deps + ../bsc/rescript_compiler_main.exe + ../../analysis/bin/main.exe + ../../tools/bin/main.exe) + (mode + (promote + (until-clean) + (into ../../packages/@rescript/linux-x64/bin))) + (action + (progn + (run strip -o bsc.exe ../bsc/rescript_compiler_main.exe) + (run strip -o rescript-editor-analysis.exe ../../analysis/bin/main.exe) + (run strip -o rescript-tools.exe ../../tools/bin/main.exe)))) + +(rule + (enabled_if + (and + (<> %{profile} browser) + (= %{system} mingw64) + (= %{architecture} amd64))) + (targets bsc.exe rescript-editor-analysis.exe rescript-tools.exe) + (deps + ../bsc/rescript_compiler_main.exe + ../../analysis/bin/main.exe + ../../tools/bin/main.exe) + (mode + (promote + (until-clean) + (into ../../packages/@rescript/win32-x64/bin))) + (action + (progn + (copy ../bsc/rescript_compiler_main.exe bsc.exe) + (copy ../../analysis/bin/main.exe rescript-editor-analysis.exe) + (copy ../../tools/bin/main.exe rescript-tools.exe)))) diff --git a/scripts/checkCompilerExes.js b/scripts/checkCompilerExes.js new file mode 100644 index 00000000000..6c55782bc09 --- /dev/null +++ b/scripts/checkCompilerExes.js @@ -0,0 +1,54 @@ +#!/usr/bin/env node + +// @ts-check + +// Verify that the compiler binaries in the platform npm package were +// produced by dune promotion from the current build (compiler/sync/dune is +// their only producer). Fails when a binary is missing - e.g. this +// platform's promotion rule did not fire - or when a stale binary from an +// earlier build is still in place. + +import * as fs from "node:fs"; +import * as path from "node:path"; +import { binDir } from "#cli/bins"; + +const syncDir = path.join( + import.meta.dirname, + "..", + "_build", + "default", + "compiler", + "sync", +); + +let ok = true; +for (const exe of ["bsc", "rescript-editor-analysis", "rescript-tools"]) { + const promoted = path.join(binDir, `${exe}.exe`); + const built = path.join(syncDir, `${exe}.exe`); + if ( + !fs.existsSync(promoted) || + !fs.existsSync(built) || + !fs.readFileSync(promoted).equals(fs.readFileSync(built)) + ) { + console.error(`Error: ${promoted} does not match ${built}.`); + ok = false; + } else if (process.platform !== "win32") { + // Content being right is not enough: an archive round-trip can drop the + // executable bit while preserving bytes. + try { + fs.accessSync(promoted, fs.constants.X_OK); + } catch { + console.error(`Error: ${promoted} is not executable.`); + ok = false; + } + } +} + +if (!ok) { + console.error( + "Dune promotion did not produce these binaries; check that this platform is covered by a rule in compiler/sync/dune.", + ); + process.exit(1); +} + +console.log("Compiler binaries in the platform package match the dune build."); diff --git a/scripts/copyExes.js b/scripts/copyExes.js index a7e2aadd92b..e859d7f39a6 100755 --- a/scripts/copyExes.js +++ b/scripts/copyExes.js @@ -2,14 +2,16 @@ // @ts-check -// Copy exes built by dune to platform bin dir +// Copy the rewatch exe built by cargo to the platform bin dir. +// The dune-built compiler binaries are copied by dune promotion instead +// (see compiler/sync/dune). import * as child_process from "node:child_process"; import * as fs from "node:fs"; import * as path from "node:path"; import { parseArgs } from "node:util"; import { binDir } from "#cli/bins"; -import { compilerBinDir, rewatchDir } from "#dev/paths"; +import { rewatchDir } from "#dev/paths"; const args = parseArgs({ args: process.argv.slice(2), @@ -17,24 +19,14 @@ const args = parseArgs({ all: { type: "boolean", }, - compiler: { - type: "boolean", - }, rewatch: { type: "boolean", }, }, }); -const shouldCopyCompiler = args.values.all || args.values.compiler; const shouldCopyRewatch = args.values.all || args.values.rewatch; -if (shouldCopyCompiler) { - copyExe(compilerBinDir, "rescript-editor-analysis"); - copyExe(compilerBinDir, "rescript-tools"); - copyExe(compilerBinDir, "bsc"); -} - if (shouldCopyRewatch) { copyExe(path.join(rewatchDir, "target", "release"), "rescript"); }