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

Explicitly include ctype.h to fix compilation warning #2314

Merged
merged 2 commits into from Aug 31, 2020

Conversation

venables
Copy link

Description

This simply adds #include <ctype.h> to puma_http11.c

On the 4.x branch, we get the following warning during compilation:

warning: implicitly declaring library function 'isspace' with type 'int (int)'

On MacOS Big Sur, this results in a build failure with:

include the header <ctype.h> or explicitly provide a declaration for 'isspace' 
1 error generated.

This change has already been applied to 5.0.0.beta / master as part of befe00a

Closes #2304

This PR is going to the 4.3.5 branch -- unsure if that's where you'd like it merged.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] the pull request title.
  • I have added appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@dentarg
Copy link
Member

dentarg commented Jul 20, 2020

IMHO worth mentioning in the changelog if we get a 4.x release with this fix :)

@venables venables changed the title [changelog skip] Explicitly include ctype.h to fix compilation warning Explicitly include ctype.h to fix compilation warning Jul 21, 2020
@venables
Copy link
Author

@dentarg great call -- updated.

@nateberkopec nateberkopec added bug waiting-for-review Waiting on review from anyone labels Jul 31, 2020
@MSP-Greg MSP-Greg mentioned this pull request Aug 5, 2020
History.md Outdated Show resolved Hide resolved
Co-authored-by: Patrik Ragnarsson <patrik@starkast.net>
@shepmaster
Copy link

On MacOS Big Sur, this results in a build failure with:

I believe that this is tied to the compiler version, not necessarily to the OS version. For example, I'm still running Catalina (10.15.5), but have the Xcode 12 beta and get the same error. Once Xcode 12 is released, I expect many people using macOS will upgrade to it and experience this problem.

@yahonda
Copy link

yahonda commented Aug 11, 2020

There is a similar issue reported macournoyer/thin#364

It says this is due to Xcode 12 change described here.
https://developer.apple.com/documentation/xcode-release-notes/xcode-12-beta-release-notes#Updates-in-Xcode-12-Beta

Clang now reports an error when you use a function without an explicit declaration when building C or Objective-C code for macOS (-Werror=implicit-function-declaration flag is on). This additional error detection unifies Clang’s behavior for iOS/tvOS and macOS 64-bit targets for this diagnostic. (49917738)

A simple workaround is here.

$ bundle config build.puma --with-cflags="-Wno-error=implicit-function-declaration"

@dentarg
Copy link
Member

dentarg commented Aug 11, 2020

@nateberkopec WDYT about a 4.3.6 release with only this change?

@@ -1,7 +1,7 @@
## Master

* Bugfixes
* Explicitly include ctype.h to fix compilation warning and build error on MacOS Big Sur (#2304)
* Explicitly include ctype.h to fix compilation warning and build error on macOS with Xcode 12 (#2304)

Choose a reason for hiding this comment

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

👍

@nateberkopec
Copy link
Member

4.3.5 and a 5.0.0.beta2 will be out Soon (tm)

@nateberkopec nateberkopec merged commit 8d1ccc6 into puma:4.3.5 Aug 31, 2020
@ilikepi
Copy link

ilikepi commented Sep 1, 2020

4.3.5 and a 5.0.0.beta2 will be out Soon (tm)

Apologies for commenting on a closed PR...

I noticed this change was merged into a branch called 4.3.5, however it looks like version 4.3.5 of the gem has been available on rubygems.org since May 19, 2020. I'm not familiar with the release practices of this project, so I very well might be missing something. I just wanted to mention it though in case this was...unexpected.

@nateberkopec
Copy link
Member

Sorry, I meant 4.3.6!

nateberkopec added a commit that referenced this pull request Sep 5, 2020
Explicitly include ctype.h to fix compilation warning
heynan0 added a commit to heynan0/view_components that referenced this pull request Oct 6, 2020
lawrence-forooghian added a commit to dxw/team-dashboard that referenced this pull request Mar 2, 2021
This fixes a compilation error - see
puma/puma#2314
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug important! waiting-for-review Waiting on review from anyone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants