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

project requires c99 #42

Closed

Conversation

autoantwort
Copy link

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build was run locally and without warnings or errors
  • All previous and new tests pass

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming, typo fix)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

If the compiler uses a very old c standard as default, it will fail to build

Related Issue URL: microsoft/vcpkg#21680 (comment)

What is the new behavior?

Set the required c standard to C99

Does this introduce a breaking change?

  • Yes
  • No

@JiaT75
Copy link
Contributor

JiaT75 commented Feb 27, 2023

Hi! Thanks for the bug report and PR.

All of the targets that need to be compiled require C99, so it would be better to just set the CMAKE_C_STANDARD variable at the top of CMakeLists.txt like this:

set(CMAKE_C_STANDARD 99)

@autoantwort
Copy link
Author

Do the headers also require C99 or only the source code?

@JiaT75
Copy link
Contributor

JiaT75 commented Feb 27, 2023

The liblzma API headers should be compatible with C89, but the internal headers used by the source code are not.

Larhzu added a commit that referenced this pull request Feb 27, 2023
Thanks to autoantwort for reporting the issue and suggesting
a different patch:
#42
@JiaT75
Copy link
Contributor

JiaT75 commented Feb 27, 2023

This should be fixed as of commit 4b7fb3b on master. @autoantwort can you let us know if this does not solve the bug?

This will be in a new stable 5.4.2 release in the near future. Thanks again for reporting this!

@autoantwort autoantwort deleted the feature/require-c99 branch February 28, 2023 07:50
Larhzu added a commit that referenced this pull request Mar 11, 2023
Thanks to autoantwort for reporting the issue and suggesting
a different patch:
#42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.4.2 Item earmarked for 5.4.2 release bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants