Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Stylo and WebRender to src/tools/cargotest #44603

Merged
merged 4 commits into from
Oct 25, 2017
Merged

Conversation

SimonSapin
Copy link
Contributor

This is a subset of Servo that takes relatively less time to compile and does not use unstable Rust features.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@SimonSapin
Copy link
Contributor Author

@aturon
Copy link
Member

aturon commented Sep 15, 2017

Awesome, thanks @SimonSapin!

@alexcrichton, see any issue with this?

@alexcrichton
Copy link
Member

Thanks yeah! Looking into this to make sure it can work. Right now cargotest runs on the x86_64-gnu-aux builder where the last successful run clocked in at 1h51m. Locally it took my computer 208s to compile stylo_tests/selectors in the Servo repository (not accounting for download times) and 140s for the webrender repo. I normally multiply by 3-ish for travis's less parallelism as well as using a debug compiler.

Along those lines I'm a little worried that this is going to push us over the 2 hour "soft limit". I think along those lines to land this PR we should allocate a new builder rather than tack on an existing one. We've got some good headroom with capacity on Travis and seeing how this is indeed directly in Mozilla's interests (the organization footing the Travis bill currently) I think it's fine to just go ahead and do this.

@SimonSapin to do this, can you...

  • Copy src/ci/docker/x86_64-gnu-aux to src/ci/docker/x86_64-gnu-cargotest
  • Change this line of the Dockerfile in x86_64-gnu-cargotest to instead look like this line, namely ../x.py test src/tools/cargotest. That'll make this "the builder for cargotset"
  • Copy this section of appveyor.yml, deleting RUST_CHECK_TARGET and adding a SCRIPT environment variable like this which also does ./x.py test src/tools/cargotest.
  • Delete this line to ensure the aux builder today doesn't continue to do double-testing of cargotest.

That should give us tons of headroom for expanding cargotest after that!

@alexcrichton alexcrichton added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 15, 2017
@SimonSapin
Copy link
Contributor Author

@alexcrichton Done, and added a line in .travis.yml. I don’t know how to test that last commit, though.

@alexcrichton
Copy link
Member

@bors: r+

Oh that's ok, that's what we have bors for!

@bors
Copy link
Contributor

bors commented Sep 15, 2017

📌 Commit a589684 has been approved by alexcrichton

@alexcrichton
Copy link
Member

@bors: rollup

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Sep 16, 2017
Add Stylo and WebRender to src/tools/cargotest

This is a subset of Servo that takes relatively less time to compile and does not use unstable Rust features.
@alexcrichton
Copy link
Member

@bors: rollup- r-

one bug is that appveyor.yml doesn't haev ./x.py test

another is;

/cargo/git/checkouts/angle-017278ddf8240375/c598aaf/include/EGL/eglplatform.h:118:22: fatal error: X11/Xlib.h: No such file or directory

@bors
Copy link
Contributor

bors commented Sep 17, 2017

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

@SimonSapin
Copy link
Contributor Author

appveyor.yml doesn't have ./x.py test

I don’t understand what that means

EGL/eglplatform.h:118:22: fatal error: X11/Xlib.h: No such file or directory

I’ve added libgl1-mesa-dev, llvm-dev, and libedit-dev to the list of packages to install in x86_64-gnu-cargotest/Dockerfile, based on WebRender’s .travis.yml.

@alexcrichton, is there some "try" mode to make a full CI run without landing?

@alexcrichton
Copy link
Member

The appveyor.yml contains:

SCRIPT: python x.py src/tools/cargotest

there's a missing test there

is there some "try" mode to make a full CI run without landing?

Yes, you can execute this locally via:

./src/ci/docker/run.sh x86_64-gnu-cargotest

@SimonSapin
Copy link
Contributor Author

Fixed appveyor.toml.

As to running the docker script locally I got to the point where it looks like this:

…
Copying stage0 test from stage0 (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu / x86_64-unknown-linux-gnu)
Building LLVM for x86_64-unknown-linux-gnu
running: "cmake" "/checkout/src/llvm" "-DLLVM_ENABLE_ASSERTIONS=ON" "-DLLVM_TARGETS_TO_BUILD=X86;ARM;AArch64;Mips;PowerPC;SystemZ;JSBackend;MSP430;Sparc;NVPTX;Hexagon" "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=" "-DLLVM_INCLUDE_EXAMPLES=OFF" "-DLLVM_INCLUDE_TESTS=OFF" "-DLLVM_INCLUDE_DOCS=OFF" "-DLLVM_ENABLE_ZLIB=OFF" "-DWITH_POLLY=OFF" "-DLLVM_ENABLE_TERMINFO=OFF" "-DLLVM_ENABLE_LIBEDIT=OFF" "-DLLVM_PARALLEL_COMPILE_JOBS=8" "-DLLVM_TARGET_ARCH=x86_64" "-DLLVM_DEFAULT_TARGET_TRIPLE=x86_64-unknown-linux-gnu" "-DLLVM_LINK_LLVM_DYLIB=ON" "-DCMAKE_C_COMPILER=sccache" "-DCMAKE_C_COMPILER_ARG1=cc" "-DCMAKE_CXX_COMPILER=sccache" "-DCMAKE_CXX_COMPILER_ARG1=c++" "-DCMAKE_C_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_CXX_FLAGS=-ffunction-sections -fdata-sections -fPIC -m64" "-DCMAKE_INSTALL_PREFIX=/checkout/obj/build/x86_64-unknown-linux-gnu/llvm" "-DCMAKE_BUILD_TYPE=Release"

With no further output or error message, and run.sh exits without an error code.

@alexcrichton
Copy link
Member

What version of docker do you have locally? I believe to work we need the --init argument which was added relatively recently.

You can also test on Travis if you'd prefer by editing .travis.yml and editing the if: branch = auto condition on the relevant builder

@SimonSapin
Copy link
Contributor Author

I installed Docker from Ubuntu packages. It did complain about unknown flag: --init so I removed that flag and hoped for the best 😀. I’ll try with Travis.

@alexcrichton
Copy link
Member

[00:49:40] --- stderr

[00:49:40] configure: error: 

[00:49:40] *** expat is required. or try to use --enable-libxml2

[00:49:40] make: *** [/checkout/obj/build/ct/webrender/target/debug/build/servo-fontconfig-sys-41d9b2a793d1c36f/out/Makefile] Error 1

[00:49:40] thread 'main' panicked at 'assertion failed: Command::new("make").args(&["-R", "-f",

[00:49:40]                             "makefile.cargo"]).status().unwrap().success()', /cargo/registry/src/github.com-1ecc6299db9ec823/servo-fontconfig-sys-4.0.3/build.rs:17:4

[00:49:40] note: Run with `RUST_BACKTRACE=1` for a backtrace.

[00:49:40] 

[00:49:40] warning: build failed, waiting for other jobs to finish...

[00:50:03] error: build failed

[00:50:03] thread 'main' panicked at 'tests failed for https://github.com/servo/webrender', /checkout/src/tools/cargotest/main.rs:100:8

@alexcrichton
Copy link
Member

ping @SimonSapin, just want to make sure this doesn't fall off your radar

@SimonSapin
Copy link
Contributor Author

Err, Servo CI even only runs (and compiles) these tests on Linux.

SimonSapin added a commit to servo/servo that referenced this pull request Oct 24, 2017
bors-servo pushed a commit to servo/servo that referenced this pull request Oct 24, 2017
Make test-stylo compile an empty crate on Windows without Gecko

That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo: rust-lang/rust#44603

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19003)
<!-- Reviewable:end -->
@alexcrichton
Copy link
Member

@SimonSapin AFAIK there's no "easy" way to turn off these linker errors unfortunately :(

If you'd like though, the test can be disabled in cargotest on Windows?

@SimonSapin
Copy link
Contributor Author

I’m working on a Servo-side work-around so that we don’t need to pile up more per-test config in cargotest.

SimonSapin added a commit to servo/servo that referenced this pull request Oct 24, 2017
That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo:
rust-lang/rust#44603
SimonSapin added a commit to servo/servo that referenced this pull request Oct 24, 2017
That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo:
rust-lang/rust#44603
SimonSapin added a commit to servo/servo that referenced this pull request Oct 24, 2017
Exercise the previous commit’s fix.

This used to fail to link:
rust-lang/rust#44603 (comment)
bors-servo pushed a commit to servo/servo that referenced this pull request Oct 24, 2017
Make test-stylo compile an empty crate on Windows without Gecko

That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo: rust-lang/rust#44603

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19003)
<!-- Reviewable:end -->
@SimonSapin
Copy link
Contributor Author

Updated with servo/servo#19003.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 24, 2017
@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Oct 24, 2017

📌 Commit daf84db has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Oct 24, 2017

⌛ Testing commit daf84db with merge aa40292...

bors added a commit that referenced this pull request Oct 24, 2017
Add Stylo and WebRender to src/tools/cargotest

This is a subset of Servo that takes relatively less time to compile and does not use unstable Rust features.
@bors
Copy link
Contributor

bors commented Oct 25, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing aa40292 to master...

@bors bors merged commit daf84db into rust-lang:master Oct 25, 2017
@SimonSapin SimonSapin deleted the stylo branch October 25, 2017 06:16
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 25, 2017
…ws without Gecko (from servo:stylo_tests_windows); r=emilio

That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo: rust-lang/rust#44603

Source-Repo: https://github.com/servo/servo
Source-Revision: 38fe9533b93e985657f99a29772bf3d3c8694822

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 3a2413e03efb4754cf657f4637a3543fbc13074a
xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this pull request Oct 25, 2017
…ws without Gecko (from servo:stylo_tests_windows); r=emilio

That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo: rust-lang/rust#44603

Source-Repo: https://github.com/servo/servo
Source-Revision: 38fe9533b93e985657f99a29772bf3d3c8694822
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
…ws without Gecko (from servo:stylo_tests_windows); r=emilio

That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo: rust-lang/rust#44603

Source-Repo: https://github.com/servo/servo
Source-Revision: 38fe9533b93e985657f99a29772bf3d3c8694822

UltraBlame original commit: 6d6d75c9f64f65ecf8c71983a29f036d477c6ed6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…ws without Gecko (from servo:stylo_tests_windows); r=emilio

That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo: rust-lang/rust#44603

Source-Repo: https://github.com/servo/servo
Source-Revision: 38fe9533b93e985657f99a29772bf3d3c8694822

UltraBlame original commit: 6d6d75c9f64f65ecf8c71983a29f036d477c6ed6
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…ws without Gecko (from servo:stylo_tests_windows); r=emilio

That is, in cases where it would fail to link.

This will help make Rust CI be gated on compiling Stylo: rust-lang/rust#44603

Source-Repo: https://github.com/servo/servo
Source-Revision: 38fe9533b93e985657f99a29772bf3d3c8694822

UltraBlame original commit: 6d6d75c9f64f65ecf8c71983a29f036d477c6ed6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants