From 26bbfe9b551c268188d952b1b565da890d3eb6f4 Mon Sep 17 00:00:00 2001 From: Jonathan Schwender <55576758+jschwe@users.noreply.github.com> Date: Mon, 24 Jun 2024 13:46:43 +0200 Subject: [PATCH] Make `crown` optional (#32494) * Make `crown` optional Add the optional `--use-crown` flag to mach * --use-crown for all platforms in CI Signed-off-by: Jonathan Schwender * Add documentation for `--use-crown` Signed-off-by: Jonathan Schwender * Update python/servo/command_base.py Co-authored-by: Mukilan Thiyagarajan Signed-off-by: Jonathan Schwender * Raise Error if CARGO_BUILD_RUSTC conflicts with --use-crown Signed-off-by: Jonathan Schwender * add dummy RUSTFLAG to trigger re-checking Signed-off-by: Jonathan Schwender --------- Signed-off-by: Jonathan Schwender Signed-off-by: Jonathan Schwender Co-authored-by: Mukilan Thiyagarajan --- .cargo/config.toml | 1 - .github/workflows/android.yml | 2 +- .github/workflows/linux.yml | 2 +- .github/workflows/mac.yml | 2 +- .github/workflows/windows.yml | 2 +- docs/HACKING_QUICKSTART.md | 37 ++++++++++++++++++++++++----------- python/servo/command_base.py | 23 ++++++++++++++++++++++ 7 files changed, 53 insertions(+), 16 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index e1bdd061199..f08e8c6a42e 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -34,4 +34,3 @@ RUSTC_BOOTSTRAP = "crown,script,style_tests" [build] rustdocflags = ["--document-private-items"] -rustc = "crown" diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml index efc6e0c364c..1da4d73c01c 100644 --- a/.github/workflows/android.yml +++ b/.github/workflows/android.yml @@ -75,7 +75,7 @@ jobs: env: ANDROID_NDK_ROOT: ${{ steps.setup-ndk.outputs.ndk-path }} run: | - python3 ./mach build --locked --android --target ${{ matrix.arch }} --${{ inputs.profile }} + python3 ./mach build --use-crown --locked --android --target ${{ matrix.arch }} --${{ inputs.profile }} cp -r target/cargo-timings target/cargo-timings-android-${{ matrix.arch }} # TODO: This is disabled since APK crashes during startup. # See https://github.com/servo/servo/issues/31134 diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 5ca80ad6a16..1dc56f91532 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -111,7 +111,7 @@ jobs: run: python3 ./mach test-tidy --no-progress --all - name: Build (${{ inputs.profile }}) run: | - python3 ./mach build --locked --${{ inputs.profile }} + python3 ./mach build --use-crown --locked --${{ inputs.profile }} cp -r target/cargo-timings target/cargo-timings-linux - name: Smoketest run: xvfb-run python3 ./mach smoketest --${{ inputs.profile }} diff --git a/.github/workflows/mac.yml b/.github/workflows/mac.yml index b9d14cf8a69..582d2ec7f21 100644 --- a/.github/workflows/mac.yml +++ b/.github/workflows/mac.yml @@ -96,7 +96,7 @@ jobs: brew install gnu-tar - name: Build (${{ inputs.profile }}) run: | - python3 ./mach build --locked --${{ inputs.profile }} + python3 ./mach build --use-crown --locked --${{ inputs.profile }} cp -r target/cargo-timings target/cargo-timings-macos - name: Smoketest uses: nick-fields/retry@v3 diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 26d8388f8df..9133fd30c1a 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -94,7 +94,7 @@ jobs: "$env:WIX\bin" >> $env:GITHUB_PATH - name: Build (${{ inputs.profile }}) run: | - python mach build --locked --${{ inputs.profile }} + python mach build --use-crown --locked --${{ inputs.profile }} cp C:\a\servo\servo\target\cargo-timings C:\a\servo\servo\target\cargo-timings-windows -Recurse - name: Copy resources run: cp D:\a\servo\servo\resources C:\a\servo\servo -Recurse diff --git a/docs/HACKING_QUICKSTART.md b/docs/HACKING_QUICKSTART.md index 6094e8b913c..14fb3730c85 100644 --- a/docs/HACKING_QUICKSTART.md +++ b/docs/HACKING_QUICKSTART.md @@ -78,8 +78,10 @@ The `--` separates `mach` options from `servo` options. This is not required, bu This guide only covers the most important options. Be sure to look at all the available mach commands and the servo options: ```shell -./mach --help # mach options -./mach run -- --help # servo options +./mach --help # mach options +./mach run -- --help # servo options +./mach build # Build servo with the default configuration +./mach build --use-crown # Enable the crown linting tool - recommended when hacking on DOM code. ``` ## Some basic Rust @@ -183,26 +185,39 @@ work done by `./mach` (and vice versa). You can override this in a `.vscode/settings.json` file: -``` +```json { "rust-analyzer.check.overrideCommand": [ "./mach", "check", "--message-format=json" ], "rust-analyzer.cargo.buildScripts.overrideCommand": [ "./mach", "check", "--message-format=json" ], - "rust-analyzer.rustfmt.overrideCommand": [ "./mach", "fmt" ], + "rust-analyzer.rustfmt.overrideCommand": [ "./mach", "fmt" ] +} +``` + +If you are hacking on DOM code it is recommended to build servo with `./mach build --use-crown` and configure +rust-analyzer to do the same by adding `--use-crown` to the `rust-analyzer.check.overrideCommand`: + +```json +{ + "rust-analyzer.check.overrideCommand": [ + "./mach", "check", "--message-format=json", "--use-crown" ], + "rust-analyzer.cargo.buildScripts.overrideCommand": [ + "./mach", "check", "--message-format=json", "--use-crown" ], + "rust-analyzer.rustfmt.overrideCommand": [ "./mach", "fmt" ] } ``` If that still causes problems, then supplying a different target directory should fix this (although it will increase the amount of disc space used). -``` +```json { "rust-analyzer.checkOnSave.overrideCommand": [ "./mach", "check", "--message-format=json", "--target-dir", "target/lsp" ], "rust-analyzer.cargo.buildScripts.overrideCommand": [ "./mach", "check", "--message-format=json", "--target-dir", "target/lsp" ], - "rust-analyzer.rustfmt.overrideCommand": [ "./mach", "fmt" ], + "rust-analyzer.rustfmt.overrideCommand": [ "./mach", "fmt" ] } ``` @@ -211,11 +226,11 @@ where `/nix/store/.../crown` is the output of `nix-shell etc/shell.nix --run 'co These settings should be enough to not need to run `code .` from within a `nix-shell etc/shell.nix`, but it wouldn’t hurt to try that if you still have problems. -``` +```json { "rust-analyzer.server.extraEnv": { - "CARGO_BUILD_RUSTC": "/nix/store/.../crown", - }, + "CARGO_BUILD_RUSTC": "/nix/store/.../crown" + } } ``` @@ -234,11 +249,11 @@ info: component 'llvm-tools' for target 'x86_64-unknown-linux-gnu' is up to date Then configure either your sysroot path or proc macro server path in `.vscode/settings.json`: -``` +```json { "rust-analyzer.procMacro.enable": true, "rust-analyzer.cargo.sysroot": "[paste what you copied]", - "rust-analyzer.procMacro.server": "[paste what you copied]/libexec/rust-analyzer-proc-macro-srv", + "rust-analyzer.procMacro.server": "[paste what you copied]/libexec/rust-analyzer-proc-macro-srv" } ``` diff --git a/python/servo/command_base.py b/python/servo/command_base.py index f9c46143320..71fe267e10e 100644 --- a/python/servo/command_base.py +++ b/python/servo/command_base.py @@ -863,6 +863,12 @@ class CommandBase(object): help='Build with frame pointer enabled, used by the background hang monitor.', ), CommandArgument('--without-wgl', group="Feature Selection", default=None, action='store_true'), + CommandArgument( + '--use-crown', + default=False, + action='store_true', + help="Enable Servo's `crown` linter tool" + ) ] def decorator_function(original_function): @@ -977,6 +983,7 @@ class CommandBase(object): env=None, verbose=False, debug_mozjs=False, with_debug_assertions=False, with_frame_pointer=False, without_wgl=False, + use_crown=False, target_override: Optional[str] = None, **_kwargs ): @@ -1009,6 +1016,22 @@ class CommandBase(object): if command == 'rustc': args += ["--lib", "--crate-type=cdylib"] + if use_crown: + if 'CARGO_BUILD_RUSTC' in env: + current_rustc = env['CARGO_BUILD_RUSTC'] + if current_rustc != 'crown': + print('Error: `mach` was called with `--use-crown` while `CARGO_BUILD_RUSTC` was' + f'already set to `{current_rustc}` in the parent environment.\n' + 'These options conflict, please specify only one of them.') + sys.exit(1) + env['CARGO_BUILD_RUSTC'] = 'crown' + # Changing `RUSTC` or `CARGO_BUILD_RUSTC` does not cause `cargo check` to + # recheck files with the new compiler. `cargo build` is not affected and + # triggers a rebuild as expected. To also make `check` work as expected, + # we add a dummy `cfg` to RUSTFLAGS when using crown, so as to have different + # RUSTFLAGS when using `crown`, to reliably trigger re-checking. + env['RUSTFLAGS'] = env.get('RUSTFLAGS', "") + " --cfg=crown" + if "-p" not in cargo_args: # We're building specific package, that may not have features features = list(self.features) if self.enable_media: