Skip to content

Commit 5c15979

Browse files
authored
[turbopack] Enable debug assertions in CI (#80739)
## Enable debug assertions in CI ### What? Build using `--profile release-with-assertions` for all tests in CI. A previous PR (#80511) enabled them for unit tests, this completes the effort for e2e tests run in CI. There is one exception. The production tests currently have a failing assertion for duplicate modules, so a new env var (`TURBOPACK_TEMP_DISABLE_DUPLICATE_MODULES_CHECK`) is introduced to disable that exact check to allow this to be landed. ### Why? Debug assertions help catch bugs early by validating assumptions in the code. By enabling them in tests, we can improve quality ### How? - Added a new `rustBuildProfile` parameter to the reusable build workflow to control the build profile - Created a new `build-native-release-with-assertions` script in next-swc to set the cargo profile - Updated the turbopack production tests to set the opt out variable Part-of PACK-4578
1 parent ee84a02 commit 5c15979

File tree

5 files changed

+43
-11
lines changed

5 files changed

+43
-11
lines changed

.github/workflows/build_and_test.yml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,8 @@ jobs:
315315
export TURBOPACK_BUILD=1
316316
export NEXT_TEST_MODE=start
317317
export NEXT_TEST_REACT_VERSION="${{ matrix.react }}"
318+
# TODO(PACK-4578): Remove
319+
export TURBOPACK_TEMP_DISABLE_DUPLICATE_MODULES_CHECK=1
318320
319321
node run-tests.js --timings -g ${{ matrix.group }} --type production
320322
stepName: 'test-turbopack-production-react-${{ matrix.react }}-${{ matrix.group }}'

.github/workflows/build_reusable.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ on:
4242
required: false
4343
description: 'if nextest rust dep is needed'
4444
type: string
45+
rustBuildProfile:
46+
required: false
47+
description: 'The profile to use for the build, default is `release-with-assertions`, also supports `` for debug and `release` for normal release'
48+
type: string
49+
default: 'release-with-assertions'
4550
uploadSwcArtifact:
4651
required: false
4752
description: 'if swc artifact needs uploading'
@@ -179,7 +184,7 @@ jobs:
179184
with:
180185
cache-provider: 'turbo'
181186
save-if: ${{ github.ref_name == 'canary' }}
182-
shared-key: ${{ inputs.rustCacheKey }}-${{ inputs.buildNativeTarget }}-build-${{ hashFiles('.cargo/config.toml') }}
187+
shared-key: ${{ inputs.rustCacheKey }}-${{ inputs.buildNativeTarget }}-build-${{ inputs.rustBuildProfile }}-${{ hashFiles('.cargo/config.toml') }}
183188

184189
# clean up any previous artifacts to avoid hitting disk space limits
185190
- run: git clean -xdf && rm -rf /tmp/next-repo-*; rm -rf /tmp/next-install-* /tmp/yarn-* /tmp/ncc-cache target
@@ -197,7 +202,7 @@ jobs:
197202
- run: node scripts/normalize-version-bump.js
198203
name: normalize versions
199204

200-
- run: pnpm dlx turbo@${TURBO_VERSION} run build-native-release -v --env-mode loose --remote-cache-timeout 90 --summarize -- --target ${{ inputs.buildNativeTarget }}
205+
- run: pnpm dlx turbo@${TURBO_VERSION} run build-native-${{ inputs.rustBuildProfile }} -v --env-mode loose --remote-cache-timeout 90 --summarize -- --target ${{ inputs.buildNativeTarget }}
201206
if: ${{ inputs.skipNativeBuild != 'yes' }}
202207

203208
- name: Upload next-swc artifact

packages/next-swc/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
"clean": "node ../../scripts/rm.mjs native",
1010
"build-native": "napi build --platform -p next-swc-napi --cargo-cwd ../../ --cargo-name next_swc_napi --features plugin,image-extended --js false native",
1111
"build-native-release": "napi build --platform -p next-swc-napi --cargo-cwd ../../ --cargo-name next_swc_napi --release --features plugin,image-extended,tracing/release_max_level_info --js false native",
12+
"build-native-release-with-assertions": "napi build --platform -p next-swc-napi --cargo-cwd ../../ --cargo-name next_swc_napi --profile release-with-assertions --features plugin,image-extended,tracing/release_max_level_info --js false native",
1213
"build-native-no-plugin": "napi build --platform -p next-swc-napi --cargo-cwd ../../ --cargo-name next_swc_napi --features image-webp --js false native",
1314
"build-native-no-plugin-release": "napi build --platform -p next-swc-napi --cargo-cwd ../../ --cargo-name next_swc_napi --release --features image-webp,tracing/release_max_level_info --js false native",
1415
"build-native-wasi": "npx --package=@napi-rs/[email protected] napi build --platform --target wasm32-wasip1-threads -p next-swc-napi --cwd ../../ --output-dir packages/next-swc/native --no-default-features",

packages/next-swc/turbo.json

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,19 @@
2828
"env": ["CI"],
2929
"outputs": ["native/*.node"]
3030
},
31+
"build-native-release-with-assertions": {
32+
"inputs": [
33+
"../../.cargo/**",
34+
"../../crates/**",
35+
"../../turbopack/crates/**",
36+
"../../**/Cargo.toml",
37+
"../../**/Cargo.lock",
38+
"../../.github/workflows/build_and_deploy.yml",
39+
"../../rust-toolchain"
40+
],
41+
"env": ["CI"],
42+
"outputs": ["native/*.node"]
43+
},
3144
"build-native-no-plugin": {
3245
"inputs": [
3346
"../../.cargo/**",

turbopack/crates/turbopack-core/src/module_graph/mod.rs

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -344,16 +344,27 @@ impl SingleModuleGraph {
344344

345345
#[cfg(debug_assertions)]
346346
{
347-
let mut duplicates = Vec::new();
348-
let mut set = FxHashSet::default();
349-
for &module in modules.keys() {
350-
let ident = module.ident().to_string().await?;
351-
if !set.insert(ident.clone()) {
352-
duplicates.push(ident)
347+
use once_cell::sync::Lazy;
348+
349+
// TODO(PACK-4578): This is temporary while the last issues are being addressed.
350+
static CHECK_FOR_DUPLICATE_MODULES: Lazy<bool> = Lazy::new(|| {
351+
match std::env::var_os("TURBOPACK_TEMP_DISABLE_DUPLICATE_MODULES_CHECK") {
352+
Some(v) => v != "1" && v != "true",
353+
None => true,
354+
}
355+
});
356+
if *CHECK_FOR_DUPLICATE_MODULES {
357+
let mut duplicates = Vec::new();
358+
let mut set = FxHashSet::default();
359+
for &module in modules.keys() {
360+
let ident = module.ident().to_string().await?;
361+
if !set.insert(ident.clone()) {
362+
duplicates.push(ident)
363+
}
364+
}
365+
if !duplicates.is_empty() {
366+
panic!("Duplicate module idents in graph: {duplicates:#?}");
353367
}
354-
}
355-
if !duplicates.is_empty() {
356-
panic!("Duplicate module idents in graph: {duplicates:#?}");
357368
}
358369
}
359370

0 commit comments

Comments
 (0)