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

OpenSSL-sys build failure / Cargo MAKEFLAGS not configurable #13476

Closed
threeseed opened this issue Feb 21, 2024 · 15 comments
Closed

OpenSSL-sys build failure / Cargo MAKEFLAGS not configurable #13476

threeseed opened this issue Feb 21, 2024 · 15 comments
Labels
A-jobserver Area: jobserver, concurrency, parallelism C-bug Category: bug S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.

Comments

@threeseed
Copy link

threeseed commented Feb 21, 2024

Problem

So we have an issue building OpenSSL-sys on Mac M1: sfackler/rust-openssl#2179

Which is likely caused by this command:

running cd "/opt/actions-runner/_work/search/search/search/src-tauri/target/aarch64-apple-darwin/release/build/openssl-sys-8aca377df6729114/out/openssl-build/build/src" 
&& MAKEFLAGS="-j --jobserver-fds=7,13 --jobserver-auth=7,13" "make" "build_libs"

Namely that the "-j" flag for make is overloading the resources of the host.

And there doesn't seem to be a way to override this MAKEFLAGS to workaround it.

Steps

No response

Possible Solution(s)

No response

Notes

No response

Version

cargo 1.75.0 (1d8b05cdd 2023-11-20)
release: 1.75.0
commit-hash: 1d8b05cdd1287c64467306cf3ca2c8ac60c11eb0
commit-date: 2023-11-20
host: aarch64-apple-darwin
libgit2: 1.7.1 (sys:0.18.1 vendored)
libcurl: 8.4.0 (sys:0.4.68+curl-8.4.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Mac OS 14.2.1 [64-bit]
@threeseed threeseed added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Feb 21, 2024
@weihanglo
Copy link
Member

Could you provide steps to reproduce and also include the version or commit of that crate?

@threeseed
Copy link
Author

@weihanglo .. You can see the code and full build failure here under the Build Tauri step.

Looks to be an issue both with openssl-sys-v0.9.99 and openssl-sys-v0.9.100 versions.

@weihanglo
Copy link
Member

Is the issue about building openssl exhausting system resource?

For MAKEFLAGS="-j --jobserver-auth=..., it's jobserver crate that passes MAKEFLAGS="-j ...". By definition, -j without a value means "to run as many jobs as possible in parallel". Given that cargo controls the max parallelism, it won't spawn more than N jobs, where N is either # of core or -j <value>.

However, cc crate has a parallel feature that might go beyond the limit. I've identified the potential symptom from the issue you've linked:

If the issue is inherently about using too many system resources, then either those dependencies shouldn't enable the parallel feature, or cc crate needs to provide a way to opt-out the parallel feature, for example via <pkg-name>_NO_CC_PARALLEL=true.

(Note that features are additive and there is no cargo-native way to opt-out at this moment)

@weihanglo
Copy link
Member

weihanglo commented Feb 22, 2024

Do you mind running cargo tree -i cc -e features | grep parallel and check if your rust package has parallel feature enabled on cc crate?

@weihanglo weihanglo added A-jobserver Area: jobserver, concurrency, parallelism S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request. and removed S-triage Status: This issue is waiting on initial triage. labels Feb 22, 2024
@threeseed
Copy link
Author

threeseed commented Feb 22, 2024

@weihanglo ..

MacBook-Pro src-tauri % cargo tree -i cc -e features | grep parallel
│   └── cc feature "parallel"
└── cc feature "parallel" (*)
├── cc feature "jobserver"
│   └── cc feature "parallel"
│       [build-dependencies]
│       └── zstd-sys v2.0.9+zstd.1.5.5 (*)
└── cc feature "parallel" (*)

@weihanglo
Copy link
Member

Great! Thanks for the reply.

Now, to identify if the problem is really a resource exhaustion, you can patch the cc crate, making parallel feature a no-op, and see if the error occurs in your CI.

@threeseed
Copy link
Author

@weihanglo .. So that fix works and have created a PR for zstd to disable CC parallelism.

Thanks for all your help !

@NobodyXu
Copy link

NobodyXu commented Feb 22, 2024

Which version of cc are you using @nadenf ?

I'd like to identify the cause instead of error.

Latest release 1.0.86 (compared to 1.0.79) removes thread spawning in cc when parallel is enabled, it should prevent resource exhaustion and improve performance, if that's the cause of the error.

@threeseed
Copy link
Author

@NobodyXu .. ZStd is using 1.0.45. I will try upgrading to 1.0.86 and leaving parallel enabled.

n-eq added a commit to EliseChouleur/zstd-rs that referenced this issue Feb 22, 2024
@threeseed
Copy link
Author

threeseed commented Feb 22, 2024

I have tried with 1.086 and parallel enabled and it doesn't fix the problem. Only removing parallel entirely works.

@NobodyXu
Copy link

Hmmm I think I know the reason.

Since cc >=1.0.80, we set the jobserver inherited to O_NONBLOCK, make probably isn't designed for this, which is why it failed.

I will open a new ticket in cc and fix it.

@NobodyXu
Copy link

@nadenf Can you try make >= 4.4.1 please?

I think make might have introduced support for non-vlocking jobserver pipe in newer version.

@threeseed
Copy link
Author

@NobodyXu .. Would add that the default on a Mac is 3.81 so you're asking developers to override that with the Homebrew version.

That said it doesn't work for me.

@NobodyXu
Copy link

NobodyXu commented Feb 22, 2024

Thanks, my proposed solution is to change rust-lang/jobserver to use a named pipe instead of an annoymous one.

The named pipe is passed down by its path in filesystem and opening it using O_NONBLOCK doesn't affect others.

We can also retain backwards compatibility by keeping the jobserver fds inheritable and keeping them in the environment variable.

E.g. MAKEFLAGS='--jobserver-auth=fifo:/path/to/fifo --jobserver-fds=3,5'

@NobodyXu
Copy link

@nadenf I tried it in my own project that updates to make 4.4.1 does fix the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-jobserver Area: jobserver, concurrency, parallelism C-bug Category: bug S-needs-info Status: Needs more info, such as a reproduction or more background for a feature request.
Projects
None yet
Development

No branches or pull requests

3 participants