MSVC support for Servo, and CMake builds for native code

[vvuk]

This is the base PR for MSVC builds of servo and dependent crates. It's got replacements in the Cargo.toml to pull in the right versions, to make sure that crates were properly converted to CMake for all other platforms, not just Windows. (Servo builds with MSVC 2015 with this PR; also with 2013, though a manual change in rust-mozjs to select a different set of bindings is needed.)

This PR isn't quite ready yet, but I want bors-servo to do builds.

---

This change is

[highfive]

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

[highfive]

Warning

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

[aneeshusa]

@bors-servo try (by request)

[bors-servo]

🔒 Merge conflict

[Ms2ger]

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

[vvuk]

@bors-servo try

[bors-servo]

⌛ Trying commit d5c2e45 with merge e715025...

[bors-servo]

💔 Test failed - mac-dev-unit

[bors-servo]

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

[vvuk]

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

[bors-servo]

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

[metajack]

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

[vvuk]

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

---

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

Previously, metajack (Jack Moffitt) wrote…

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.

I updated this comment to explain what was going on in the latest commit; but basically, if cmake has a custom command A for a target (RESULT), and that target depends on the output of another custom command B, it creates a rule that runs both B and A for RESULT.

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):

Previously, metajack (Jack Moffitt) wrote…

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

The issue is that cmake makefiles/ninja/etc. will prefer cc/gcc before cl if it finds them -- so if you're running from a shell that has cc/gcc in the path at all, it won't choose cl. The msbuild cmake generator prefers cl.

---

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

Previously, metajack (Jack Moffitt) wrote…

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

They're all being generated in the cmake build dir, script-.../out/build. We need them in script-.../out. For the Bindings/*, we have the install(DIRECTORY... ) command, but for these one-off files that are not in the Bindings dir, we have to manually prepend the "build/" bit.

---

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

Previously, metajack (Jack Moffitt) wrote…

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

It does -- we add dependencies to it in the foreach() loop below. But we have to create the target first before doing so.

---

Comments from Reviewable

[vvuk]

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

[larsbergstrom]

@bors-servo r+

[bors-servo]

📌 Commit ae117c1 has been approved by larsbergstrom

[bors-servo]

⌛ Testing commit ae117c1 with merge ff970e4...

[bors-servo]

💔 Test failed - linux-dev

[larsbergstrom]

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.

[larsbergstrom]

@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)

[bors-servo]

📌 Commit dd37716 has been approved by larsbergstrom

[bors-servo]

⌛ Testing commit dd37716 with merge ec53136...

[bors-servo]

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