Skip to content

Remove unconditional -Wno-deprecated-declaration on macOS - #4931

Merged
ethomson merged 1 commit into
libgit2:masterfrom
tiennou:cmake/osx-deprecated-warn
Jan 10, 2019
Merged

Remove unconditional -Wno-deprecated-declaration on macOS#4931
ethomson merged 1 commit into
libgit2:masterfrom
tiennou:cmake/osx-deprecated-warn

Conversation

@tiennou

@tiennou tiennou commented Jan 10, 2019

Copy link
Copy Markdown
Contributor

Just noticed that while investigating #4924.

After taking into consideration the following, I think this should be removed :

  • OpenSSL isn't the default on Apple platforms
  • you have to jump through hoops to get CMake to use OpenSSL on macOS (headers aren't in /usr/include, so you have to provide -DOPENSSL_* overrides)
  • users are likely (as getting anywhere near the installed 0.9.8 version is insanity IMHO) to package a "modern" version, which wouldn't be marked as deprecated

There might be a way to detect which version is getting used, or maybe #pragma the deprecation warnings out of openssl.c instead.

@tiennou tiennou changed the title Remove unconditional -Wdeprecated-declaration on macOS Remove unconditional -Wno-deprecated-declaration on macOS Jan 10, 2019
After taking into consideration the following, I think this should be
removed :

- OpenSSL isn't the default on Apple platforms
- you have to jump through hoops to get CMake to use OpenSSL on macOS
(headers aren't in `/usr/include`, so you have to provide `-DOPENSSL_*`
overrides)
- users are likely (as getting anywhere near the installed 0.9.8 version
is insanity IMHO) to package a "modern" version, which wouldn't be
marked as deprecated
@tiennou
tiennou force-pushed the cmake/osx-deprecated-warn branch from 6c38226 to 2cc66dd Compare January 10, 2019 16:03
@ethomson

Copy link
Copy Markdown
Member

Appreciate the analysis - agree that we should remove 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.

2 participants