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

disable HAVE_PRINTF_Z when directly compiling on Windows #166

Merged
merged 3 commits into from Sep 17, 2019

Conversation

Adsun701
Copy link
Contributor

@Adsun701 Adsun701 commented Sep 16, 2019

This fixes warnings when directly compiling on Windows rather than just cross-compiling. The CheckCSourceRuns check for HAVE_PRINTF_Z somehow results in a success for HAVE_PRINTF_Z when directly configuring on Windows, resulting in multiple format warnings during compilation.

Adsun701 and others added 3 commits Sep 16, 2019
Re-arrange the logic to be a little less convoluted, and add comments explaining why it's done that way.
@dbaarda
Copy link
Member

@dbaarda dbaarda commented Sep 17, 2019

Thanks again for this. It's pretty strange. Are you sure that the HAVE_PRINTF_Z checks weren't being cached from an earlier (perhaps cross-compiled) cmake run? Try purging your git client and trying again with;

$ git clean -fdx
$ cmake .

Assuming it wasn't as simple as that, I've done some tidyups and added comments, and I'll probably merge this soon.

@Adsun701
Copy link
Contributor Author

@Adsun701 Adsun701 commented Sep 17, 2019

@dbaarda I am pretty sure HAVE_PRINTF_Z checks were not being cached from an earlier run. Did git clean -fdx and the problem was still there. I tested the new commits on both Windows and Linux, and they work fine.

Thanks for your help!

@dbaarda dbaarda merged commit c9f5b2c into librsync:master Sep 17, 2019
1 check passed
@Adsun701 Adsun701 deleted the native-win32-warning-fix branch Sep 17, 2019
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