From a3d2f0c586e97e26906bd2327395186bf140f49b Mon Sep 17 00:00:00 2001 From: Delan Azabani Date: Thu, 26 Oct 2023 16:22:14 +0800 Subject: [PATCH] Enable debug assertions for all builds other than official releases (#30509) * Run main and try jobs with debug assertions * use single quotes in workflow expressions * set force-debug-assertions in main.yml * set force-debug-assertions as part of decision job * fix typo in MachCommands.build * fix more hardcoded profile names * fix tidy * split cargo_profile_option on windows * Fix running servoshell and unit tests through a symlink * rename steps to make them less confusing * fix more hardcoded cargo profile options * fix missing inputs in linux-wpt and mac-wpt * make filename an inherent method of Resource * rework release-with-debug-assertions profile to production profile * rework resource logic to eliminate std_test_override * set production flag in nightly release builds * clean up servobuild.example and windows.yml * oops forgot to check in embedder_traits/build.rs * fix mach test-unit behaviour through symlink * unit tests only need current_dir and ancestors * fix macOS package smoketest breakage * expect css/css-color/currentcolor-003 to crash under layout 2013 * fix more references to {force,release-with}-debug-assertions * fix local build failures under --profile production --- .github/workflows/linux-wpt.yml | 8 +- .github/workflows/linux.yml | 34 ++++--- .github/workflows/mac-wpt.yml | 8 +- .github/workflows/mac.yml | 38 +++++--- .github/workflows/main.yml | 4 + .github/workflows/nightly.yml | 5 +- .github/workflows/windows.yml | 30 +++--- Cargo.lock | 14 +-- Cargo.toml | 5 + components/config/Cargo.toml | 3 - components/embedder_traits/Cargo.toml | 1 + components/embedder_traits/build.rs | 23 +++++ components/embedder_traits/resources.rs | 93 ++++++++++--------- components/net/Cargo.toml | 1 - components/net_traits/Cargo.toml | 3 - components/script_traits/Cargo.toml | 3 - components/std_test_override/Cargo.toml | 14 --- components/std_test_override/lib.rs | 14 --- docs/HACKING_QUICKSTART.md | 14 ++- ports/gstplugin/Cargo.toml | 1 + ports/gstplugin/build.rs | 22 ++++- ports/gstplugin/resources.rs | 34 ++++--- ports/servoshell/build.rs | 20 +++- ports/servoshell/prefs.rs | 1 - ports/servoshell/resources.rs | 34 ++++--- python/servo/build_commands.py | 6 +- python/servo/command_base.py | 85 +++++++++++++---- python/servo/package_commands.py | 7 +- servobuild.example | 2 +- tests/unit/style/Cargo.toml | 1 - .../css/css-color/currentcolor-003.html.ini | 2 +- 31 files changed, 342 insertions(+), 188 deletions(-) create mode 100644 components/embedder_traits/build.rs delete mode 100644 components/std_test_override/Cargo.toml delete mode 100644 components/std_test_override/lib.rs diff --git a/.github/workflows/linux-wpt.yml b/.github/workflows/linux-wpt.yml index 9b5a1861940..9ae9ba2b01f 100644 --- a/.github/workflows/linux-wpt.yml +++ b/.github/workflows/linux-wpt.yml @@ -2,6 +2,9 @@ name: Linux WPT Tests on: workflow_call: inputs: + production: + required: false + type: boolean wpt: required: false type: string @@ -10,6 +13,7 @@ on: type: string env: + cargo_profile_option: ${{ inputs.production && '--profile production' || '--release' }} RUST_BACKTRACE: 1 SHELL: /bin/bash WPT_COMMAND_LINE_ARG: "${{ inputs.layout == 'layout-2013' && '--legacy-layout' || '' }}" @@ -67,7 +71,7 @@ jobs: if: ${{ inputs.wpt != 'sync' }} run: | python3 ./mach test-wpt $WPT_COMMAND_LINE_ARG \ - --release --processes $(nproc) --timeout-multiplier 2 \ + ${cargo_profile_option} --processes $(nproc) --timeout-multiplier 2 \ --total-chunks ${{ env.max_chunk_id }} --this-chunk ${{ matrix.chunk_id }} \ --log-raw test-wpt.${{ matrix.chunk_id }}.log \ --log-raw-unexpected unexpected-test-wpt.${{ matrix.chunk_id }}.log \ @@ -79,7 +83,7 @@ jobs: if: ${{ inputs.wpt == 'sync' }} run: | python3 ./mach test-wpt $WPT_COMMAND_LINE_ARG \ - --release --processes $(nproc) --timeout-multiplier 2 \ + ${cargo_profile_option} --processes $(nproc) --timeout-multiplier 2 \ --total-chunks ${{ env.max_chunk_id }} --this-chunk ${{ matrix.chunk_id }} \ --log-raw test-wpt.${{ matrix.chunk_id }}.log \ --always-succeed diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 4ce40e07992..5d7003edfb3 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -2,6 +2,9 @@ name: Linux on: workflow_call: inputs: + production: + required: false + type: boolean wpt: required: false type: string @@ -21,6 +24,9 @@ on: type: string workflow_dispatch: inputs: + production: + required: false + type: boolean wpt: default: "test" required: false @@ -42,6 +48,8 @@ on: branches: ["try-linux", "try-wpt", "try-wpt-2020"] env: + cargo_profile_option: ${{ inputs.production && '--profile production' || '--release' }} + cargo_profile_name: ${{ inputs.production && 'production' || 'release' }} RUST_BACKTRACE: 1 SHELL: /bin/bash SCCACHE_GHA_ENABLED: "true" @@ -82,15 +90,15 @@ jobs: run: sudo apt update && python3 ./mach bootstrap - name: Tidy run: python3 ./mach test-tidy --no-progress --all - - name: Release build - run: python3 ./mach build --release + - name: Release build (${{ inputs.production && 'without' || 'with' }} debug assertions) + run: python3 ./mach build ${cargo_profile_option} - name: Smoketest - run: xvfb-run python3 ./mach smoketest --release + run: xvfb-run python3 ./mach smoketest ${cargo_profile_option} - name: Script tests run: ./mach test-scripts - name: Unit tests if: ${{ inputs.unit-tests || github.ref_name == 'try-linux' }} - run: python3 ./mach test-unit --release + run: python3 ./mach test-unit ${cargo_profile_option} - name: Rename build timing run: cp -r target/cargo-timings target/cargo-timings-linux - name: Archive build timing @@ -101,14 +109,14 @@ jobs: path: target/cargo-timings-* - name: Lockfile check run: ./etc/ci/lockfile_changed.sh - - name: Package - run: python3 ./mach package --release - - name: Upload Package + - name: Build mach package + run: python3 ./mach package ${cargo_profile_option} + - name: Upload artifact for mach package uses: actions/upload-artifact@v3 with: name: linux - path: target/release/servo-tech-demo.tar.gz - - name: Upload + path: target/${cargo_profile_name}/servo-tech-demo.tar.gz + - name: Upload nightly if: ${{ inputs.upload }} run: | python3 ./mach upload-nightly linux \ @@ -118,9 +126,9 @@ jobs: S3_UPLOAD_CREDENTIALS: ${{ secrets.S3_UPLOAD_CREDENTIALS }} NIGHTLY_REPO_TOKEN: ${{ secrets.NIGHTLY_REPO_TOKEN }} NIGHTLY_REPO: ${{ github.repository_owner }}/servo-nightly-builds - - name: Package binary - run: tar -czf target.tar.gz target/release/servo resources - - name: Archive binary + - name: Build package for target + run: tar -czf target.tar.gz target/${cargo_profile_name}/servo resources + - name: Upload artifact for target uses: actions/upload-artifact@v3 with: name: release-binary @@ -132,6 +140,7 @@ jobs: needs: ["build"] uses: ./.github/workflows/linux-wpt.yml with: + production: ${{ inputs.production }} wpt: ${{ inputs.wpt }} layout: "layout-2020" @@ -141,6 +150,7 @@ jobs: needs: ["build"] uses: ./.github/workflows/linux-wpt.yml with: + production: ${{ inputs.production }} wpt: ${{ inputs.wpt }} layout: "layout-2013" diff --git a/.github/workflows/mac-wpt.yml b/.github/workflows/mac-wpt.yml index be13d2ebc7b..28940a2fab5 100644 --- a/.github/workflows/mac-wpt.yml +++ b/.github/workflows/mac-wpt.yml @@ -3,11 +3,15 @@ name: Mac WPT Tests on: workflow_call: inputs: + production: + required: false + type: boolean layout: required: true type: string env: + cargo_profile_option: ${{ inputs.production && '--profile production' || '--release' }} RUST_BACKTRACE: 1 SHELL: /bin/bash WPT_COMMAND_LINE_ARG: "${{ inputs.layout == 'layout-2013' && '--legacy-layout' || '' }}" @@ -43,11 +47,11 @@ jobs: python3 -m pip install --upgrade pip virtualenv python3 ./mach bootstrap - name: Smoketest - run: python3 ./mach smoketest --release + run: python3 ./mach smoketest ${cargo_profile_option} - name: Run tests run: | python3 ./mach test-wpt $WPT_COMMAND_LINE_ARG \ - --release --processes $(sysctl -n hw.logicalcpu) --timeout-multiplier 8 \ + ${cargo_profile_option} --processes $(sysctl -n hw.logicalcpu) --timeout-multiplier 8 \ --total-chunks ${{ env.max_chunk_id }} --this-chunk ${{ matrix.chunk_id }} \ --log-raw test-wpt.${{ matrix.chunk_id }}.log \ --log-raw-unexpected unexpected-test-wpt.${{ matrix.chunk_id }}.log \ diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index 83d820bef9e..6654b03b43a 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -3,6 +3,9 @@ name: Mac on: workflow_call: inputs: + production: + required: false + type: boolean wpt-layout: required: false type: string @@ -19,6 +22,9 @@ on: type: string workflow_dispatch: inputs: + production: + required: false + type: boolean wpt-layout: required: false type: choice @@ -35,6 +41,8 @@ on: branches: ["try-mac", "try-wpt-mac", "try-wpt-mac-2020"] env: + cargo_profile_option: ${{ inputs.production && '--profile production' || '--release' }} + cargo_profile_name: ${{ inputs.production && 'production' || 'release' }} RUST_BACKTRACE: 1 SHELL: /bin/bash SCCACHE_GHA_ENABLED: "true" @@ -70,21 +78,21 @@ jobs: python3 -m pip install --upgrade pip virtualenv python3 ./mach bootstrap brew install gnu-tar - - name: Release build + - name: Release build (${{ inputs.production && 'without' || 'with' }} debug assertions) run: | - python3 ./mach build --release + python3 ./mach build ${cargo_profile_option} - name: Smoketest - run: python3 ./mach smoketest --release + run: python3 ./mach smoketest ${cargo_profile_option} - name: Script tests run: ./mach test-scripts - name: Unit tests if: ${{ inputs.unit-tests || github.ref_name == 'try-mac' }} timeout-minutes: 30 # https://github.com/servo/servo/issues/30275 - run: python3 ./mach test-unit --release - - name: Package - run: python3 ./mach package --release - - name: Package smoketest - run: ./etc/ci/macos_package_smoketest.sh target/release/servo-tech-demo.dmg + run: python3 ./mach test-unit ${cargo_profile_option} + - name: Build mach package + run: python3 ./mach package ${cargo_profile_option} + - name: Run smoketest for mach package + run: ./etc/ci/macos_package_smoketest.sh target/${cargo_profile_name}/servo-tech-demo.dmg - name: Rename build timing run: cp -r target/cargo-timings target/cargo-timings-macos - name: Archive build timing @@ -93,12 +101,12 @@ jobs: name: cargo-timings # Using a wildcard here ensures that the archive includes the path. path: target/cargo-timings-* - - name: Upload package + - name: Upload artifact for mach package uses: actions/upload-artifact@v3 with: name: mac - path: target/release/servo-tech-demo.dmg - - name: Upload + path: target/${cargo_profile_name}/servo-tech-demo.dmg + - name: Upload nightly if: ${{ inputs.upload }} run: | python3 ./mach upload-nightly mac --secret-from-environment \ @@ -108,9 +116,9 @@ jobs: GITHUB_HOMEBREW_TOKEN: ${{ secrets.HOMEBREW_TOKEN }} NIGHTLY_REPO_TOKEN: ${{ secrets.NIGHTLY_REPO_TOKEN }} NIGHTLY_REPO: ${{ github.repository_owner }}/servo-nightly-builds - - name: Package binary - run: gtar -czf target.tar.gz target/release/servo target/release/*.dylib target/release/lib/*.dylib resources - - name: Archive binary + - name: Build package for target + run: gtar -czf target.tar.gz target/${cargo_profile_name}/servo target/${cargo_profile_name}/*.dylib target/${cargo_profile_name}/lib/*.dylib resources + - name: Upload package for target uses: actions/upload-artifact@v3 with: name: release-binary-macos @@ -122,6 +130,7 @@ jobs: needs: ["build"] uses: ./.github/workflows/mac-wpt.yml with: + production: ${{ inputs.production }} layout: "layout-2020" wpt-2013: @@ -130,6 +139,7 @@ jobs: needs: ["build"] uses: ./.github/workflows/mac-wpt.yml with: + production: ${{ inputs.production }} layout: "layout-2013" result: diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c3eb7ef78ba..5147f99251c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -71,6 +71,7 @@ jobs: } let returnValue = { + production: false, platforms, layout, unit_tests, @@ -85,6 +86,7 @@ jobs: if: ${{ contains(fromJson(needs.decision.outputs.configuration).platforms, 'windows') }} uses: ./.github/workflows/windows.yml with: + production: ${{ fromJson(needs.decision.outputs.configuration).production }} unit-tests: ${{ fromJson(needs.decision.outputs.configuration).unit_tests }} build-mac: @@ -93,6 +95,7 @@ jobs: if: ${{ contains(fromJson(needs.decision.outputs.configuration).platforms, 'macos') }} uses: ./.github/workflows/mac.yml with: + production: ${{ fromJson(needs.decision.outputs.configuration).production }} unit-tests: ${{ fromJson(needs.decision.outputs.configuration).unit_tests }} build-linux: @@ -101,6 +104,7 @@ jobs: if: ${{ contains(fromJson(needs.decision.outputs.configuration).platforms, 'linux') }} uses: ./.github/workflows/linux.yml with: + production: ${{ fromJson(needs.decision.outputs.configuration).production }} wpt: 'test' layout: ${{ fromJson(needs.decision.outputs.configuration).layout }} unit-tests: ${{ fromJson(needs.decision.outputs.configuration).unit_tests }} diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index f210d320d87..2da883f87f7 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -70,6 +70,7 @@ jobs: - create-draft-release uses: ./.github/workflows/windows.yml with: + production: true upload: true github-release-id: ${{ needs.create-draft-release.outputs.release-id }} secrets: inherit @@ -82,6 +83,7 @@ jobs: - create-draft-release uses: ./.github/workflows/mac.yml with: + production: true upload: true github-release-id: ${{ needs.create-draft-release.outputs.release-id }} secrets: inherit @@ -94,6 +96,7 @@ jobs: - create-draft-release uses: ./.github/workflows/linux.yml with: + production: true upload: true github-release-id: ${{ needs.create-draft-release.outputs.release-id }} - secrets: inherit \ No newline at end of file + secrets: inherit diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 0b7e31ecc03..5f09b077fc6 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -3,6 +3,9 @@ name: Windows on: workflow_call: inputs: + production: + required: false + type: boolean unit-tests: required: false default: false @@ -16,6 +19,9 @@ on: type: string workflow_dispatch: inputs: + production: + required: false + type: boolean unit-tests: required: false default: false @@ -28,6 +34,8 @@ on: branches: ["try-windows"] env: + cargo_profile_option: ${{ inputs.production && '--profile production' || '--release' }} + cargo_profile_name: ${{ inputs.production && 'production' || 'release' }} RUST_BACKTRACE: 1 SHELL: /bin/bash CCACHE: "ccache" @@ -69,15 +77,15 @@ jobs: python -m pip install --upgrade pip virtualenv python mach fetch python mach bootstrap-gstreamer - - name: Release build - run: python mach build --release + - name: Release build (${{ inputs.production && 'without' || 'with' }} debug assertions) + run: python mach build ($env:cargo_profile_option -split ' ') - name: Copy resources run: cp D:\a\servo\servo\resources C:\a\servo\servo -Recurse - name: Smoketest - run: python mach smoketest --angle --release + run: python mach smoketest --angle ($env:cargo_profile_option -split ' ') - name: Unit tests if: ${{ inputs.unit-tests || github.ref_name == 'try-windows' }} - run: python mach test-unit --release + run: python mach test-unit ($env:cargo_profile_option -split ' ') - name: Rename build timing run: cp C:\a\servo\servo\target\cargo-timings C:\a\servo\servo\target\cargo-timings-windows -Recurse - name: Archive build timing @@ -86,17 +94,17 @@ jobs: name: cargo-timings # Using a wildcard here ensures that the archive includes the path. path: C:\\a\\servo\\servo\\target\\cargo-timings-* - - name: Package - run: python mach package --release - - name: Upload package + - name: Build mach package + run: python mach package ($env:cargo_profile_option -split ' ') + - name: Upload artifact for mach package uses: actions/upload-artifact@v3 with: name: win # These files are available - # MSI Installer: C:\a\servo\servo\target\release\msi\Installer.msi - # Bundle: C:\a\servo\servo\target\release\msi\Servo.exe - # Zip: C:\a\servo\servo\target\release\msi\Servo.zip - path: C:\\a\\servo\\servo\\target/release/msi/Servo.exe + # MSI Installer: C:\a\servo\servo\target\$env:cargo_profile_name\msi\Installer.msi + # Bundle: C:\a\servo\servo\target\$env:cargo_profile_name\msi\Servo.exe + # Zip: C:\a\servo\servo\target\$env:cargo_profile_name\msi\Servo.zip + path: C:\\a\\servo\\servo\\target\\$env:cargo_profile_name\\msi\\Servo.exe - name: Upload nightly if: ${{ inputs.upload }} run: | diff --git a/Cargo.lock b/Cargo.lock index 9d60e2dc164..4d78bfb04a8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1533,6 +1533,7 @@ dependencies = [ name = "embedder_traits" version = "0.0.1" dependencies = [ + "cfg-if 1.0.0", "crossbeam-channel", "ipc-channel", "keyboard-types", @@ -3804,7 +3805,6 @@ dependencies = [ "servo_config", "servo_url", "sha2", - "std_test_override", "time 0.1.45", "tokio", "tokio-rustls", @@ -3855,7 +3855,6 @@ dependencies = [ "servo_arc", "servo_rand", "servo_url", - "std_test_override", "time 0.1.45", "url", "uuid", @@ -4987,7 +4986,6 @@ dependencies = [ "servo_atoms", "servo_url", "smallvec", - "std_test_override", "style_traits", "time 0.1.45", "uuid", @@ -5115,6 +5113,7 @@ dependencies = [ name = "servo-gst-plugin" version = "0.0.1" dependencies = [ + "cfg-if 1.0.0", "crossbeam-channel", "euclid", "glib", @@ -5332,7 +5331,6 @@ dependencies = [ "servo_config_plugins", "servo_geometry", "servo_url", - "std_test_override", "url", ] @@ -5692,13 +5690,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" -[[package]] -name = "std_test_override" -version = "0.0.1" -dependencies = [ - "embedder_traits", -] - [[package]] name = "str-buf" version = "1.0.6" @@ -5828,7 +5819,6 @@ dependencies = [ "servo_atoms", "servo_config", "servo_url", - "std_test_override", "style", "style_traits", ] diff --git a/Cargo.toml b/Cargo.toml index 4cc334367a0..01463ea3abb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -96,10 +96,15 @@ xml5ever = "0.17" [profile.release] opt-level = 3 +debug-assertions = true # Uncomment to profile on Linux: # debug = true # lto = false +[profile.production] +inherits = "release" +debug-assertions = false + [patch.crates-io] # If you need to temporarily test Servo with a local fork of some upstream # crate, add that here. Use the form: diff --git a/components/config/Cargo.toml b/components/config/Cargo.toml index b33e2c608ac..7671b55810c 100644 --- a/components/config/Cargo.toml +++ b/components/config/Cargo.toml @@ -24,8 +24,5 @@ servo_geometry = { path = "../geometry" } servo_url = { path = "../url" } url = { workspace = true } -[dev-dependencies] -std_test_override = { path = "../std_test_override" } - [target.'cfg(not(target_os = "android"))'.dependencies] dirs-next = "2.0" diff --git a/components/embedder_traits/Cargo.toml b/components/embedder_traits/Cargo.toml index 82621113882..283a8c52b66 100644 --- a/components/embedder_traits/Cargo.toml +++ b/components/embedder_traits/Cargo.toml @@ -11,6 +11,7 @@ name = "embedder_traits" path = "lib.rs" [dependencies] +cfg-if = { workspace = true } crossbeam-channel = { workspace = true } ipc-channel = { workspace = true } keyboard-types = { workspace = true } diff --git a/components/embedder_traits/build.rs b/components/embedder_traits/build.rs new file mode 100644 index 00000000000..25c737ffaa9 --- /dev/null +++ b/components/embedder_traits/build.rs @@ -0,0 +1,23 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +use std::error::Error; +use std::path::Path; + +fn main() -> Result<(), Box> { + // Cargo does not expose the profile name to crates or their build scripts, + // but we can extract it from OUT_DIR and set a custom cfg() ourselves. + let out = std::env::var("OUT_DIR")?; + let out = Path::new(&out); + let krate = out.parent().unwrap(); + let build = krate.parent().unwrap(); + let profile = build.parent().unwrap(); + if profile.file_name().unwrap() == "production" { + println!("cargo:rustc-cfg=servo_production"); + } else { + println!("cargo:rustc-cfg=servo_do_not_use_in_production"); + } + + Ok(()) +} diff --git a/components/embedder_traits/resources.rs b/components/embedder_traits/resources.rs index 04d3b0a706b..21cee14cc07 100644 --- a/components/embedder_traits/resources.rs +++ b/components/embedder_traits/resources.rs @@ -3,13 +3,25 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use std::path::PathBuf; -use std::sync::{Once, RwLock}; +use std::sync::RwLock; +use cfg_if::cfg_if; use lazy_static::lazy_static; lazy_static! { - static ref RES: RwLock>> = - RwLock::new(None); + static ref RES: RwLock>> = { + cfg_if! { + if #[cfg(servo_production)] { + RwLock::new(None) + } else { + // Static assert that this is really a non-production build, rather + // than a failure of the build script’s production check. + const _: () = assert!(cfg!(servo_do_not_use_in_production)); + + RwLock::new(Some(resources_for_tests())) + } + } + }; } pub fn set(reader: Box) { @@ -88,42 +100,13 @@ pub trait ResourceReaderMethods { fn sandbox_access_files_dirs(&self) -> Vec; } -// Can’t #[cfg(test)] the following because it breaks tests in dependent crates. - -pub fn set_for_tests() { - static ONCE: Once = Once::new(); - ONCE.call_once(|| set(resources_for_tests())); -} - -lazy_static::lazy_static! { - static ref CMD_RESOURCE_DIR: std::sync::Mutex> = std::sync::Mutex::new(None); -} - -fn resources_dir_path_for_tests() -> PathBuf { - // This needs to be called before the process is sandboxed - // as we only give permission to read inside the resources directory, - // not the permissions the "search" for the resources directory. - let mut dir = CMD_RESOURCE_DIR.lock().unwrap(); - if let Some(ref path) = *dir { - return PathBuf::from(path); - } - - // Try ./resources in the current directory, then each of its ancestors. - let mut path = std::env::current_dir().unwrap(); - loop { - path.push("resources"); - if path.is_dir() { - *dir = Some(path); - return dir.clone().unwrap(); - } - path.pop(); - - if !path.pop() { - panic!("Can't find resources directory") - } - } -} - +/// Bake all of our resources into this crate for tests, unless we are `cfg!(servo_production)`. +/// +/// Local non-production embedder builds (e.g. servoshell) can still override these with [`set`], +/// if runtime loading of prefs.json and other resources is needed. +/// +/// In theory this can be `#[cfg(servo_production)]`, but omitting the attribute ensures that the +/// code is always checked by the compiler, even if it later gets optimised out as dead code. fn resources_for_tests() -> Box { struct ResourceReader; impl ResourceReaderMethods for ResourceReader { @@ -131,12 +114,36 @@ fn resources_for_tests() -> Box { vec![] } fn sandbox_access_files_dirs(&self) -> Vec { - vec![resources_dir_path_for_tests()] + vec![] } fn read(&self, file: Resource) -> Vec { - let mut path = resources_dir_path_for_tests(); - path.push(file.filename()); - std::fs::read(path).expect("Can't read file") + match file { + Resource::Preferences => &include_bytes!("../../resources/prefs.json")[..], + Resource::BluetoothBlocklist => { + &include_bytes!("../../resources/gatt_blocklist.txt")[..] + }, + Resource::DomainList => &include_bytes!("../../resources/public_domains.txt")[..], + Resource::HstsPreloadList => { + &include_bytes!("../../resources/hsts_preload.json")[..] + }, + Resource::BadCertHTML => &include_bytes!("../../resources/badcert.html")[..], + Resource::NetErrorHTML => &include_bytes!("../../resources/neterror.html")[..], + Resource::UserAgentCSS => &include_bytes!("../../resources/user-agent.css")[..], + Resource::ServoCSS => &include_bytes!("../../resources/servo.css")[..], + Resource::PresentationalHintsCSS => { + &include_bytes!("../../resources/presentational-hints.css")[..] + }, + Resource::QuirksModeCSS => &include_bytes!("../../resources/quirks-mode.css")[..], + Resource::RippyPNG => &include_bytes!("../../resources/rippy.png")[..], + Resource::MediaControlsCSS => { + &include_bytes!("../../resources/media-controls.css")[..] + }, + Resource::MediaControlsJS => { + &include_bytes!("../../resources/media-controls.js")[..] + }, + Resource::CrashHTML => &include_bytes!("../../resources/crash.html")[..], + } + .to_owned() } } Box::new(ResourceReader) diff --git a/components/net/Cargo.toml b/components/net/Cargo.toml index 93bc042e87c..928cddb4bb9 100644 --- a/components/net/Cargo.toml +++ b/components/net/Cargo.toml @@ -70,7 +70,6 @@ webpki-roots = { workspace = true } [dev-dependencies] futures = { version = "0.3", features = ["compat"] } -std_test_override = { path = "../std_test_override" } tokio-test = "0.4" tokio-stream = { version = "0.1", features = ["net"] } hyper = { workspace = true, features = ["full"] } diff --git a/components/net_traits/Cargo.toml b/components/net_traits/Cargo.toml index 92e82dd0704..13005f40790 100644 --- a/components/net_traits/Cargo.toml +++ b/components/net_traits/Cargo.toml @@ -40,6 +40,3 @@ time = { workspace = true } url = { workspace = true } uuid = { workspace = true } webrender_api = { git = "https://github.com/servo/webrender" } - -[dev-dependencies] -std_test_override = { path = "../std_test_override" } diff --git a/components/script_traits/Cargo.toml b/components/script_traits/Cargo.toml index f9f79814a9c..7d6978dff02 100644 --- a/components/script_traits/Cargo.toml +++ b/components/script_traits/Cargo.toml @@ -45,6 +45,3 @@ webdriver = { workspace = true } webgpu = { path = "../webgpu" } webrender_api = { workspace = true } webxr-api = { git = "https://github.com/servo/webxr", features = ["ipc"] } - -[dev-dependencies] -std_test_override = { path = "../std_test_override" } diff --git a/components/std_test_override/Cargo.toml b/components/std_test_override/Cargo.toml deleted file mode 100644 index 483d46c0ec4..00000000000 --- a/components/std_test_override/Cargo.toml +++ /dev/null @@ -1,14 +0,0 @@ -[package] -name = "std_test_override" -version = "0.0.1" -authors = ["The Servo Project Developers"] -license = "MPL-2.0" -edition = "2018" -publish = false - -[lib] -name = "test" -path = "lib.rs" - -[dependencies] -embedder_traits = { path = "../embedder_traits" } diff --git a/components/std_test_override/lib.rs b/components/std_test_override/lib.rs deleted file mode 100644 index 0d3d26c2fbf..00000000000 --- a/components/std_test_override/lib.rs +++ /dev/null @@ -1,14 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ - -#![feature(test)] - -extern crate test; - -pub use test::*; - -pub fn test_main_static(tests: &[&TestDescAndFn]) { - embedder_traits::resources::set_for_tests(); - test::test_main_static(tests); -} diff --git a/docs/HACKING_QUICKSTART.md b/docs/HACKING_QUICKSTART.md index 5efca5a2377..24960afb131 100644 --- a/docs/HACKING_QUICKSTART.md +++ b/docs/HACKING_QUICKSTART.md @@ -35,9 +35,19 @@ Building Servo is quite easy. Install the prerequisites described in the [README *Note: on Mac, you might run into an SSL issue while compiling. You'll find a solution to this problem [here](https://github.com/sfackler/rust-openssl/issues/255).* -The `-d` option means "debug build". You can also build with the `-r` option which means "release build". Building with `-d` will allow you to use a debugger (lldb). A `-r` build is more performant. Release builds are slower to build. +There are three main build profiles, which you can build and use independently of one another: -You can use and build a release build and a debug build in parallel. +* debug builds, which allow you to use a debugger (lldb) +* release builds, which are slower to build but more performant +* production builds, which are used for official releases only + +| profile | mach option | optimised? | debug
info? | debug
assertions? | finds resources in
current working dir? | +|---|---|---|---|---|---| +| debug | `-d` | no | yes | yes | yes | +| release | `-r` | yes | no | yes(!) | yes | +| production | `--profile production` | yes | yes | no | no | + +You can change these settings in a servobuild file (see [servobuild.example](../servobuild.example)) or in the root [Cargo.toml](../Cargo.toml). ## Running Servo diff --git a/ports/gstplugin/Cargo.toml b/ports/gstplugin/Cargo.toml index 739610d4959..a02cae8e437 100644 --- a/ports/gstplugin/Cargo.toml +++ b/ports/gstplugin/Cargo.toml @@ -15,6 +15,7 @@ crate-type = ["cdylib"] path = "lib.rs" [dependencies] +cfg-if = { workspace = true } crossbeam-channel = { workspace = true } euclid = { workspace = true } glib = "0.9" diff --git a/ports/gstplugin/build.rs b/ports/gstplugin/build.rs index d1555ca6b37..bf58fffa32f 100644 --- a/ports/gstplugin/build.rs +++ b/ports/gstplugin/build.rs @@ -2,6 +2,24 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -fn main() { - gst_plugin_version_helper::get_info() +use std::error::Error; +use std::path::Path; + +fn main() -> Result<(), Box> { + // Cargo does not expose the profile name to crates or their build scripts, + // but we can extract it from OUT_DIR and set a custom cfg() ourselves. + let out = std::env::var("OUT_DIR")?; + let out = Path::new(&out); + let krate = out.parent().unwrap(); + let build = krate.parent().unwrap(); + let profile = build.parent().unwrap(); + if profile.file_name().unwrap() == "production" { + println!("cargo:rustc-cfg=servo_production"); + } else { + println!("cargo:rustc-cfg=servo_do_not_use_in_production"); + } + + gst_plugin_version_helper::get_info(); + + Ok(()) } diff --git a/ports/gstplugin/resources.rs b/ports/gstplugin/resources.rs index 1dfb8d96cad..25c1cfae7ce 100644 --- a/ports/gstplugin/resources.rs +++ b/ports/gstplugin/resources.rs @@ -10,6 +10,7 @@ use std::path::PathBuf; use std::sync::Mutex; use std::{env, fs}; +use cfg_if::cfg_if; use servo::embedder_traits::resources::{self, Resource}; lazy_static::lazy_static! { @@ -51,18 +52,29 @@ fn resources_dir_path() -> PathBuf { path.pop(); } - // Try ./resources in the current directory, then each of its ancestors. - let mut path = std::env::current_dir().unwrap(); - loop { - path.push("resources"); - if path.is_dir() { - *dir = Some(path); - return dir.clone().unwrap(); - } - path.pop(); - - if !path.pop() { + cfg_if! { + if #[cfg(servo_production)] { panic!("Can't find resources directory") + } else { + // Static assert that this is really a non-production build, rather + // than a failure of the build script’s production check. + const _: () = assert!(cfg!(servo_do_not_use_in_production)); + + // Try ./resources in the current directory, then each of its ancestors. + // Not to be used in production builds without considering the security implications! + let mut path = std::env::current_dir().unwrap(); + loop { + path.push("resources"); + if path.is_dir() { + *dir = Some(path); + return dir.clone().unwrap(); + } + path.pop(); + + if !path.pop() { + panic!("Can't find resources directory") + } + } } } } diff --git a/ports/servoshell/build.rs b/ports/servoshell/build.rs index 2aadf13beed..961e30ba36e 100644 --- a/ports/servoshell/build.rs +++ b/ports/servoshell/build.rs @@ -2,9 +2,25 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use std::error::Error; +use std::path::Path; + use vergen::EmitBuilder; -fn main() { +fn main() -> Result<(), Box> { + // Cargo does not expose the profile name to crates or their build scripts, + // but we can extract it from OUT_DIR and set a custom cfg() ourselves. + let out = std::env::var("OUT_DIR")?; + let out = Path::new(&out); + let krate = out.parent().unwrap(); + let build = krate.parent().unwrap(); + let profile = build.parent().unwrap(); + if profile.file_name().unwrap() == "production" { + println!("cargo:rustc-cfg=servo_production"); + } else { + println!("cargo:rustc-cfg=servo_do_not_use_in_production"); + } + #[cfg(windows)] { let mut res = winres::WindowsResource::new(); @@ -36,4 +52,6 @@ fn main() { // linker to locate them. See `man dyld` for more info. #[cfg(target_os = "macos")] println!("cargo:rustc-link-arg=-Wl,-rpath,@executable_path/lib/"); + + Ok(()) } diff --git a/ports/servoshell/prefs.rs b/ports/servoshell/prefs.rs index 325b766d4bb..d11754b8608 100644 --- a/ports/servoshell/prefs.rs +++ b/ports/servoshell/prefs.rs @@ -62,7 +62,6 @@ pub fn register_user_prefs(opts_matches: &Matches) { #[cfg(test)] fn test_parse_pref(arg: &str) { - servo::embedder_traits::resources::set_for_tests(); let mut opts = getopts::Options::new(); opts.optmulti("", "pref", "", ""); let args = vec!["servo".to_string(), "--pref".to_string(), arg.to_string()]; diff --git a/ports/servoshell/resources.rs b/ports/servoshell/resources.rs index d27a1714969..8a713522785 100644 --- a/ports/servoshell/resources.rs +++ b/ports/servoshell/resources.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use std::sync::Mutex; use std::{env, fs}; +use cfg_if::cfg_if; use servo::embedder_traits::resources::{self, Resource}; lazy_static::lazy_static! { @@ -47,18 +48,29 @@ fn resources_dir_path() -> PathBuf { path.pop(); } - // Try ./resources in the current directory, then each of its ancestors. - let mut path = std::env::current_dir().unwrap(); - loop { - path.push("resources"); - if path.is_dir() { - *dir = Some(path); - return dir.clone().unwrap(); - } - path.pop(); - - if !path.pop() { + cfg_if! { + if #[cfg(servo_production)] { panic!("Can't find resources directory") + } else { + // Static assert that this is really a non-production build, rather + // than a failure of the build script’s production check. + const _: () = assert!(cfg!(servo_do_not_use_in_production)); + + // Try ./resources in the current directory, then each of its ancestors. + // Not to be used in production builds without considering the security implications! + let mut path = std::env::current_dir().unwrap(); + loop { + path.push("resources"); + if path.is_dir() { + *dir = Some(path); + return dir.clone().unwrap(); + } + path.pop(); + + if !path.pop() { + panic!("Can't find resources directory") + } + } } } } diff --git a/python/servo/build_commands.py b/python/servo/build_commands.py index 4b0858f6ff1..d354ca02df4 100644 --- a/python/servo/build_commands.py +++ b/python/servo/build_commands.py @@ -62,8 +62,12 @@ class MachCommands(CommandBase): opts = params or [] has_media_stack = "media-gstreamer" in self.features - if build_type == BuildType.RELEASE: + if build_type.is_release(): opts += ["--release"] + elif build_type.is_dev(): + pass # there is no argument for debug + else: + opts += ["--profile", build_type.profile] if jobs is not None: opts += ["-j", jobs] diff --git a/python/servo/command_base.py b/python/servo/command_base.py index 137dfeea090..752cdda4474 100644 --- a/python/servo/command_base.py +++ b/python/servo/command_base.py @@ -7,7 +7,7 @@ # option. This file may not be copied, modified, or distributed # except according to those terms. -from __future__ import print_function +from __future__ import print_function, annotations import contextlib from enum import Enum @@ -27,6 +27,7 @@ import tarfile import urllib import zipfile +from dataclasses import dataclass from errno import ENOENT as NO_SUCH_FILE_OR_DIRECTORY from glob import glob from os import path @@ -46,10 +47,44 @@ from servo.util import download_file, get_default_cache_dir NIGHTLY_REPOSITORY_URL = "https://servo-builds2.s3.amazonaws.com/" -class BuildType(Enum): - """ The build type of this Servo build. Either `DEV` or `RELEASE`.""" - DEV = 1 - RELEASE = 2 +@dataclass +class BuildType: + class Kind(Enum): + DEV = 1 + RELEASE = 2 + CUSTOM = 3 + + kind: Kind + profile: Optional[str] + + def dev() -> BuildType: + return BuildType(BuildType.Kind.DEV, None) + + def release() -> BuildType: + return BuildType(BuildType.Kind.RELEASE, None) + + def custom(profile: str) -> BuildType: + return BuildType(BuildType.Kind.CUSTOM, profile) + + def is_dev(self) -> bool: + return self.kind == BuildType.Kind.DEV + + def is_release(self) -> bool: + return self.kind == BuildType.Kind.RELEASE + + def is_custom(self) -> bool: + return self.kind == BuildType.Kind.CUSTOM + + def directory_name(self) -> str: + if self.is_dev(): + return "debug" + elif self.is_release(): + return "release" + else: + return self.profile + + def __eq__(self, other: object) -> bool: + raise Exception("BUG: do not compare BuildType with ==") @contextlib.contextmanager @@ -290,8 +325,7 @@ class CommandBase(object): base_path = util.get_target_dir() base_path = path.join(base_path, "android", self.config["android"]["target"]) apk_name = "servoapp.apk" - build_type_string = "release" if build_type == BuildType.RELEASE else "debug" - return path.join(base_path, build_type_string, apk_name) + return path.join(base_path, build_type.directory_name(), apk_name) def get_binary_path(self, build_type: BuildType, target=None, android=False, simpleservo=False): base_path = util.get_target_dir() @@ -310,8 +344,7 @@ class CommandBase(object): else: binary_name = "libsimpleservo.so" - build_type_string = "release" if build_type == BuildType.RELEASE else "debug" - binary_path = path.join(base_path, build_type_string, binary_name) + binary_path = path.join(base_path, build_type.directory_name(), binary_name) if not path.exists(binary_path): raise BuildNotFound('No Servo binary found. Perhaps you forgot to run `./mach build`?') @@ -701,6 +734,8 @@ class CommandBase(object): CommandArgument('--dev', '--debug', '-d', group="Build Type", action='store_true', help='Build in development mode'), + CommandArgument('--profile', group="Build Type", + help='Build with custom Cargo profile'), ] if build_configuration: @@ -762,9 +797,12 @@ class CommandBase(object): if build_type: # If `build_type` already exists in kwargs we are doing a recursive dispatch. if 'build_type' not in kwargs: - kwargs['build_type'] = self.configure_build_type(kwargs['release'], kwargs['dev']) + kwargs['build_type'] = self.configure_build_type( + kwargs['release'], kwargs['dev'], kwargs['profile'], + ) kwargs.pop('release', None) kwargs.pop('dev', None) + kwargs.pop('profile', None) if build_configuration: self.configure_cross_compilation(kwargs['target'], kwargs['android'], kwargs['win_arm64']) @@ -781,24 +819,31 @@ class CommandBase(object): return decorator_function - def configure_build_type(self, release: bool, dev: bool) -> BuildType: - if release and dev: - print("Please specify either --dev (-d) for a development") - print(" build, or --release (-r) for an optimized build.") - sys.exit(1) + def configure_build_type(self, release: bool, dev: bool, profile: Optional[str]) -> BuildType: + option_count = release + dev + (profile is not None) - if not release and not dev: + if option_count > 1: + print("Please specify either --dev (-d) for a development") + print(" build, or --release (-r) for an optimized build,") + print(" or --profile PROFILE for a custom Cargo profile.") + sys.exit(1) + elif option_count < 1: if self.config["build"]["mode"] == "dev": print("No build type specified, but .servobuild specified `--dev`.") - dev = True + return BuildType.dev() elif self.config["build"]["mode"] == "release": print("No build type specified, but .servobuild specified `--release`.") - release = True + return BuildType.release() else: print("No build type specified so assuming `--dev`.") - dev = True + return BuildType.dev() - return BuildType.DEV if dev else BuildType.RELEASE + if release: + return BuildType.release() + elif dev: + return BuildType.dev() + else: + return BuildType.custom(profile) def configure_cross_compilation( self, diff --git a/python/servo/package_commands.py b/python/servo/package_commands.py index 8dff69b29c2..e9e5465ec0a 100644 --- a/python/servo/package_commands.py +++ b/python/servo/package_commands.py @@ -155,7 +155,12 @@ class PackageCommands(CommandBase): else: arch_string = "Arm" - build_type_string = "Debug" if build_type == BuildType.DEV else "Release" + if build_type.is_dev(): + build_type_string = "Debug" + elif build_type.is_release(): + build_type_string = "Release" + else: + raise Exception("TODO what should this be?") flavor_name = "Main" if flavor is not None: diff --git a/servobuild.example b/servobuild.example index 1e3d0986bc9..3e0897a845c 100644 --- a/servobuild.example +++ b/servobuild.example @@ -21,7 +21,7 @@ android = false # Enable `debug_assert!` macros in release mode -debug-assertions = false +debug-assertions = true # Set "debug-mozjs" or use `mach build --debug-mozjs` to build a debug spidermonkey. debug-mozjs = false diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml index 38439c1f280..e55c057a1f0 100644 --- a/tests/unit/style/Cargo.toml +++ b/tests/unit/style/Cargo.toml @@ -23,4 +23,3 @@ servo_config = {path = "../../../components/config"} servo_url = {path = "../../../components/url"} style = {path = "../../../components/style", features = ["servo"]} style_traits = {path = "../../../components/style_traits"} -std_test_override = { path = "../../../components/std_test_override" } diff --git a/tests/wpt/meta-legacy-layout/css/css-color/currentcolor-003.html.ini b/tests/wpt/meta-legacy-layout/css/css-color/currentcolor-003.html.ini index 8f2ab03eb22..5fe269b0cab 100644 --- a/tests/wpt/meta-legacy-layout/css/css-color/currentcolor-003.html.ini +++ b/tests/wpt/meta-legacy-layout/css/css-color/currentcolor-003.html.ini @@ -1,2 +1,2 @@ [currentcolor-003.html] - expected: FAIL + expected: CRASH