Support crater runs on Windows #399

ecstatic-morse · 2019-02-18T11:34:09Z

Resolves #149.

This allows builds to run on the newly added docker image. In order to run successfully, you'll need to correctly configure the Docker host ~~and crater's work directory should have an ACL to enable full-access by the "Authenticated Users" group. This allows the container to write to the newly created target directories.~~

The changes in the first commit don't belong in the finished product: Windows' permissions are fundamentally different than the POSIX ones so I need to think more about how to change the current API to be more cross-platform. However, .exes are executable by default on Windows and user remapping isn't done within the container anyways, so my changes were enough to this PR up and running.

I couldn't get OpenSSL to link properly when building crater on Windows, despite setting OPENSSL_DIR, so I've temporarily reverted "switch from ring to openssl" to let me run tests. This should be un-reverted before this is merged.

~~The last commit points to a self-published version of the docker image, not a rust-ops published one. An official version can be published once I get CI up and running for the Windows image.~~

The concerns listed above have all been resolved. The ACL change seems to have been obsoleted by later versions of Docker on Windows.

pietroalbini · 2019-02-18T11:56:15Z

Thanks, this looks great!

I couldn't get OpenSSL to link properly when building crater on Windows, despite setting OPENSSL_DIR, so I've temporarily reverted "switch from ring to openssl" to let me run tests. This should be un-reverted before this is merged.

ring/OpenSSL are only used for HMAC at the moment, you can switch to the hmac and sha-1 crates (maybe in a separate, smaller PR?).

bors · 2019-02-18T16:37:29Z

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

ecstatic-morse · 2019-02-20T10:29:18Z

Unit tests are now running on AppVeyor, I had to change the heartbeat unit test to allow for successive timestamps to be equal.

The check_config::good integration test fails on master while running on my local (Linux) machine with the same error message as on AppVeyor. Is there some additional configuration I need to do?

pietroalbini · 2019-02-20T10:33:25Z

The check_config::good integration test fails on master while running on my local (Linux) machine as well with the same error message as on AppVeyor. Is there some additional configuration I need to do?

You need to run cargo run -- create-lists before running the tests, as we do on Linux.

ecstatic-morse · 2019-07-15T16:54:57Z

@pietroalbini

There's been some interest in getting this working, so I'm going to pick it up again.

One issue that arose was the fact that docker doesn't set the OOMKilled flag on windows, so we can't reliably check for memory exhaustion. I will open a feature request on moby, but I don't know how complex this would be to implement, since it depends on Windows internals.

Is this a blocker for doing proper crater runs? It means you'll have more spurious failures, but it can be mitigated via the blacklist

pietroalbini · 2019-07-15T17:03:41Z

Is this a blocker for doing proper crater runs? It means you'll have more spurious failures, but it can be mitigated via the blacklist

Nah, of course it would be nice to get it working but it's not a blocker.

.gitattributes

ecstatic-morse · 2019-07-19T08:16:38Z

@pietroalbini

Some of this is kinda hacky, but I think it's ready for review. All unit tests and minicrater tests now pass on the azure instance I'm using for testing (once I adjust the hard-coded docker image to one which is compatible with Windows 1803). I'm curious if you have a more elegant way to separate out the memory-hungry test.

I still need to integrate my Windows-specific provisioning instructions into the crater docs. That still depends somewhat on what version of Windows the rustops published docker image is based on.

pietroalbini

All of this mostly looks good! Huge thanks for working on it! Left a few comments.

src/utils/fs.rs

src/run.rs

Add steps for provisioning a Windows box to run `crater` This adds detailed set-up instructions for running `crater` on a Windows box. Obviously this depends on #399. I also tried to explain some of the complexity around Docker image compatibility.

ecstatic-morse · 2019-07-24T17:37:40Z

@pietroalbini should be ready for a final review.

pietroalbini · 2019-07-24T17:46:16Z

Travis failed

Also ensure that we get platform-specific line endings when checking out the expected logs.

Successive calls to `UTC::now` are not guaranteed to be monotonically increasing (indeed, they're not even guaranteed to be monotonically non-decreasing thanks to leap seconds and daylight saving time). However, the heartbeat unit test expects two timestamps constructed on either side of a database insert to not be equal. This test fails occasionally on AppVeyor. Perhaps they have an especially coarse clock (Spectre mitigation)? This test can still fail during a leap second or time zone adjustment, but such failures will be much rarer.

Due to rust-lang/rustup#1540, installing the docs on Windows can be slow enough to time out a fresh rustc install. Marking this `Command` as `quiet` increases the timeout.

Previously, `Crate::id` was used to define the location of the source directory of a crate on the filesystem as well as their identifier in generated reports. This meant paths containing "/" were passed as arguments to `docker --mount` on Windows.

This test will always on Windows because its docker engine does not yet support the `State.OOMKilled` flag. We move it out of `full` and into its own minicrater run (which is disabled on Windows).

This is obselete now that we normalize newlines in `*.expected.json` files.

This also adds a section in the provisioning docs about the "Enable Win32 long paths" option. Precise instructions are behind a Stack Overflow link, because this option is unstable and may change.

ecstatic-morse · 2019-07-25T17:18:44Z

@pietroalbini. Rebased with s/rust-lang-nursery/rust-lang/g. Should be good to go.

pietroalbini · 2019-07-25T17:19:13Z

@bors r+

Awesome!

bors · 2019-07-25T17:19:14Z

📌 Commit cdae40c has been approved by pietroalbini

bors · 2019-07-25T17:19:22Z

⌛ Testing commit cdae40c with merge f44bc7c...

Support crater runs on Windows Resolves #149. This allows builds to run on [the newly added docker image](rust-lang/crates-build-env#5). In order to run successfully, you'll need to correctly [configure the Docker host](https://github.com/rust-lang/crater/blob/master/docs/agent-machine-setup-windows.md) ~~and `crater`'s `work` directory should have an ACL to enable full-access by the `"Authenticated Users"` group. This allows the container to write to the newly created `target` directories.~~ ~~The changes in the first commit don't belong in the finished product: Windows' permissions are [fundamentally different than the POSIX ones](https://en.wikipedia.org/wiki/Access_control_list) so I need to think more about how to change the current API to be more cross-platform. However, `.exe`s are executable by default on Windows and user remapping isn't done within the container anyways, so my changes were enough to this PR up and running.~~ ~~I couldn't get OpenSSL to link properly when building crater on Windows, despite setting `OPENSSL_DIR`, so I've temporarily reverted "switch from ring to openssl" to let me run tests. This should be un-reverted before this is merged.~~ ~~The last commit points to a self-published version of the docker image, not a `rust-ops` published one. An official version can be published once I get [CI up and running](rust-lang/crates-build-env#5 (comment)) for the Windows image.~~ The concerns listed above have all been resolved. The ACL change seems to have been obsoleted by later versions of Docker on Windows.

bors · 2019-07-25T17:43:21Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: pietroalbini
Pushing f44bc7c to master...

mati865 · 2019-07-25T21:06:59Z

I think bors failed to push it.

pietroalbini · 2019-07-25T21:11:01Z

What is going on with it...

ecstatic-morse mentioned this pull request Feb 18, 2019

Windows support #149

Open

13 tasks

ecstatic-morse force-pushed the windows-support branch 2 times, most recently from ab6f8f5 to 07fd80d Compare February 19, 2019 16:44

ecstatic-morse force-pushed the windows-support branch 2 times, most recently from 07b8926 to 942515e Compare February 20, 2019 21:24

mati865 reviewed Jul 17, 2019

View reviewed changes

.gitattributes Outdated Show resolved Hide resolved

ecstatic-morse force-pushed the windows-support branch 5 times, most recently from 3e8dab2 to 4f4bf57 Compare July 19, 2019 08:04

ecstatic-morse marked this pull request as ready for review July 19, 2019 08:07

ecstatic-morse force-pushed the windows-support branch from 4f4bf57 to 70d22ae Compare July 19, 2019 08:09

ecstatic-morse force-pushed the windows-support branch from 70d22ae to 61fdedf Compare July 19, 2019 08:20

pietroalbini reviewed Jul 19, 2019

View reviewed changes

src/utils/fs.rs Show resolved Hide resolved

src/run.rs Outdated Show resolved Hide resolved

ecstatic-morse force-pushed the windows-support branch 2 times, most recently from df0fd33 to 78cfcbb Compare July 19, 2019 18:11

ecstatic-morse mentioned this pull request Jul 21, 2019

Add steps for provisioning a Windows box to run crater #423

Merged

ecstatic-morse force-pushed the windows-support branch 2 times, most recently from 5243fb1 to 9a5aa01 Compare July 24, 2019 17:36

ecstatic-morse and others added 18 commits July 25, 2019 09:57

Normalize path names in report

702aaaa

Also ensure that we get platform-specific line endings when checking out the expected logs.

Use a custom Docker image on Windows

5e09c9d

Escape reserved characters on Windows from log path

93d5fbc

Run unit tests on AppVeyor

77f9ceb

Run create-lists before tests

ef555a4

Ensure work/local exists during create-lists

a32708d

Allow more time for rustup-init install

8862925

Due to rust-lang/rustup#1540, installing the docs on Windows can be slow enough to time out a fresh rustc install. Marking this `Command` as `quiet` increases the timeout.

Add Crate::to_path

b8219f2

Previously, `Crate::id` was used to define the location of the source directory of a crate on the filesystem as well as their identifier in generated reports. This meant paths containing "/" were passed as arguments to `docker --mount` on Windows.

Call Toolchain::to_path_component when building paths

6968dcc

Ensure process isolation is used on Windows

e1c3c8c

Allow drive space check to fail without marking task as failed

1330a18

Move memory exhaustion out of full

ed18e8f

This test will always on Windows because its docker engine does not yet support the `State.OOMKilled` flag. We move it out of `full` and into its own minicrater run (which is disabled on Windows).

Add test for strip_verbatim_from_prefix

4f08179

Remove excessive to_str calls for paths passed to mount

8523ba3

Normalize line endings when comparing minicrater output

9c612b8

Remove section on minicrater line endings from Windows docs

be930f9

This is obselete now that we normalize newlines in `*.expected.json` files.

Warn when paths are too long for Windows

cdae40c

This also adds a section in the provisioning docs about the "Enable Win32 long paths" option. Precise instructions are behind a Stack Overflow link, because this option is unstable and may change.

ecstatic-morse force-pushed the windows-support branch from eb182fa to cdae40c Compare July 25, 2019 17:00

ecstatic-morse mentioned this pull request Jul 25, 2019

Spurious failures in docker daemon on Windows #426

Open

pietroalbini merged commit 7f53140 into rust-lang:master Jul 25, 2019

pietroalbini mentioned this pull request Jul 28, 2019

Add experiment requirements and agent capabilities #430

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support crater runs on Windows #399

Support crater runs on Windows #399

ecstatic-morse commented Feb 18, 2019 •

edited

Loading

pietroalbini commented Feb 18, 2019

bors commented Feb 18, 2019

ecstatic-morse commented Feb 20, 2019 •

edited

Loading

pietroalbini commented Feb 20, 2019

ecstatic-morse commented Jul 15, 2019

pietroalbini commented Jul 15, 2019

ecstatic-morse commented Jul 19, 2019

pietroalbini left a comment

ecstatic-morse commented Jul 24, 2019

pietroalbini commented Jul 24, 2019

ecstatic-morse commented Jul 25, 2019

pietroalbini commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

mati865 commented Jul 25, 2019 •

edited

Loading

pietroalbini commented Jul 25, 2019

Support crater runs on Windows #399

Support crater runs on Windows #399

Conversation

ecstatic-morse commented Feb 18, 2019 • edited Loading

pietroalbini commented Feb 18, 2019

bors commented Feb 18, 2019

ecstatic-morse commented Feb 20, 2019 • edited Loading

pietroalbini commented Feb 20, 2019

ecstatic-morse commented Jul 15, 2019

pietroalbini commented Jul 15, 2019

ecstatic-morse commented Jul 19, 2019

pietroalbini left a comment

Choose a reason for hiding this comment

ecstatic-morse commented Jul 24, 2019

pietroalbini commented Jul 24, 2019

ecstatic-morse commented Jul 25, 2019

pietroalbini commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

bors commented Jul 25, 2019

mati865 commented Jul 25, 2019 • edited Loading

pietroalbini commented Jul 25, 2019

ecstatic-morse commented Feb 18, 2019 •

edited

Loading

ecstatic-morse commented Feb 20, 2019 •

edited

Loading

mati865 commented Jul 25, 2019 •

edited

Loading