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

Make sure openssl compiles with only one core #41563

Merged

Conversation

aidanhs
Copy link
Member

@aidanhs aidanhs commented Apr 26, 2017

This is (hopefully) a fix for the osx openssl spurious failure - #40417.

The intermittent failures and failing in different ways made me think of a race condition. But programs are parallel make safe right? Not openssl. But we don't do a parallel make on openssl do we? This confused me, except "Waiting for unfinished jobs" is present in the logs...which is evidence of a parallel make!

It turns out that when we invoke to top level target in run.sh, make will pass the flags downwards in order to take advantage of parallelism in sub-makes. Of course, we don't want this in openssl! Override this by explicitly disabling parallelism on the command line.

I don't know why this hasn't happened on anything except OSX. Maybe Linux binutils check if the file is in use?

r? @alexcrichton

@alexcrichton
Copy link
Member

@bors: r+ p=10

Holy cow nice find! That'd do it...

@bors
Copy link
Contributor

bors commented Apr 26, 2017

📌 Commit 367e907 has been approved by alexcrichton

@bors
Copy link
Contributor

bors commented Apr 26, 2017

⌛ Testing commit 367e907 with merge 612847b...

bors added a commit that referenced this pull request Apr 26, 2017
… r=alexcrichton

Make sure openssl compiles with only one core

This is (hopefully) a fix for the osx openssl spurious failure - #40417.

The intermittent failures and failing in different ways made me think of a race condition. But programs are parallel make safe right? [Not openssl](openssl/openssl#298). But we don't do a parallel make on openssl [do we](https://github.com/rust-lang/rust/blob/8c4f2c64c6759a82f143e23964a46a65c67509c9/src/bootstrap/native.rs#L309)? This confused me, except "Waiting for unfinished jobs" is present in the logs...which is evidence of a parallel make!

It turns out that when we invoke to top level target [in run.sh](https://github.com/rust-lang/rust/blob/036983201d4e9aeb5c5e56e47c305971972b2569/src/ci/run.sh#L75-L77), make will [pass the flags downwards](https://www.gnu.org/software/make/manual/html_node/Options_002fRecursion.html) in order to take advantage of parallelism in sub-makes. Of course, we don't want this in openssl! Override this by explicitly disabling parallelism on the command line.

I don't know why this hasn't happened on anything except OSX. Maybe Linux binutils check if the file is in use?

r? @alexcrichton
@denji
Copy link

denji commented Apr 26, 2017

libressl is not reproduced?

@aidanhs
Copy link
Member Author

aidanhs commented Apr 26, 2017

@denji sorry, I'm not sure what you're asking - can you elaborate?

@denji
Copy link

denji commented Apr 26, 2017

@aidanhs my question: build libressl instead of openssl can solve the problem of parallel make?

@aidanhs
Copy link
Member Author

aidanhs commented Apr 26, 2017

@denji ah I see. To be honest, I don't know. I can't see anything in the git log for libressl that suggests they've done anything about it, so my instinctive answer is "no".

That said, openssl have put some effort in (here and here), so potentially upgrading to 1.1.0 might also fix our issue.

If we verify that this PR fixes the failure over a period of a few weeks, then we could consider an experiment to upgrade to openssl 1.1.0 with an eye kept out for future failures. For now this PR is probably the fix with the lowest risk (assuming it works!).

@bors
Copy link
Contributor

bors commented Apr 27, 2017

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

@bors bors merged commit 367e907 into rust-lang:master Apr 27, 2017
@aidanhs aidanhs deleted the aphs-fix-spurious-osx-openssl-failure branch April 27, 2017 00:09
@kennytm kennytm mentioned this pull request May 24, 2017
bors added a commit that referenced this pull request May 24, 2017
Beta backports

Backports of:

- #41913 (or some of it)
- #41937
- #41716
- #41563
aidanhs added a commit to aidanhs/rust-openssl that referenced this pull request Sep 17, 2017
aidanhs added a commit to aidanhs/rust-openssl that referenced this pull request Sep 17, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants