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

Remove direct dependency on curl #651

Merged
merged 2 commits into from
Jan 18, 2022
Merged

Remove direct dependency on curl #651

merged 2 commits into from
Jan 18, 2022

Conversation

adlerjohn
Copy link
Contributor

@adlerjohn adlerjohn commented Jan 15, 2022

Fixes #567

  1. Use the rustls-tls feature for reqwest and turn off default features (which would use native OpenSSL).
  2. Use ureq instead of curl.

@adlerjohn adlerjohn added compiler General compiler. Should eventually become more specific as the issue is triaged dependencies labels Jan 15, 2022
@adlerjohn adlerjohn self-assigned this Jan 15, 2022
@adlerjohn adlerjohn added forc and removed compiler General compiler. Should eventually become more specific as the issue is triaged labels Jan 15, 2022
@adlerjohn
Copy link
Contributor Author

adlerjohn commented Jan 15, 2022

openssl-sys is still required by fuel-gql-client, but is no longer required directly by forc:

$ cargo tree --invert openssl-sys
openssl-sys v0.9.72
├── curl v0.4.42
│   └── isahc v0.9.14
│       └── http-client v6.5.1
│           └── surf v2.3.2
│               ├── cynic v0.14.1
│               │   └── fuel-gql-client v0.1.2
│               │       └── forc v0.2.1 (/home/devel/local/fuellabs/sway/forc)
│               │           └── test v0.2.1 (/home/devel/local/fuellabs/sway/test)
│               └── fuel-gql-client v0.1.2 (*)
└── curl-sys v0.4.52+curl-7.81.0
    ├── curl v0.4.42 (*)
    └── isahc v0.9.14 (*)

Filed follow-up issue: FuelLabs/fuel-core#133

@adlerjohn adlerjohn marked this pull request as ready for review January 15, 2022 17:26
@adlerjohn adlerjohn requested a review from sezna as a code owner January 15, 2022 17:26
@adlerjohn adlerjohn changed the title Remove dependency on curl Remove direct dependency on curl Jan 15, 2022
@leviathanbeak
Copy link
Contributor

are we ok with having two http clients? one blocking (ureq) and the other non-blocking (reqwest)

@adlerjohn
Copy link
Contributor Author

are we ok with having two http clients? one blocking (ureq) and the other non-blocking (reqwest)

Prior to this PR, curl was used. So the PR doesn't increase the number of clients compared to the status quo. Longer-terms, it's probably a good idea to use non-blocking reqwest for everything. Would involve a larger refactor of these functions.

Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@adlerjohn
Copy link
Contributor Author

Will wait for @sezna

@sezna sezna merged commit 2e51c1a into master Jan 18, 2022
@sezna sezna deleted the adlerjohn/remove_curl branch January 18, 2022 14:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Sway depends on openssl
4 participants