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

Fix build with curl 7.62.0 #296

Merged
merged 1 commit into from
Apr 7, 2019
Merged

Fix build with curl 7.62.0 #296

merged 1 commit into from
Apr 7, 2019

Conversation

sunpoet
Copy link
Contributor

@sunpoet sunpoet commented Nov 3, 2018

from CHANGES:
ssl: deprecate CURLE_SSL_CACERT in favour of a unified error code
Long live CURLE_PEER_FAILED_VERIFICATION

from CHANGES:
ssl: deprecate CURLE_SSL_CACERT in favour of a unified error code
Long live CURLE_PEER_FAILED_VERIFICATION
@RPGillespie6
Copy link

Need this PR to resolve GCC error:

/build_env/gcc9_build/ext/cpr/libcpr-prefix/src/libcpr/cpr/error.cpp:41:9: error: duplicate case value
   41 |         case CURLE_SSL_CACERT:
      |         ^~~~
/build_env/gcc9_build/ext/cpr/libcpr-prefix/src/libcpr/cpr/error.cpp:25:9: note: previously used here
   25 |         case CURLE_PEER_FAILED_VERIFICATION:
      |         ^~~~

@TheAssassin
Copy link
Contributor

I've implemented a similar workaround:

diff --git a/cpr/error.cpp b/cpr/error.cpp
index 713cb10..e37f8d9 100644
--- a/cpr/error.cpp
+++ b/cpr/error.cpp
@@ -38,8 +38,10 @@ ErrorCode Error::getErrorCodeForCurlError(std::int32_t curl_code) {
             return ErrorCode::SSL_LOCAL_CERTIFICATE_ERROR;
         case CURLE_SSL_CIPHER:
             return ErrorCode::GENERIC_SSL_ERROR;
+#if CURLE_PEER_FAILED_VERIFICATION != CURLE_SSL_CACERT
         case CURLE_SSL_CACERT:
             return ErrorCode::SSL_CACERT_ERROR;
+#endif
         case CURLE_USE_SSL_FAILED:
             return ErrorCode::GENERIC_SSL_ERROR;
         case CURLE_SSL_ENGINE_INITFAILED:

Of course, this proposal is more elegant. I wish I had seen it earlier.

This project seems really dead. I wish someone would fork and take over maintenance.

@SleepingSoul
Copy link

Yeah. the same issue. Please, merge it :)

@Unarelith
Copy link

@whoshuu any reason why this is not merged yet?

@TheAssassin
Copy link
Contributor

There hasn't been any activity within the last 1.5 years. Maybe it's time to fork this project as a community project. If @whoshuu should ever return/respond, the projects could be merged again.

@whoshuu
Copy link
Contributor

whoshuu commented Apr 7, 2019

This is great. Thanks @sunpoet!

@TheAssassin as it happens, my career has taken me off the road of C++ enlightenment. In the next couple of months that may change for the better, and in the meantime I'll try to give the project some of my attention.

The project has more legs than I initially thought was possible. In light of that, and to protect against the single bus factor that is me, I think the healthiest thing to do is to have 2-3 maintainers on the project that can help guide it in the right direction.

Though making a call on who to have as a maintainer is hard (and something I've never done before), it is better than the current state of affairs.

@whoshuu whoshuu merged commit feebd2f into libcpr:master Apr 7, 2019
@sunpoet
Copy link
Contributor Author

sunpoet commented Apr 7, 2019

Thank you, @whoshuu.

@TheAssassin
Copy link
Contributor

@whoshuu good to see you're open to the idea of sharing responsibility. Let me know if you need help finding a strategy that works for you and for the project. I'm pretty sure there's people interested in this.

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.

6 participants