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.

Sign up for GitHub

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

Jump to bottom

Problem: Windows build broken because of multiple issues #204

Merged
merged 5 commits into from
Apr 17, 2018
Merged

Problem: Windows build broken because of multiple issues #204

merged 5 commits into from
Apr 17, 2018

Conversation

kurdybacha
Copy link
Contributor

Windows issues:

  • missing includes files
    Solution: added missing and <unordered_map>
    Here was replaced with <unordered_map> as there is no need for
    sorted map.
  • googletest fails because deprecation warning causing errors.
    Solution: D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING option added.
  • googletest fails because by default, new Visual Studio projects
    link the C runtimes dynamically but Google Test links them statically.
    Solution: gtest_force_shared_crt=ON option added.

Two last point should fix issue #199.

Besides adding appveyor.yml configuration to add Windows build to
CI in order to prevent accidental Windows build breakage on merge.
For now only Debug configuration as Release requires more time to figure
out.

@kurdybacha
Problem: Windows build broken because of multiple issues
0150281 
Windows issues:
* missing includes files
  Solution: added missing <memory> and <unordered_map>
  Here <map> was replaced with <unordered_map> as there is no need for
  sorted map.
* googletest fails because deprecation warning causing errors.
  Solution: D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING option added.
* googletest fails because by default, new Visual Studio projects
  link the C runtimes dynamically but Google Test links them statically.
  Solution: gtest_force_shared_crt=ON option added.

Besides that adding appveyor.yml configuration to add Windows build to
CI in order to prevent accidental Windows build breakage.
For now only Debug configuration as Release requires more time to figure
out.
@coveralls
Copy link

coveralls commented Apr 16, 2018 •
edited
Loading

Pull Request Test Coverage Report for Build 34

  • 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage remained the same at 56.25%
Files with Coverage Reduction New Missed Lines %
zmq.hpp 2 64.38%
Totals Coverage Status
Change from base Build 30: 0.0%
Covered Lines: 36
Relevant Lines: 64
💛 - Coveralls

@bluca bluca closed this Apr 16, 2018
@bluca bluca reopened this Apr 16, 2018
@bluca
Copy link
Member

bluca commented Apr 16, 2018

I've enabled appveyor let's see if it runs

@bluca
Copy link
Member

bluca commented Apr 16, 2018

Building libzmq on appveyor is extremely slow for some reasons...
If possible, you might want to disable building the tests as that might speed it up - as opposed to autotools, it looks like with cmake the tests are always built with "make" rather than "make test/check"

@bluca
Copy link
Member

bluca commented Apr 16, 2018

The test fails:

1: Test command: C:\projects\cppzmq\Build\bin\Debug\unit_tests.exe
1: Test timeout computed to be: 10000000
1/1 Test #1: unit .............................***Exception: Exit code 0xc0000135
  0.20 sec

@kurdybacha
Copy link
Contributor Author

@bluca build it is really slow indeed, don't know why though yet.
I will check in the evening and come back on this. Thanks.

@kurdybacha
Problem: Windows build takes too long
c4d6532 
Solution: disabling tests and perf tools
@bluca
Copy link
Member

bluca commented Apr 16, 2018

Note that the slow build is a problem in libzmq's repository as well, so it's nothing to do with the appveyor file you added. Try with -DBUILD_TESTS=OFF

@kurdybacha
Copy link
Contributor Author

kurdybacha commented Apr 16, 2018 •
edited
Loading

I have disabled tests and perfs so build time is 2x faster now (~5min). Proper way would be to "cache" googletest and libzmq build between runs. Until then I am thinking to disable VS 2015.
Failing ctest is still pending investigation.

@bluca
Copy link
Member

bluca commented Apr 16, 2018

If you could implement ccache it would be fantastic, please also port it to libzmq and zproject :-)
It takes more than an hour right now to get a full run of the libzmq appveyor tests...

@kurdybacha
Copy link
Contributor Author

@bluca Windows build now passes on my branch. Should be fine here in a bit...

@bluca bluca merged commit fdc145a into zeromq:master Apr 17, 2018
sigiesec
sigiesec reviewed Apr 17, 2018
View reviewed changes
zmq.hpp
@@ -1107,7 +1108,7 @@ namespace zmq
private:
void *poller_ptr;
std::vector<zmq_poller_event_t> poller_events;
std::map<socket_t*, handler_t> handlers;
std::unordered_map<socket_t*, handler_t> handlers;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this change is related to the Windows build support (maybe #include was missing?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sigiesec sigiesec sigiesec left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@kurdybacha @coveralls @bluca @sigiesec