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

Windows build support #71

Merged
merged 7 commits into from
Nov 20, 2018
Merged

Windows build support #71

merged 7 commits into from
Nov 20, 2018

Conversation

dneto0
Copy link
Collaborator

@dneto0 dneto0 commented Nov 19, 2018

Builds on #59

And then also:

  • force static linking of C Runtime into executables
  • Use fopen_s to avoid security warning in MSVC

This version completes the build, then fails a unit test.

@dneto0 dneto0 requested a review from dj2 November 19, 2018 21:59
@dneto0 dneto0 force-pushed the use-fopen-s branch 2 times, most recently from 5b66225 to 45be664 Compare November 19, 2018 22:19
@dneto0
Copy link
Collaborator Author

dneto0 commented Nov 19, 2018

I think the Windows build support is ready now.

Getting failures on Linux and Windows, as described by the bots:

[ RUN      ] CommandParserTest.MaxDepthBounds
1093
../src/vkscript/command_parser_test.cc:1666: Failure
1094
Expected equality of these values:
1095
  3.f
1096
    Which is: 3
1097
  cp.PipelineDataForTesting()->GetMaxDepthBounds()
1098
    Which is: 3.4000001
1099
[  FAILED  ] CommandParserTest.MaxDepthBounds (0 ms)

I don't know where that's coming from. I tried a GCC UBSAN debug build and it didn't catch the problem. ???

@dneto0
Copy link
Collaborator Author

dneto0 commented Nov 19, 2018

Ok. The test had been accidentally broken by the first commit in this MR. :-)

@dj2
Copy link
Collaborator

dj2 commented Nov 20, 2018

Hah, nice find. And oops.

@dj2 dj2 mentioned this pull request Nov 20, 2018
@dneto0 dneto0 merged commit e390591 into google:master Nov 20, 2018
@dj2 dj2 mentioned this pull request Nov 21, 2018
@dneto0 dneto0 deleted the use-fopen-s branch November 23, 2018 15:15
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