Heads up! This PR modifies the following files:

• @wafflespeanut: python/servo/command_base.py
• @KiChjang: components/script/dom/bindings/codegen/GlobalGen.py, components/script/dom/mod.rs, components/script/Cargo.toml, components/script/build.rs, components/script/CMakeLists.txt, components/script/dom/bindings/codegen/pythonpath.py, components/script/script_runtime.rs

Warning

• These commits modify gfx and script code, but no tests are modified. Please consider adding a test!

@bors-servo try (by request)

🔒 Merge conflict

Given that Gecko is dropping 2013, is there any point to us trying to?

@bors-servo try

⌛ Trying commit d5c2e45 with merge e715025...

💔 Test failed - mac-dev-unit

☔ The latest upstream changes (presumably #11785) made this pull request unmergeable. Please resolve the merge conflicts.

@metajack can we get ninja installed on the builders, or should I try to figure out the parallel make issue?

☔ The latest upstream changes (presumably #12451) made this pull request unmergeable. Please resolve the merge conflicts.

This is looking good. I had a few comments, but the main stuff left to do is:

1. Land dependent PRs so that we can remove the [replace] section.
2. Update the other Cargo.locks if needed

Reviewed 9 of 9 files at r1, 11 of 11 files at r2, 1 of 1 files at r3, 2 of 2 files at r4, 10 of 10 files at r5, 1 of 1 files at r6, 2 of 2 files at r7, 7 of 7 files at r8, 1 of 1 files at r9, 1 of 1 files at r10, 1 of 1 files at r11, 1 of 1 files at r12, 8 of 8 files at r13, 1 of 1 files at r14, 1 of 1 files at r15, 1 of 1 files at r16, 1 of 1 files at r17, 1 of 1 files at r18, 2 of 2 files at r19.
Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

components/script/build.rs, line 11 [r10] (raw file):

    // parallelizes cmake's output properly.  (Cmake generates
    // separate makefiles, each of which try to build
    // ParserResults.pkl, and then stomp on eachother.)

I'm not sure what you mean by it generating separate makefiles. Which build generator did that? Nmake? You might mention that the VS generator serializes all custom commands, so they won't run in parallel even if it is successful. I seem to recall nmake had a similar behavior, but not 100% sure.

components/script/build.rs, line 20 [r11] (raw file):

    if target.contains("windows-msvc") {
        // because we're using ninja, we need to explicitly set these
        // to VC++, otherwise it'll try to use cc

I think we should use vcvars.bat to do this and check for the right stuff in mach. I don't remember needing to do any of this, but I was definitely running in cmd.exe shells that had run vcvars.bat

components/script/CMakeLists.txt, line 93 [r2] (raw file):

endforeach()

PREPEND(globalgen_out ${CMAKE_BINARY_DIR}/ ${globalgen_base_src})

Where were these going before? It's not clear why this bit is needed.

components/script/CMakeLists.txt, line 82 [r19] (raw file):

# below would cause GlobalGen.py to be executed each time.
add_custom_target(ParserResults ALL DEPENDS ParserResults.pkl)
add_custom_target(generate-bindings ALL)

Isn't this weird to have a target that has no dependencies?

components/servo/Cargo.toml, line 21 [r4] (raw file):

bench = false

[replace]

We'll need to remove this when it's ready to finally land.

Comments from Reviewable

Review status: all files reviewed at latest revision, 5 unresolved discussions, some commit checks failed.

components/script/build.rs, line 11 [r10] (raw file):

So in our case, we had a command to generate ParserResults.pkl, and then each individual binding depended on ParserResults.pkl. Without an intermediate target, every binding target re-created ParserResults.pkl first -- which would often error out with a pickle error, as one command was trying to read it while something else was stomping on it.

This happened with makefiles, msbuild, or ninja. In a later commit once I figured out what was going on I fixed it, so in theory we could use any generator here, including msbuild... I kept it using ninja for msvc targets though.

components/script/build.rs, line 20 [r11] (raw file):

components/script/CMakeLists.txt, line 93 [r2] (raw file):

components/script/CMakeLists.txt, line 82 [r19] (raw file):

Comments from Reviewable

Hmm, we'll need to bump version numbers on everything as well... I forgot to do that, I'll make more PRs.

@bors-servo r+

📌 Commit ae117c1 has been approved by larsbergstrom

⌛ Testing commit ae117c1 with merge ff970e4...

💔 Test failed - linux-dev

Gah, looks like the CEF (and possibly geckolib?) Cargo.lock files are not updated:

error: failed to select a version for `expat-sys` (required by `servo-fontconfig-sys`):
all possible versions conflict with previously selected versions of `expat-sys`
  version 2.1.2 in use by expat-sys v2.1.2
  possible versions to select: 2.1.4

An easy way to "fix" this is to copy the components/servo/Cargo.lock over them and then do ./mach build-geckolib and ./mach build-cef to remove the extra bits.

Once @SimonSapin lands the removal of the [replace] for geckolib, I can land the move to cargo workspaces and we'll only have one Cargo.lock file.

@bors-servo r+

(the additional checksum blocks in geckolib seem spurious, but the builders will tell us if they're redundant via checking for a changed Cargo.lock file)

📌 Commit dd37716 has been approved by larsbergstrom

⌛ Testing commit dd37716 with merge ec53136...

☀️ Test successful - arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows-dev