diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0ea045088f..0beb1e1e04 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 e29f818a89..fdc5ac0713 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 bec611f766..eaa5dcfbcd 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 dd56eeb2ce..9c81af6a72 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 0000000000..e11fdfacaa --- /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 0000000000..6c55782bc0 --- /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 a7e2aadd92..e859d7f39a 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"); }