oxidecomputer / cockroach

CockroachDB 22.1.x long-term maintenance branch
Other
3 stars 1 forks source link

Current revision of GEOS does not build on GCC 13 or later #3

Open iliana opened 3 days ago

iliana commented 3 days ago

... and moving GEOS to a version where GCC 13 is supported causes GEOS-related tests in Cockroach to fail with what look like behavior changes.

dspearson commented 3 days ago

Patch to apply to geos submodule:

From: Dominic Pearson <dsp@technoanimal.net>
Date: Sat, 19 Oct 2024 06:19:22 +0200
Subject: [PATCH] compat: add cstdint for gcc13 builds

---
 include/geos/geomgraph/TopologyLocation.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/geos/geomgraph/TopologyLocation.h b/include/geos/geomgraph/TopologyLocation.h
index 0bf0854d4..d02989382 100644
--- a/include/geos/geomgraph/TopologyLocation.h
+++ b/include/geos/geomgraph/TopologyLocation.h
@@ -28,6 +28,7 @@
 #include <vector>
 #include <array>
 #include <string>
+#include <cstdint>

 #ifdef _MSC_VER
 #pragma warning(push)
-- 
2.45.2

There is already discussion: https://github.com/libgeos/geos/commit/5fe03b63e2c41ac2922a9865b3d67c006433efe9 - not sure if it'll get backported though. For now I just did gsed -i '/\[submodule "c-deps\/geos"\]/,/\[submodule /{s|url = .*|url = https://github.com/dspearson/geos.git|}' .gitmodules && git update-index --cacheinfo 160000,09751cd5b5ca5cc04812305234204900d83d4190,c-deps/geos

I offer this workaround simply because I had the same experience during a late night weekend project, and was too stubborn to go to sleep.

dspearson commented 3 days ago

Just an addendum: I hit followup issues due to the outdated webpack etc. not playing well with more recent OpenSSL. Unfortunately the Makefile looks to be explicitly setting NODE_OPTIONS rather than extending anything from the calling environment. For sure there's a better solution, but rather than inevitably fighting against makefile syntax and multiple callpoints I just modified build/node-run.sh (which is called for all the invocations) like: NODE_OPTIONS="${NODE_OPTIONS:-} --openssl-legacy-provider" "$@" > >(cat) 2> >(cat >&2)

The final issue I encountered was again related to me having a too-recent toolchain. The goschedstats util has go version-specific runtime information encoded in files like runtime_go1.17.go, runtime_go1.19.go -- and of course the scheduler internals have changed in newer releases. The upstream copyright notice claims that their new revisions, while necessarily being lifted to match the go runtime structs, are now proprietary -- and so for the sake of future maintainability it might be worth throwing it out the window. Pretty sure go's runtime/metrics can be adopted to achieve the same functionality while also removing this maintenance burden.

Cheers!