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

cmake/FindIconv.cmake: fix return type in test code #233

Merged
merged 1 commit into from Jul 30, 2021

Conversation

arjendekorte
Copy link
Contributor

In openSUSE Tumbleweed, this test always fails because it compiles with -Werror=return-type by default. Fixing this by adding a return value in the test script to keep the compiler happy.

In openSUSE Tumbleweed, this test always fails because it compiles with `-Werror=return-type` by default. Fixing this by adding a return value in the test script to keep the compiler happy.
@micahsnyder
Copy link
Contributor

Wow @arjendekorte I was going to work on this but you beat me to it! This is awesome. I'll give test before/after now. I'm also going to add opensuse docker containers to our internal CI pipeline, since it's clearly popular enough that we ought to add automated testing.

@arjendekorte
Copy link
Contributor Author

Fixing this was rather easy (even for a CMake-novice like me), once I figured our where to find the CMakeError.log file. In the openSUSE Build Service, the build environment is discarded once the build fails and only the build log remains. After running a local build, it was clear what was going on and the fix is quite trivial.

[   41s] -- Performing Test Iconv_IS_BUILT_IN
[   41s] -- Performing Test Iconv_IS_BUILT_IN - Success

Adding openSUSE docker containers is probably a good idea for the Leap (stable) versions, but likely not so much for Tumbleweed. The latter is a rolling release with (almost) daily changes, so that would require a lot of maintenance to keep it up-to-date.

@micahsnyder
Copy link
Contributor

I wasn't able to reproduce the issue with a default opensuse/tumbleweed:latest container.
I did reproduce it by building with:

cmake .. -D CMAKE_C_FLAGS="-Werror=return-type"

and confirmed that your PR fixes the issue.

@micahsnyder micahsnyder merged commit 9f8b01e into Cisco-Talos:main Jul 30, 2021
22 of 24 checks passed
@micahsnyder
Copy link
Contributor

Merged to main and cherry-picked to the rel/0.104 branch so it gets into the next RC & release

@arjendekorte
Copy link
Contributor Author

You probably couldn't reproduce this, because you didn't use the openSUSE Build Service (which is used for building all packages). By default, the CFLAGS are -O2 -Wall -D_FORTIFY_SOURCE=2 -fstack-protector-strong -funwind-tables -fasynchronous-unwind-tables -fstack-clash-protection -Werror=return-type -flto=auto -g -DNDEBUG, so this is where the error is from.

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.

None yet

2 participants