-
Notifications
You must be signed in to change notification settings - Fork 422
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 curl 8.4.0 #1666
Add curl 8.4.0 #1666
Conversation
Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (curl) have been updated in this PR. Please review the changes. |
8b7aef3
to
edd75e5
Compare
it looks like the linux build for this doesn't support SSL (https). Is that right? Would a future patch supporting SSL through boringssl be possible? |
Interesting, considering how this is being used in opentelemetry I'm surprised by that. Does that require boringssl? But regardless I think we should enable things like that through starlark flags for flipping that behavior on / off and just try to pick the most reasonable defaults as best we can |
No, but its the only ssl implementation in the bcr. The curl build here https://github.com/googleapis/google-cloud-cpp/blob/main/bazel/curl.BUILD uses boringssl.
Honestly I'm not sure what the best approach is. I've done config settings here in the past, but I don't know if it was the right call. I think it would be nice if SSL worked without configuration as a default. It would be nice if we could declare features at the module level like this: bazel_dep(name = "curl", version = "8.4.0", features = ["openssl"]) but I don't think something like that exists :/ |
This is something that is coming up in more and more contexts. I think that it could be built in Starlark with no changes to Bazel. I'm planning to look into this soon. |
This is based on https://github.com/open-telemetry/opentelemetry-cpp/blob/47db4d0c0adfe2572d8b072518ae51fd5ec43961/bazel/curl.BUILD