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 <jonathan.schwender@huawei.com>

* Add documentation for `--use-crown`

Signed-off-by: Jonathan Schwender <jonathan.schwender@huawei.com>

* Update python/servo/command_base.py

Co-authored-by: Mukilan Thiyagarajan <mukilanthiagarajan@gmail.com>
Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>

* Raise Error if CARGO_BUILD_RUSTC conflicts with --use-crown

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>

* add dummy RUSTFLAG to trigger re-checking

Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>

---------

Signed-off-by: Jonathan Schwender <jonathan.schwender@huawei.com>
Signed-off-by: Jonathan Schwender <schwenderjonathan@gmail.com>
Co-authored-by: Mukilan Thiyagarajan <mukilanthiagarajan@gmail.com>

.cargo/config.toml

@@ -34,4 +34,3 @@ RUSTC_BOOTSTRAP = "crown,script,style_tests"
 
 [build]
 rustdocflags = ["--document-private-items"]
-rustc = "crown"

.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

.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  }}

.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

.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

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"
 }
 ```
 

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: