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

Fix command line build for macOS #49

Merged
merged 1 commit into from
Aug 10, 2017
Merged

Fix command line build for macOS #49

merged 1 commit into from
Aug 10, 2017

Conversation

eyeplum
Copy link
Contributor

@eyeplum eyeplum commented Aug 10, 2017

When try to build in command line on macOS (cmake without generating Xcode project then make), the build will fail with some errors.

Errors in this repo

This pull request fixes an error in the examples.

Erros in the contribs

There are also two errors in one of the dependencies:

/s2client-api/contrib/civetweb/src/civetweb.c:141:34: error: unknown warning group '-Wno-reserved-id-macro', ignored [-Werror,-Wunknown-warning-option]
#pragma clang diagnostic ignored "-Wno-reserved-id-macro"
                                 ^
/s2client-api/contrib/civetweb/src/civetweb.c:142:34: error: unknown warning group '-Wno-keyword-macro', ignored [-Werror,-Wunknown-warning-option]
#pragma clang diagnostic ignored "-Wno-keyword-macro"
                                 ^

When I try to build KevinCalderone/civetweb separately, these two errors appear as warnings, not sure why they are errors when build in s2client-api. Any ideas?

@AnthonyBrunasso AnthonyBrunasso merged commit 9a04eef into Blizzard:master Aug 10, 2017
@AnthonyBrunasso
Copy link
Contributor

When I get a chance I will test this on mac today.

@KevinCalderone KevinCalderone self-requested a review August 10, 2017 17:36
@KevinCalderone
Copy link

I can look into that error. If it is a legit issue, we should probably add a separate issue for it.

@KevinCalderone
Copy link

Tested it and they are appearing as warnings to me.

What compiler version are you using? Can you post the command line options it is using?

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 11, 2017

I'm using Xcode 9 beta 5 (9M202q):

$ cc -v
Apple LLVM version 9.0.0 (clang-900.0.34.1)
Target: x86_64-apple-darwin17.0.0
Thread model: posix
InstalledDir: /Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

Here is the output of make VERBOSE=1:

/Applications/Xcode-beta.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -DMAX_REQUEST_SIZE=16384 -DUSE_STACK_SIZE=102400 -DUSE_WEBSOCKET -I/s2client-api/contrib/civetweb/include  -std=c11 -Wall -Wextra -Wshadow -Wconversion -Wmissing-prototypes -Weverything -Wno-padded -Wno-unused-macros -Wno-format-nonliteral -Wparentheses -pedantic-errors -fvisibility=hidden   -o CMakeFiles/c-library.dir/civetweb.c.o   -c /s2client-api/contrib/civetweb/src/civetweb.c

Yeah, when I try to compile with Xcode 8.3.3 (8E3004b), those errors are appearing as warnings. 🤔

Also, the output of make VERBOSE=1 with Xcode 8.3.3 is identical to the one with Xcode 9:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc -DMAX_REQUEST_SIZE=16384 -DUSE_STACK_SIZE=102400 -DUSE_WEBSOCKET -I/s2client-api/contrib/civetweb/include  -std=c11 -Wall -Wextra -Wshadow -Wconversion -Wmissing-prototypes -Weverything -Wno-padded -Wno-unused-macros -Wno-format-nonliteral -Wparentheses -pedantic-errors -fvisibility=hidden   -o CMakeFiles/c-library.dir/civetweb.c.o   -c /s2client-api/contrib/civetweb/src/civetweb.c

@KevinCalderone
Copy link

Interesting. You also said the behavior was different when you cloned/build the civetweb repo directly. What command line in that case for Xcode 8.3.3 vs Xcode 9? Are they any different?

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 11, 2017

Xcode 8.3.3:

cc -c -Wall -Wextra -Wshadow -Wformat-security -Winit-self -Wmissing-prototypes -DOSX -Iinclude  -DUSE_STACK_SIZE=102400 -O2 -DNDEBUG src/civetweb.c -o out/src/civetweb.o
src/civetweb.c:141:34: warning: unknown warning group '-Wno-reserved-id-macro', ignored [-Wunknown-pragmas]
#pragma clang diagnostic ignored "-Wno-reserved-id-macro"
                                 ^
src/civetweb.c:142:34: warning: unknown warning group '-Wno-keyword-macro', ignored [-Wunknown-pragmas]
#pragma clang diagnostic ignored "-Wno-keyword-macro"
                                 ^
2 warnings generated.

Xcode 9:

cc -c -Wall -Wextra -Wshadow -Wformat-security -Winit-self -Wmissing-prototypes -DOSX -Iinclude  -DUSE_STACK_SIZE=102400 -O2 -DNDEBUG src/civetweb.c -o out/src/civetweb.o
src/civetweb.c:141:34: warning: unknown warning group '-Wno-reserved-id-macro', ignored [-Wunknown-warning-option]
#pragma clang diagnostic ignored "-Wno-reserved-id-macro"
                                 ^
src/civetweb.c:142:34: warning: unknown warning group '-Wno-keyword-macro', ignored [-Wunknown-warning-option]
#pragma clang diagnostic ignored "-Wno-keyword-macro"
                                 ^
2 warnings generated.

Interesting. The flag that triggers these warnings seems to be different in Xcode 8.3.3 (-Wunknown-pragmas) and Xcode 9 (-Wunknown-warning-option).

Looks like an implementation change in clang? But that didn't explain why they are appearing as errors in the main project when built with Xcode 9.

@KevinCalderone
Copy link

Yeah when just comparing Xcode 9 between the api and civetweb repo, the arguments you posted are different. There must be something about those flags that is causing a difference in behavior.

I don't see -Werror in these options, so I am confused why it is showing up as an error. Maybe one of the other flags is indirectly turning errors on for that warning type?

From what you posted:

Civetweb repo
cc -c -Wall -Wextra -Wshadow -Wformat-security -Winit-self -Wmissing-prototypes -DOSX -Iinclude -DUSE_STACK_SIZE=102400 -O2 -DNDEBUG src/civetweb.c -o out/src/civetweb.o
-> Warning

API repo
cc -DMAX_REQUEST_SIZE=16384 -DUSE_STACK_SIZE=102400 -DUSE_WEBSOCKET -I/s2client-api/contrib/civetweb/include -std=c11 -Wall -Wextra -Wshadow -Wconversion -Wmissing-prototypes -Weverything -Wno-padded -Wno-unused-macros -Wno-format-nonliteral -Wparentheses -pedantic-errors -fvisibility=hidden -o CMakeFiles/c-library.dir/civetweb.c.o -c /s2client-api/contrib/civetweb/src/civetweb.c
-> Error

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 12, 2017

You are correct! The -pedantic-errors causes those warnings to be treated as errors.

You can try compiling civetweb.c with -pedantic-errors flag:

$ cc -c -Wall -Wextra -Wshadow -Wformat-security -Winit-self -Wmissing-prototypes -pedantic-errors -DOSX -Iinclude -DUSE_STACK_SIZE=102400 -O2 -DNDEBUG src/civetweb.c -o out/src/civetweb.o

src/civetweb.c:141:34: error: unknown warning group '-Wno-reserved-id-macro', ignored [-Werror,-Wunknown-warning-option]
#pragma clang diagnostic ignored "-Wno-reserved-id-macro"
                                 ^
src/civetweb.c:142:34: error: unknown warning group '-Wno-keyword-macro', ignored [-Werror,-Wunknown-warning-option]
#pragma clang diagnostic ignored "-Wno-keyword-macro"
                                 ^
2 errors generated.

It's coming from the CMakeLists.txt of civetweb.

This explains why make was fine, because this flag will only be there if we use CMake.

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 12, 2017

Also, since these two warning options don't exist (at least for the clang versions we are using), can we remove them so there will be no warnings in the first place?

@KevinCalderone
Copy link

KevinCalderone commented Aug 12, 2017

These two problematic lines are also inside of civetweb:
civetweb/civetweb@62c8182

As you pointed out, it looks like this is an issue with how their CMake target is setup. I can reproduce the same issue in the latest of their master branch. I see their Travis CI jobs are only testing Xcode8, so they may not be aware of the issue.

I think we should file an issue in their repository. Do you want to help summarize our findings here and submit that?

In the interim, you can work around by compiling with 8.3.3. If the problem still persists after Xcode 9 is officially released, we can try to find a workaround. Thanks for all the help investigating this!

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 12, 2017

Sure, I will file an issue in their repo. Thanks for your help!

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 16, 2017

@KevinCalderone Hello, issue civetweb/civetweb#503 has been fixed, can we update the dependency to pull the fix in? Thanks!

@KevinCalderone
Copy link

Thanks for following up on that!

I'm not sure how stable their latest master branch is, but I can at least pull that one fix into our civetweb fork.

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 16, 2017

Sounds great, thanks!

KevinCalderone pushed a commit that referenced this pull request Aug 16, 2017
Updated civetweb submodule to fix Xcode9 error. (Pull request #49)
@KevinCalderone
Copy link

Ok I have updated the submodule. The warning seems to be gone for me. Can you retest it please?

@eyeplum
Copy link
Contributor Author

eyeplum commented Aug 17, 2017

Yeah, tested with Xcode 9, and it builds fine.

Thanks!

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

3 participants