Android and Windows (MSVC2019, MinGW32/64) ready, and other minor improvements (#76)

* Fix out-of-source build

 Now `cmake -B target-dir -S .` works fine

* Remove obsolete options and other garbage, improve warnings

TODO: fix warnings

* _REENTRANT is dead since POSIX 1995
* -O3 should not be turned on for all compilers, but is on by default for fresh GCC
* -g for Debug is redundant (-ggdb3 or something like this is preferable in some cases)
* _XOPEN_SOURCE for better cross-platform compatibility
* CMake does not need `distclean`: just remove the whole directory. In case of mistake when build is located in the source tree just stick to `git clean -dfx`
* -Wextra provides useful warnings

* Set target directories from build script

* Fix types: avoid system id_t, use size_t instead

Now can build for MinGW

* Avoid implicit copy in for-loops

* Fix potentially uninitialized POD-fields

And few minor code improvements

* Update Eigen submodule to the next patch 3.3.9

* MSVC2019 compatiblity

But missing corresponding CI at the moment

* Update PyBind module to 2.6.2

* Fix CI build problem: use correct full-featured git with ssh

Also few warnings fixed in code

* Android-ready version

Author: iakov

.circleci/config.yml

@@ -20,9 +20,11 @@ jobs:
       - run:
           name: "Configure and build native mrob library with Python bindings"
           command: |
-            mkdir -p build && pushd build
-            cmake $(dirs -l +1) && make -j 4
-            popd
+            mkdir -p build
+            cmake -S $PWD -B $PWD/build \
+             -DCMAKE_RUNTIME_OUTPUT_DIRECTORY=$PWD/bin \
+             -DCMAKE_LIBRARY_OUTPUT_DIRECTORY=$PWD/lib \
+            && cmake --build build -j $(nproc)
       - run:
           name: "Run FGraph_2d"
           command: |
@@ -62,6 +64,9 @@ jobs:
     docker:
       - image: quay.io/pypa/manylinux2010_x86_64
     steps:
+      - run:
+          name: "Install additional utils"
+          command: yum install -y chrpath git openssh-clients
       - checkout
       - run:
           name: "Install fresh CMake into ManyLinux container"
@@ -71,9 +76,6 @@ jobs:
       - run:
           name: "Clone submodules"
           command: git submodule update --init --recursive
-      - run:
-          name: "Install additional utils"
-          command: yum install -y chrpath
       - run:
           name: "Build Python wheels"
           command: scripts/build-wheels.sh
@@ -136,14 +138,14 @@ jobs:
           when: always
   publish-github-release:
     docker:
-      - image: cibuilds/github:0.10
+      - image: cibuilds/github:0.13
     steps:
       - attach_workspace:
           at: ~/
       - run:
           name: "Publish Release on GitHub"
           command: |
-              VERSION=v$(ls ~/project/build/wheelhouse/*.whl | awk -F"-" '{ print $2 }')
+              VERSION=v$(ls ~/project/build/wheelhouse/*.whl | cut -f2 -d-)
               ghr -t "${GITHUB_TOKEN}" \
                   -u "${CIRCLE_PROJECT_USERNAME}" \
                   -r "${CIRCLE_PROJECT_REPONAME}" \

.github/workflows/wheels.yml

@@ -1,6 +1,6 @@
 name: Build macOS wheels
 
-on: [push, pull_request]
+on: [push, pull_request, workflow_dispatch]
 
 jobs:
   build_wheel:
@@ -19,7 +19,7 @@ jobs:
         run: brew install coreutils
 
       - name: Build wheel
-        run: sudo ./scripts/build-wheels-macOS.sh
+        run: ./scripts/build-wheels-macOS.sh
 
       - uses: actions/upload-artifact@v2
         with:

CMakeLists.txt

@@ -6,33 +6,31 @@ if(COMMAND cmake_policy)
   cmake_policy(SET CMP0003 NEW)
 endif(COMMAND cmake_policy)
 
-# The project name and the type of project
 PROJECT(mrob)
-#MESSAGE(Project\ name:\ ${PROJECT_NAME})
-
-SET(EXECUTABLE_OUTPUT_PATH ${CMAKE_CURRENT_SOURCE_DIR}/bin)
-SET(LIBRARY_OUTPUT_PATH ${CMAKE_CURRENT_SOURCE_DIR}/lib)
-SET(CMAKE_INSTALL_PREFIX /usr/local)
 
 IF (NOT CMAKE_BUILD_TYPE)
  SET(CMAKE_BUILD_TYPE "Release") 
 ENDIF (NOT CMAKE_BUILD_TYPE)
 
-# flag _REENTRANT for safe threads and NDEBUG to disable asserts
-SET(CMAKE_CXX_FLAGS_DEBUG "-g -Wall -D_REENTRANT")
-SET(CMAKE_CXX_FLAGS_RELEASE "-O3 -D_REENTRANT -DNDEBUG")
-#SET(CMAKE_CXX_STANDARD 11)
 SET(CMAKE_CXX_STANDARD 14)
+SET(CMAKE_CXX_EXTENSIONS ON)
+
+IF(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+    SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") #TODO: /Wall but disable noisy warnings
+    ADD_COMPILE_DEFINITIONS(_USE_MATH_DEFINES)
+    SET(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
+ELSEIF(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+    SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpedantic")
+ELSEIF(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+    SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Wpedantic -Wno-inconsistent-missing-override")
+ELSE()
+ENDIF(CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
+
+
 
 # ===================================================================
-# DEPENDENCIES: locate the necessary dependencies for the project
-# DEPENDENCIES: Eigen (submodule pointing to 3.3.7)
-SET(Eigen_INCLUDE_DIRS ./external/Eigen)
-INCLUDE_DIRECTORIES(${Eigen_INCLUDE_DIRS})
+INCLUDE_DIRECTORIES(SYSTEM ./external/Eigen)
 
-# DEPENDENCIES: pybind11 (submodule)
-SET(PYBIND11_CPP_STANDARD -std=c++14)
-ADD_SUBDIRECTORY(./external/pybind11)
 
 
 # ===================================================================
@@ -57,42 +55,7 @@ ADD_SUBDIRECTORY(./src/PCRegistration)
 
 # New modules should be included here
 
-# MROB python bingins, defined here:
-ADD_SUBDIRECTORY(./mrobpy)
-
-
-
-ADD_CUSTOM_TARGET (distclean @echo cleaning cmake files)
-
-IF (UNIX)
-  ADD_CUSTOM_COMMAND(
-    COMMENT "distribution clean"
-    COMMAND make ARGS clean
-    COMMAND rm ARGS -rf ${CMAKE_SOURCE_DIR}/build/*
-
-    TARGET  distclean
-  )
-ELSE(UNIX)
-  ADD_CUSTOM_COMMAND(
-    COMMENT "distclean only implemented in unix"
-    TARGET  distclean
-  )
-ENDIF(UNIX)
-
-ADD_CUSTOM_TARGET (uninstall @echo uninstall package)
-
-IF (UNIX)
-  ADD_CUSTOM_COMMAND(
-    COMMENT "uninstall package"
-    COMMAND xargs ARGS rm < install_manifest.txt
-
-    TARGET  uninstall
-  )
-ELSE(UNIX)
-  ADD_CUSTOM_COMMAND(
-    COMMENT "uninstall only implemented in unix"
-    TARGET  uninstall
-  )
-ENDIF(UNIX)
-
-
+IF(NOT ANDROID)
+    ADD_SUBDIRECTORY(./external/pybind11)
+    ADD_SUBDIRECTORY(./mrobpy)
+ENDIF(NOT ANDROID)

external/Eigen

@@ -58,6 +58,7 @@ using namespace mrob;
 class FGraphPy : public FGraphSolve
 {
 public:
+    using id_t = Factor::id_t;
     /**
      * Constructor for the python binding. By default uses the Cholesky adjoint solving type, and some estimated number of nodes and factors.
      */

external/pybind11

@@ -1,3 +1,4 @@
+#!/bin/bash
 # Copyright (c) 2018, Skolkovo Institute of Science and Technology (Skoltech)
 # 
 #  Licensed under the Apache License, Version 2.0 (the "License");
@@ -20,7 +21,6 @@
 #               miloslubov@gmail.com
 #
 
-#!/bin/bash
 set -euo pipefail
 export LC_ALL=C
 export MACOSX_DEPLOYMENT_TARGET=10.9
@@ -40,10 +40,10 @@ do
     cmake .. -DPYTHON_EXECUTABLE:FILEPATH=$PYBIN \
              -DCMAKE_MACOSX_RPATH=ON \
              -DCMAKE_BUILD_WITH_INSTALL_RPATH=TRUE \
-             -DCMAKE_INSTALL_RPATH="@loader_path" 
-    make -j $NUMPROC
-    
-    mv ../lib/* ../mrob
+             -DCMAKE_INSTALL_RPATH="@loader_path" \
+             -DCMAKE_RUNTIME_OUTPUT_DIRECTORY=$PWD/../bin \
+             -DCMAKE_LIBRARY_OUTPUT_DIRECTORY=$PWD/../mrob \
+    && cmake --build . -j $NUMPROC
 done
 
 cd ../

mrobpy/FGraphPy.cpp

@@ -1,3 +1,4 @@
+#!/bin/bash
 # Copyright (c) 2018, Skolkovo Institute of Science and Technology (Skoltech)
 # 
 #  Licensed under the Apache License, Version 2.0 (the "License");
@@ -20,7 +21,6 @@
 #               miloslubov@gmail.com
 #
 
-#!/bin/bash
 set -euo pipefail
 export LC_ALL=C
 
@@ -34,23 +34,23 @@ cp ./__init__.py ./mrob/__init__.py
 
 cd ./build
 
-NUMPROC=$(grep -Ec '^processor\s+:' /proc/cpuinfo)
+NUMPROC=$(nproc)
 echo "Running $NUMPROC parallel jobs"
 
 LATEST=""
 
 for PYBIN in /opt/python/cp3*/bin/
 do
     LATEST=${PYBIN}
-
-    cmake .. -DPYTHON_EXECUTABLE:FILEPATH=${PYBIN}python3
-    make -j $NUMPROC
-    
-    mv --verbose ../lib/* ../mrob
+    cmake -S .. -B . \
+             -DPYTHON_EXECUTABLE:FILEPATH=${PYBIN}python3 \
+             -DCMAKE_RUNTIME_OUTPUT_DIRECTORY=$PWD/../bin \
+             -DCMAKE_LIBRARY_OUTPUT_DIRECTORY=$PWD/../mrob \
+    && cmake --build . -j $(nproc)
 done
 
 chrpath -r '$ORIGIN' ../mrob/mrob.*.so
-${LATEST}python3 -m pip install --user -q pep517
+${LATEST}python3 -m pip install $([[ -n "$VIRTUAL_ENV" ]] || echo "--user") -q pep517 auditwheel
 ${LATEST}python3 -m pep517.build ../
 auditwheel repair ../dist/*.whl
 

scripts/build-wheels-macOS.sh

@@ -35,7 +35,7 @@ FGraph::FGraph() :
 FGraph::~FGraph()
 {
     // clear every node's list of neigbours factors
-    for (auto n: nodes_)
+    for (auto &&n: nodes_)
         n->clear();
     factors_.clear();
     nodes_.clear();
@@ -45,8 +45,8 @@ bool FGraph::add_factor(std::shared_ptr<Factor> &factor)
 {
 	factor->set_id(factors_.size());
 	factors_.push_back(factor);
-    auto list = factor->get_neighbour_nodes();
-    for( auto n: *list)
+    auto *list = factor->get_neighbour_nodes();
+    for( auto &&n: *list)
     {
         n->add_factor(factor);
     }
@@ -101,9 +101,9 @@ void FGraph::print(bool completePrint) const
 
     if(completePrint)
     {
-        for (auto n : nodes_)
+        for (auto &&n : nodes_)
             n->print();
-        for (auto f : factors_)
+        for (auto &&f : factors_)
             f->print();
     }
 }

scripts/build-wheels.sh

@@ -226,7 +226,7 @@ void FGraphSolve::build_adjacency()
     indNodesMatrix.reserve(nodes->size());
 
     N_ = 0;
-    for (id_t i = 0; i < nodes->size(); ++i)
+    for (size_t i = 0; i < nodes->size(); ++i)
     {
         // calculate the indices to access
         uint_t dim = (*nodes)[i]->get_dim();
@@ -365,14 +365,14 @@ void FGraphSolve::update_nodes()
 
 void FGraphSolve::synchronize_nodes_auxiliary_state()
 {
-    for (auto n : nodes_)
+    for (auto &&n : nodes_)
         n->set_auxiliary_state(n->get_state());
 }
 
 
 void FGraphSolve::synchronize_nodes_state()
 {
-    for (auto n : nodes_)
+    for (auto &&n : nodes_)
         n->set_state(n->get_auxiliary_state());
 }