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

Add Windows installation via conda-forge #207

Closed
wants to merge 3 commits into from

Conversation

mabelzhang
Copy link
Contributor

@mabelzhang mabelzhang commented Dec 24, 2020

Partially addresses gazebosim/docs#117

This replaces the old Windows installation instructions from Nov 2017, which installed dependencies by zip files and used nmake with VS 2013 on Windows 7 and 8. We have now decided to go with conda-forge, tested with VS 2019 on Windows 10.

The current instructions built and installed, but I'm unsure about two things, which may be optional.

1. CMake warning about IFADDRS

I got a CMake warning about missing IFADDRS:

-- BUILD WARNINGS
--      Missing: IFADDRS
-- END BUILD WARNINGS

I tried installing ifaddr, but that didn't fix the warning after removing the build directory and rerunning cmake:

conda install -c conda-forge ifaddr

Seeing that ifaddr is for network interfaces, and this is transport, I don’t know if that warning can be ignored.
What does it affect specifically, do we need to find it?

2. Errors building example/

The old instructions built the examples and ran responser and requester.
I wanted to keep that, but I wasn’t able to build the examples. I can run up to this:

cd ign-transport\example
mkdir build
cd build
# I used a custom -DCMAKE_INSTALL_PREFIX and had to specify the path to ignition-transport9-config.cmake
cmake .. -DCMAKE_PREFIX_PATH=Z:\sim\ign_ws\install\ignition-transport9\lib\cmake\ignition-transport9

Then when I ran

cmake --build .

I got a lot of warnings and a few errors.

Microsoft (R) Build Engine version 16.8.2+25e4d540b for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.

  Checking Build System
  Building Custom Rule Z:/sim/ign_ws/src/ign-transport/example/CMakeLists.txt
  publisher.cc
  publisher.vcxproj -> Z:\sim\ign_ws\src\ign-transport\example\build\Debug\publisher.exe
  Building Custom Rule Z:/sim/ign_ws/src/ign-transport/example/CMakeLists.txt
  publisher_c.cc
Z:\sim\ign_ws\src\ign-transport\example\publisher_c.cc(18,10): fatal error C1083: Cannot open in
clude file: 'unistd.h': No such file or directory [Z:\sim\ign_ws\src\ign-transport\example\build
\publisher_c.vcxproj]
  Building Custom Rule Z:/sim/ign_ws/src/ign-transport/example/CMakeLists.txt
  publisher_c_fast.cc
Z:\sim\ign_ws\src\ign-transport\example\publisher_c_fast.cc(18,10): fatal error C1083: Cannot op
en include file: 'unistd.h': No such file or directory [Z:\sim\ign_ws\src\ign-transport\example\
build\publisher_c_fast.vcxproj]
  Running cpp protocol buffer compiler on stringmsg.proto
  Building Custom Rule Z:/sim/ign_ws/src/ign-transport/example/msgs/CMakeLists.txt
  Building Custom Rule Z:/sim/ign_ws/src/ign-transport/example/CMakeLists.txt
  publisher_custom_msg.cc
C:\Users\mabel\miniconda3\envs\ign-ws\Library\include\google/protobuf/stubs/logging.h(102,23): w
arning C4251: 'google::protobuf::internal::LogMessage::message_': class 'std::basic_string<char,
std::char_traits<char>,std::allocator<char>>' needs to have dll-interface to be used by clients
of class 'google::protobuf::internal::LogMessage' [Z:\sim\ign_ws\src\ign-transport\example\build
\publisher_custom_msg.vcxproj]
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\
xstring(4648): message : see declaration of 'std::basic_string<char,std::char_traits<char>,std::
allocator<char>>' [Z:\sim\ign_ws\src\ign-transport\example\build\publisher_custom_msg.vcxproj]
C:\Users\mabel\miniconda3\envs\ign-ws\Library\include\google/protobuf/io/coded_stream.h(1250,64)
: warning C4251: 'google::protobuf::io::CodedOutputStream::default_serialization_deterministic_'
: struct 'std::atomic<bool>' needs to have dll-interface to be used by clients of class 'google:
:protobuf::io::CodedOutputStream' [Z:\sim\ign_ws\src\ign-transport\example\build\publisher_custo
m_msg.vcxproj]
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\VC\Tools\MSVC\14.28.29333\include\
atomic(2759): message : see declaration of 'std::atomic<bool>' [Z:\sim\ign_ws\src\ign-transport\
example\build\publisher_custom_msg.vcxproj]
... # more warnings
msgs\stringmsg.pb.cc : fatal error LNK1107: invalid or corrupt file: cannot read at 0x3028 [Z:\s
im\ign_ws\src\ign-transport\example\build\publisher_custom_msg.vcxproj]
... # more warnings
msgs\stringmsg.pb.cc : fatal error LNK1107: invalid or corrupt file: cannot read at 0x3028 [Z:\s
im\ign_ws\src\ign-transport\example\build\subscriber_custom_msg.vcxproj]
... # more warnings

@github-actions github-actions bot added 🏢 edifice Ignition Edifice 🔮 dome Ignition Dome labels Dec 24, 2020
@mabelzhang mabelzhang added the Windows Windows support label Dec 24, 2020
@codecov
Copy link

codecov bot commented Dec 24, 2020

Codecov Report

Merging #207 (fc1e3b8) into ign-transport9 (72dfeb8) will decrease coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                 @@
##           ign-transport9     #207      +/-   ##
==================================================
- Coverage           88.98%   88.98%   -0.01%     
==================================================
  Files                  49       47       -2     
  Lines                4668     4484     -184     
==================================================
- Hits                 4154     3990     -164     
+ Misses                514      494      -20     
Impacted Files Coverage Δ
src/NodeShared.cc 79.06% <0.00%> (-0.49%) ⬇️
src/Node.cc 92.32% <0.00%> (-0.02%) ⬇️
src/NodeSharedPrivate.hh 100.00% <0.00%> (ø)
src/TopicStatistics.cc
include/ignition/transport/TopicStatistics.hh
include/ignition/transport/Discovery.hh 87.86% <0.00%> (+0.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 72dfeb8...cdd1dac. Read the comment docs.

@traversaro
Copy link
Contributor

I got a lot of warnings and a few errors.

I am not sure if the error is related to that, but conda-forge only ships Release libraries, and from the output of the example it seems that you are compiling the examples in Debug mode. Unfortunately, the C++ STL library that ships with MSVC has different ABIs in Release and in Debug, so if you compile the libraries in Release, you need to also compiles the examples in Release or RelWithDebInfo. On Linux/macOS the situation is different because both libstdcxx and libcxx are ABI compatible between Release and Debug builds.

@traversaro
Copy link
Contributor

Then I think that the header https://github.com/ignitionrobotics/ign-transport/blob/ign-transport9/example/publisher_c.cc#L18 is Unix-only and can be probably dropped with no problems.

Signed-off-by: Mabel Zhang <[email protected]>
@chapulina chapulina added the documentation Improvements or additions to documentation label Dec 28, 2020
Copy link

@JShep1 JShep1 left a comment

Choose a reason for hiding this comment

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

A couple feedback comments, but LGTM

cmake --build . # This currently does not succeed
```

Try an example
Copy link

Choose a reason for hiding this comment

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

For me, the executables are placed within a Debug folder, should we add --config Release to the above build command?

cmake --build . --config Release
```

1. Optionally, install
Copy link

Choose a reason for hiding this comment

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

I had to run in admin mode to get the install to work, maybe a note here indicating that the user will need to do that too if they wish to install.

@JShep1 JShep1 changed the base branch from ign-transport9 to ign-transport8 January 20, 2021 07:05
@JShep1 JShep1 changed the base branch from ign-transport8 to ign-transport9 January 20, 2021 07:06
@JShep1 JShep1 mentioned this pull request Jan 20, 2021
@JShep1
Copy link

JShep1 commented Jan 20, 2021

Closing this PR in favor of retargeting to Citadel. #214

@JShep1 JShep1 closed this Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice Windows Windows support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants