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

cargo-c: switch back to uses_from_macos "curl" #197727

Merged
merged 2 commits into from
Nov 17, 2024
Merged

Conversation

chenrui333
Copy link
Member

@chenrui333 chenrui333 commented Nov 14, 2024

  • Have you followed the guidelines for contributing?
  • Have you ensured that your commits follow the commit style guide?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it pass brew audit --new <formula>?

relates to:

@github-actions github-actions bot added the rust Rust use is a significant feature of the PR or issue label Nov 14, 2024
@chenrui333 chenrui333 added the CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. label Nov 14, 2024
@Bo98
Copy link
Member

Bo98 commented Nov 14, 2024

Looks like macOS 13 is using vendored curl:

==> brew linkage --cached cargo-c
System libraries:
  /System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation
  /System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices
  /System/Library/Frameworks/Security.framework/Versions/A/Security
  /System/Library/Frameworks/SystemConfiguration.framework/Versions/A/SystemConfiguration
  /usr/lib/libSystem.B.dylib
  /usr/lib/libiconv.2.dylib
  /usr/lib/libz.1.dylib
Homebrew libraries:
  /opt/homebrew/opt/libgit2/lib/libgit2.1.8.dylib (libgit2)
  /opt/homebrew/opt/libssh2/lib/libssh2.1.dylib (libssh2)
  /opt/homebrew/opt/openssl@3/lib/libcrypto.3.dylib (openssl@3)
  /opt/homebrew/opt/openssl@3/lib/libssl.3.dylib (openssl@3)

@chenrui333 chenrui333 marked this pull request as draft November 14, 2024 18:48
@chenrui333 chenrui333 marked this pull request as draft November 14, 2024 18:48
@chenrui333
Copy link
Member Author

yeah, only 13 does not link to sys curl

@chenrui333 chenrui333 added 13-arm64 Ventura arm64 is specifically affected 13 Ventura is specifically affected labels Nov 14, 2024
@chenrui333
Copy link
Member Author

chenrui333 commented Nov 14, 2024

I just run a sys curl, seems working fine for me

bash-3.2$ cat cargo.toml
[package]
name = "curl_sys_checker"
version = "0.1.0"
edition = "2021"

[dependencies]
curl = "0.4.41"
curl-sys ="0.4.68"

bash-3.2$ cat src/main.rs
use std::io::{stdout, Write};

use curl::easy::Easy;

// Print a web page onto stdout
fn main() {
    let mut easy = Easy::new();
    easy.url("https://www.rust-lang.org/").unwrap();
    easy.write_function(|data| {
        stdout().write_all(data).unwrap();
        Ok(data.len())
    }).unwrap();
    easy.perform().unwrap();

    println!("{}", easy.response_code().unwrap());
}

bash-3.2$ otool -L target/debug/curl_sys_checker
target/debug/curl_sys_checker:
        /usr/lib/libcurl.4.dylib (compatibility version 7.0.0, current version 9.0.0)
        /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)
        /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1336.61.1)
bash-3.2$ brew config
HOMEBREW_VERSION: 4.4.5-25-g2fb6bf0
ORIGIN: https://github.com/Homebrew/brew
HEAD: 2fb6bf05fbf0984fe7a6ab1476537d657a9fab01
Last commit: 27 hours ago
Core tap HEAD: eac641dd5153119d2d7a7b9d6cbcf60508cb326a
Core tap last commit: 42 minutes ago
Core tap JSON: 08 Nov 21:42 UTC
Core cask tap HEAD: ee215b335e4a454f4b07063e8e0ad3d9eb567fd9
Core cask tap last commit: 89 minutes ago
Core cask tap JSON: 08 Nov 21:42 UTC
...
Homebrew Ruby: 3.3.6 => /usr/local/Homebrew/Library/Homebrew/vendor/portable-ruby/3.3.6/bin/ruby
CPU: quad-core 64-bit ivybridge
Clang: 15.0.0 build 1500
Git: 2.47.0 => /usr/local/bin/git
Curl: 8.7.1 => /usr/bin/curl
macOS: 13.7.1-x86_64
CLT: 14.3.1.0.1.1683849156
Xcode: 15.2 => /Applications/Xcode_15.2.app/Contents/Developer

@Bo98
Copy link
Member

Bo98 commented Nov 14, 2024

It'll be things that use the http2 feature. curl-config --features did not report HTTP2 prior to macOS 14.

Technically more a bug in curl-config because HTTP2 did exist since 10.13. Maybe we can make a shim workaround in brew.

Or an alternative if you want a quicker solution is to use since: :sonoma for now.

@chenrui333
Copy link
Member Author

chenrui333 commented Nov 14, 2024

It'll be things that use the http2 feature. curl-config --features did not report HTTP2 prior to macOS 14.

Technically more a bug in curl-config because HTTP2 did exist since 10.13. Maybe we can make a shim workaround in brew.

yeah, I saw HTTP2 in curl --version, but did not run curl-config check


yeah, you are right, it is the curl-config thing

@chenrui333
Copy link
Member Author

Or an alternative if you want a quicker solution is to use since: :sonoma for now.

with that, now it is using the system curl, as brew curl reports http2 feature. let me know what you think on it @Bo98

@chenrui333 chenrui333 requested a review from Bo98 November 16, 2024 19:25
Signed-off-by: Rui Chen <[email protected]>

cargo-c: add `since: :sonoma`

Signed-off-by: Rui Chen <[email protected]>
Copy link
Contributor

🤖 An automated task has requested bottles to be published to this PR.

@github-actions github-actions bot added the CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. label Nov 17, 2024
@BrewTestBot BrewTestBot added this pull request to the merge queue Nov 17, 2024
Merged via the queue into master with commit 41c2049 Nov 17, 2024
15 checks passed
@BrewTestBot BrewTestBot deleted the cargo-c-sys-curl branch November 17, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
13-arm64 Ventura arm64 is specifically affected 13 Ventura is specifically affected CI-no-fail-fast Continue CI tests despite failing GitHub Actions matrix builds. CI-published-bottle-commits The commits for the built bottles have been pushed to the PR branch. rust Rust use is a significant feature of the PR or issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants