Streamline CMake and build system, move to newer protobuf (#150)

.github/workflows/appimage.yml

@@ -31,15 +31,13 @@ jobs:
       uses: actions/checkout@v3
       with:
         submodules: recursive
-        path: viam-cartographer
 
     - name: Check out PR branch code
       if: github.event_name == 'pull_request_target'
       uses: actions/checkout@v3
       with:
         ref: ${{ github.event.pull_request.head.sha }}
         submodules: recursive
-        path: viam-cartographer
 
     - name: apt update
       run: |
@@ -48,31 +46,20 @@ jobs:
     - name: make clean
       run: |
         chown -R testbot:testbot .
-        sudo -u testbot bash -lc 'cd viam-cartographer && make clean'
+        sudo -u testbot bash -lc 'make clean'
         
-    - name: make bufinstall buf setup
-      uses: nick-fields/retry@v2
-      with:
-        timeout_minutes: 5
-        max_attempts: 3
-        retry_on: timeout
-        shell: bash
-        command: |
-          chown -R testbot:testbot .
-          sudo -u testbot bash -lc 'cd viam-cartographer && make bufinstall buf setup'
-
-    - name: make build
+    - name: make setup
       run: |
-        sudo -u testbot bash -lc 'cd viam-cartographer && make build'
+        sudo -u testbot bash -lc 'make setup'
 
-    - name: make install-lua-files
+    - name: make build
       run: |
-        sudo -u testbot bash -lc 'cd viam-cartographer && make install-lua-files'
+        sudo -u testbot bash -lc 'make build'
 
     - name: Build AppImage (PR)
       if: contains(github.event.pull_request.labels.*.name, 'appimage') || contains(github.event.pull_request.labels.*.name, 'appimage-ignore-tests')
       run: |
-        sudo -u testbot bash -lc 'cd viam-cartographer && make BUILD_CHANNEL="pr-${{ github.event.pull_request.number }}" appimage'
+        sudo -u testbot bash -lc 'make BUILD_CHANNEL="pr-${{ github.event.pull_request.number }}" appimage'
 
     - name: Authorize GCP Upload
       uses: google-github-actions/auth@v1
@@ -84,22 +71,22 @@ jobs:
       uses: google-github-actions/upload-cloud-storage@v0.10.4
       with:
         headers: "cache-control: no-cache"
-        path: 'viam-cartographer/etc/packaging/appimages/deploy/'
+        path: 'etc/packaging/appimages/deploy/'
         destination: 'packages.viam.com/apps/slam-servers/'
         glob: '*'
         parent: false
         gzip: false
 
     - name: Build AppImage (Latest)
       if: github.event_name == 'workflow_dispatch' || github.event_name == 'push'
-      run: sudo -u testbot bash -lc 'cd viam-cartographer && make appimage-ci'
+      run: sudo -u testbot bash -lc 'make appimage-ci'
 
     - name: Deploy Files (Latest)
       if: github.event_name == 'workflow_dispatch' || github.event_name == 'push'
       uses: google-github-actions/upload-cloud-storage@v0.10.4
       with:
         headers: "cache-control: no-cache"
-        path: 'viam-cartographer/etc/packaging/appimages/deploy/'
+        path: 'etc/packaging/appimages/deploy/'
         destination: 'packages.viam.com/apps/slam-servers/'
         glob: '*'
         parent: false

.github/workflows/test.yml

@@ -57,59 +57,47 @@ jobs:
 
     - name: apt update
       run: |
-        sudo apt update
+        apt-get update
 
     - name: Verify no uncommitted changes from make lint
       run: |
         git init
         git add .
         chown -R testbot:testbot .
-        sudo -u testbot bash -lc 'make lint-setup lint'
-        GEN_DIFF=$(git status -s)
+        sudo -u testbot bash -lc 'sudo apt-get install -y clang-format && make lint'
+        GEN_DIFF=$(git status -s --ignore-submodules)
 
         if [ -n "$GEN_DIFF" ]; then
             echo '"make lint" resulted in changes not in git' 1>&2
             git status
             exit 1
         fi
 
-    - name: make bufinstall buf setup setup-cpp-full-mod
-      uses: nick-fields/retry@v2
-      with:
-        timeout_minutes: 5
-        max_attempts: 3
-        retry_on: timeout
-        shell: bash
-        command: |
-          chown -R testbot:testbot .
-          sudo -u testbot bash -lc 'make bufinstall buf setup setup-cpp-full-mod'
+    - name: make setup
+      run: |
+        sudo -u testbot bash -lc 'make setup'
 
     - name: make build
       run: |
         sudo -u testbot bash -lc 'make build'
 
-    - name: make install-lua-files
+    - name: make install
       run: |
-        sudo -u testbot bash -lc 'make install-lua-files'
+        sudo -u testbot bash -lc 'make install'
 
     - name: make test
       run: |
         sudo -u testbot bash -lc 'make test'
 
-    - name: Copy carto_grpc_server binary
-      if: matrix.platform == 'linux/amd64'
-      run: |
-        sudo cp viam-cartographer/build/carto_grpc_server /usr/local/bin/carto_grpc_server
-
     - name: Run viam-cartographer cartographer integration tests
       if: matrix.platform == 'linux/amd64'
       run: |
-        sudo -u testbot bash -lc 'sudo go test -v -race -run TestCartographerIntegration'
+        sudo -u testbot bash -lc 'go test -v -race -run TestCartographerIntegration'
 
-    - name: make test-cpp-full-mod-valgrind
+    - name: make setup-cpp-full-mod test-cpp-full-mod-valgrind
       # Currently we only run valgrind on arm64 as x86
       # doesn't valgrind clean due to what appears to be a bug in GLOG
       # https://github.com/viamrobotics/viam-cartographer/actions/runs/5156238014/jobs/9287413446
       if: matrix.platform != 'linux/amd64'
       run: |
-        sudo -u testbot bash -lc 'make test-cpp-full-mod-valgrind'
+        sudo -u testbot bash -lc 'make setup-cpp-full-mod test-cpp-full-mod-valgrind'

Makefile

@@ -1,35 +1,43 @@
 BUILD_CHANNEL?=local
-TOOL_BIN = bin/gotools/$(shell uname -s)-$(shell uname -m)
-PATH_WITH_TOOLS="`pwd`/$(TOOL_BIN):$(HOME)/go/bin/:${PATH}"
-GIT_REVISION = $(shell git rev-parse HEAD | tr -d '\n')
-TAG_VERSION?=$(shell git tag --points-at | sort -Vr | head -n1)
-GO_BUILD_LDFLAGS = -ldflags "-X 'main.Version=${TAG_VERSION}' -X 'main.GitRevision=${GIT_REVISION}'"
+TOOL_BIN := $(shell pwd)/bin/tools/$(shell uname -s)-$(shell uname -m)
+GIT_REVISION := $(shell git rev-parse HEAD | tr -d '\n')
+TAG_VERSION ?= $(shell git tag --points-at | sort -Vr | head -n1)
+GO_BUILD_LDFLAGS := -ldflags "-X 'main.Version=${TAG_VERSION}' -X 'main.GitRevision=${GIT_REVISION}'"
+SHELL := /usr/bin/env bash
+export PATH := $(TOOL_BIN):$(PATH)
+export GOBIN := $(TOOL_BIN)
+
+ifneq (, $(shell which brew))
+	EXTRA_CMAKE_FLAGS := -DCMAKE_PREFIX_PATH=$(shell brew --prefix) -DQt5_DIR=$(shell brew --prefix qt5)/lib/cmake/Qt5
+	export PKG_CONFIG_PATH := $(shell brew --prefix openssl@3)/lib/pkgconfig
+endif
+
+default: build
 
-ARTIFACT="~/go/bin/artifact"
+artifact-pull: $(TOOL_BIN)/artifact
+	artifact pull
 
-artifact-pull:
-	PATH=${PATH_WITH_TOOLS} artifact pull
+$(TOOL_BIN)/artifact:
+	 go install go.viam.com/utils/artifact/cmd/artifact
 
-bufinstall:
-	sudo apt-get install -y protobuf-compiler-grpc libgrpc-dev libgrpc++-dev || brew install grpc openssl --quiet
+$(TOOL_BIN)/buf:
+	go install github.com/bufbuild/buf/cmd/buf@v1.8.0
 
-bufsetup:
-	GOBIN=`pwd`/grpc/bin go install github.com/bufbuild/buf/cmd/buf@v1.8.0
-	ln -sf `which grpc_cpp_plugin` grpc/bin/protoc-gen-grpc-cpp
+$(TOOL_BIN)/protoc-gen-grpc-cpp:
+	mkdir -p "$(TOOL_BIN)"
+	which grpc_cpp_plugin && ln -sf `which grpc_cpp_plugin` $(TOOL_BIN)/protoc-gen-grpc-cpp
 
-buf: bufsetup
-	PATH="${PATH}:`pwd`/grpc/bin" buf generate --template ./buf/buf.gen.yaml buf.build/viamrobotics/api
-	PATH="${PATH}:`pwd`/grpc/bin" buf generate --template ./buf/buf.grpc.gen.yaml buf.build/viamrobotics/api
-	PATH="${PATH}:`pwd`/grpc/bin" buf generate --template ./buf/buf.gen.yaml buf.build/googleapis/googleapis
+buf: $(TOOL_BIN)/buf $(TOOL_BIN)/protoc-gen-grpc-cpp
+	buf generate --template ./buf/buf.gen.yaml buf.build/viamrobotics/api
+	buf generate --template ./buf/buf.grpc.gen.yaml buf.build/viamrobotics/api
+	buf generate --template ./buf/buf.gen.yaml buf.build/googleapis/googleapis
 
 clean:
-	rm -rf grpc
-	rm -rf bin
-	rm -rf viam-cartographer/build
-	rm -rf viam-cartographer/cartographer/build
+	rm -rf grpc bin viam-cartographer/build
 
 clean-all:
 	git clean -fxd
+	cd viam-cartographer/cartographer && git checkout . && git clean -fxd
 
 ensure-submodule-initialized:
 	@if [ ! -d "viam-cartographer/cartographer/cartographer" ]; then \
@@ -38,21 +46,8 @@ ensure-submodule-initialized:
 	else \
 		echo "Submodule found successfully"; \
 	fi
-
-lint-setup-cpp:
-ifeq ("Darwin", "$(shell uname -s)")
-	brew install clang-format
-else
-	sudo apt-get install -y clang-format
-endif
-
-lint-setup-go:
-	GOBIN=`pwd`/$(TOOL_BIN) go install \
-		github.com/edaniels/golinters/cmd/combined \
-		github.com/golangci/golangci-lint/cmd/golangci-lint \
-		github.com/rhysd/actionlint/cmd/actionlint
-
-lint-setup: lint-setup-cpp lint-setup-go
+	grep -q viam-patched viam-cartographer/cartographer/CMakeLists.txt || \
+	(cd viam-cartographer/cartographer && git checkout . && git apply ../cartographer_patches/carto.patch)
 
 lint-cpp:
 	find . -type f -not -path \
@@ -63,42 +58,46 @@ lint-cpp:
 		-and \( -iname '*.h' -o -iname '*.cpp' -o -iname '*.cc' \) \
 		| xargs clang-format -i --style="{BasedOnStyle: Google, IndentWidth: 4}"
 
-lint-go:
+lint-go: $(TOOL_BIN)/combined $(TOOL_BIN)/golangci-lint $(TOOL_BIN)/actionlint
 	go vet -vettool=$(TOOL_BIN)/combined ./...
-	GOGC=50 $(TOOL_BIN)/golangci-lint run -v --fix --config=./etc/golangci.yaml
-	PATH=$(PATH_WITH_TOOLS) actionlint
+	GOGC=50 golangci-lint run -v --fix --config=./etc/golangci.yaml
+	actionlint
+
+$(TOOL_BIN)/combined $(TOOL_BIN)/golangci-lint $(TOOL_BIN)/actionlint:
+	go install \
+		github.com/edaniels/golinters/cmd/combined \
+		github.com/golangci/golangci-lint/cmd/golangci-lint \
+		github.com/rhysd/actionlint/cmd/actionlint
 
 lint: ensure-submodule-initialized lint-cpp lint-go
 
-setup: ensure-submodule-initialized
-ifeq ("Darwin", "$(shell uname -s)")
-	cd viam-cartographer/scripts && ./setup_cartographer_macos.sh
+setup: install-dependencies ensure-submodule-initialized artifact-pull
+
+install-dependencies:
+ifneq (, $(shell which brew))
+	brew update
+	brew install abseil boost ceres-solver protobuf ninja cairo googletest lua@5.3 pkg-config cmake go@1.20 grpc clang-format
+	brew link lua@5.3
+	brew install openssl@3 eigen gflags glog suite-sparse sphinx-doc pcl nlopt-static
+else ifneq (, $(shell which apt-get))
+	$(warning  "Installing cartographer external dependencies via APT.")
+	$(warning "Packages may be too old to work with this project.")
+	sudo apt-get update
+	sudo apt-get install -y cmake ninja-build libgmock-dev libboost-iostreams-dev liblua5.3-dev libcairo2-dev python3-sphinx libnlopt-dev \
+		libabsl-dev libceres-dev libprotobuf-dev protobuf-compiler protobuf-compiler-grpc libpcl-dev libgrpc-dev libgrpc++-dev clang-format
 else
-	cd viam-cartographer/scripts && ./setup_cartographer_linux.sh
-endif
-	@make artifact-pull
-
-build: build-module
-ifneq ($(wildcard viam-cartographer/cartographer/build/.),)
-	cd viam-cartographer && ./scripts/build_viam_cartographer.sh 
-else 
-	cd viam-cartographer && ./scripts/build_cartographer.sh && ./scripts/build_viam_cartographer.sh
+	$(error "Unsupported system. Only apt and brew currently supported.")
 endif
 
-build-debug: build-module
-ifneq ($(wildcard viam-cartographer/cartographer/build/.),)
-	cd viam-cartographer && ./scripts/build_viam_cartographer_debug.sh 
-else 
-	cd viam-cartographer && ./scripts/build_cartographer.sh && ./scripts/build_viam_cartographer_debug.sh
-endif
+build: ensure-submodule-initialized buf build-module
+	cd viam-cartographer && cmake -Bbuild -G Ninja ${EXTRA_CMAKE_FLAGS} && cmake --build build
+
+build-debug: EXTRA_CMAKE_FLAGS += -DCMAKE_BUILD_TYPE=Debug -DFORCE_DEBUG_BUILD=True
+build-debug: build
 
 build-module:
 	mkdir -p bin && go build $(GO_BUILD_LDFLAGS) -o bin/cartographer-module module/main.go
 
-install-lua-files:
-	sudo mkdir -p /usr/local/share/cartographer/lua_files/
-	sudo cp viam-cartographer/lua_files/* /usr/local/share/cartographer/lua_files/
-
 test-cpp:
 	viam-cartographer/build/unit_tests -p -l all
 
@@ -122,7 +121,11 @@ test-go:
 
 test: test-cpp test-go
 
-install:
+install-lua-files:
+	sudo mkdir -p /usr/local/share/cartographer/lua_files/
+	sudo cp viam-cartographer/lua_files/* /usr/local/share/cartographer/lua_files/
+
+install: install-lua-files
 	sudo rm -f /usr/local/bin/carto_grpc_server
 	sudo rm -f /usr/local/bin/cartographer-module
 	sudo cp viam-cartographer/build/carto_grpc_server /usr/local/bin/carto_grpc_server

README.md

@@ -84,15 +84,11 @@ These containers are set to persist between sessions via the `persistent` parame
 #### Setup, build, and run the binary
 
 ```bash
-# Setup the gRPC files
-make bufinstall && make buf 
-# Install dependencies
+# Install dependencies using brew (or apt-get)
 make setup
-# Build & install the binary
+# Build
 make build
-# Install lua files
-make install-lua-files
-# Install the cartographer-module binary
+# Install lua files and binaries to /usr/local
 make install
 # Run the binary
 cartographer-module

cartofacade/cgo_api.go

@@ -8,7 +8,7 @@ package cartofacade
 	#cgo CFLAGS: -I../viam-cartographer/src/carto_facade
 
 	// the libraries that need to be linked can be derived from line 258 of the build.ninja file that is autogenerated during make build
-	#cgo LDFLAGS: -L../viam-cartographer/build -L../viam-cartographer/cartographer/build -lviam-cartographer  -lcartographer -ldl -lm -labsl_hash  -labsl_city -labsl_bad_optional_access -labsl_strerror  -labsl_str_format_internal -labsl_synchronization -labsl_strings -labsl_throw_delegate -lcairo -llua5.3 -lstdc++ -lceres -lprotobuf -lglog -lboost_filesystem -lboost_iostreams -lpcl_io -lpcl_common -labsl_raw_hash_set
+	#cgo LDFLAGS: -L../viam-cartographer/build -L../viam-cartographer/build/cartographer -lviam-cartographer  -lcartographer -ldl -lm -labsl_hash  -labsl_city -labsl_bad_optional_access -labsl_strerror  -labsl_str_format_internal -labsl_synchronization -labsl_strings -labsl_throw_delegate -lcairo -llua5.3 -lstdc++ -lceres -lprotobuf -lglog -lboost_filesystem -lboost_iostreams -lpcl_io -lpcl_common -labsl_raw_hash_set
 
 	#include "../viam-cartographer/src/carto_facade/carto_facade.h"
 

etc/packaging/appimages/cartographer-module-aarch64.yml

@@ -7,13 +7,10 @@ script:
  - mkdir -p $TARGET_APPDIR/usr/share/cartographer/lua_files
  - cp ../../../bin/cartographer-module $TARGET_APPDIR/usr/bin/
  - cp ../../../viam-cartographer/build/carto_grpc_server $TARGET_APPDIR/usr/bin/
- - cp ../../../viam-cartographer/cartographer/build/libcartographer.a $TARGET_APPDIR/usr/lib/
- - cp ../../../viam-cartographer/build/libviam-cartographer.a $TARGET_APPDIR/usr/lib/
  - cp ../../../viam-cartographer/lua_files/* $TARGET_APPDIR/usr/share/cartographer/lua_files
  - mkdir -p $TARGET_APPDIR/usr/share/icons/viam/256x256/apps/
  - cp ./viam-server.png $TARGET_APPDIR/usr/share/icons/viam/256x256/apps/viam-server.png
- - go install github.com/Otterverse/aix@latest
- - cp `go env GOPATH`/bin/aix $TARGET_APPDIR/usr/bin/
+ - GOBIN=$TARGET_APPDIR/usr/bin/ go install github.com/Otterverse/aix@latest
  - chmod 755 $TARGET_APPDIR/usr/bin/*
 
 AppDir:

etc/packaging/appimages/cartographer-module-x86_64.yml

@@ -7,13 +7,10 @@ script:
  - mkdir -p $TARGET_APPDIR/usr/share/cartographer/lua_files
  - cp ../../../bin/cartographer-module $TARGET_APPDIR/usr/bin/
  - cp ../../../viam-cartographer/build/carto_grpc_server $TARGET_APPDIR/usr/bin/
- - cp ../../../viam-cartographer/cartographer/build/libcartographer.a $TARGET_APPDIR/usr/lib/
- - cp ../../../viam-cartographer/build/libviam-cartographer.a $TARGET_APPDIR/usr/lib/
  - cp ../../../viam-cartographer/lua_files/* $TARGET_APPDIR/usr/share/cartographer/lua_files
  - mkdir -p $TARGET_APPDIR/usr/share/icons/viam/256x256/apps/
  - cp ./viam-server.png $TARGET_APPDIR/usr/share/icons/viam/256x256/apps/viam-server.png
- - go install github.com/Otterverse/aix@latest
- - cp `go env GOPATH`/bin/aix $TARGET_APPDIR/usr/bin/
+ - GOBIN=$TARGET_APPDIR/usr/bin/ go install github.com/Otterverse/aix@latest
  - chmod 755 $TARGET_APPDIR/usr/bin/*
 
 AppDir: