Skip to content

Conversation

@davidjb
Copy link
Contributor

@davidjb davidjb commented Nov 8, 2018

As per #138 (and specifically #138 (comment)) the issue is that:

  • nginx compiles with -Werror (treating warnings as errors)
  • CentOS 6 has OpenSSL 1.0.1e, thus the LDAP module has a warning on compile
  • CentOS 6 has GCC 4.4. The version of GCC treats the LDAP module's statement #pragma GCC diagnostic warning "-Wcpp" as an error itself and thus skips the statement, leading to the #warning still being an error.

This adds version-checking to the warning options and the warning
itself. This means that no warning is issued at compile time on this
type of platform, but warnings are still emitted when starting Nginx
without certificate verification, so the issue is still visible in a notable way.

Other solutions to making this module compile on CentOS 6 / GCC 4.4 are welcomed, but this seems like the lightest touch.

As per kvspb#138 (comment)
the issue is that:

* nginx compiles with `-Werror` (treating warnings as errors)
* CentOS 6 has OpenSSL 1.0.1e and so the LDAP module has a warning on compile
* CentOS 6 has GCC 4.4.  GCC treats `#pragma GCC diagnostic warning "-Wcpp"` as itself an error and thus skips the statement, leading the the `#warning` still being an error.

This adds version-checking to the warning options and the warning
itself.  This means that no warning is issued at compile time on this
type of platform, but warnings are still emitted when starting Nginx
without certificate verification, so it isn't as though the issue has
been hidden.
@dasJ
Copy link

dasJ commented Jan 2, 2020

As an alternative to merging this, wait until November so CentOS 6 goes EOL 😄

@kvspb kvspb merged commit f022103 into kvspb:master Mar 20, 2020
@davama
Copy link

davama commented Mar 20, 2020

Wow!!!
Glad to see this cool project is still being supported!
Thank you very much! 👍

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.

4 participants