Skip to content

Commit

Permalink
use statically linked rocksdb on linux/mac, dll on windows (#2291)
Browse files Browse the repository at this point in the history
The `rocksdb` version shipped with distributions is typically old and
therefore often lacks features we use - it also doesn't match the one
assumed by nim-rocksdb leading to ABI mismatch risks.

Instead of depending on the system rocksdb, we'll now use the rocksdb
version assumed by nim-rocksdb and locked in its vendor folder by always
building it together with nimbus.

This avoids the problem of unknown rocksdb versions at a (small) cost to
build time.

CI caching and full windows support for building from source [remains
TODO](status-im/nim-rocksdb#44).
  • Loading branch information
arnetheduck committed Jun 4, 2024
1 parent 69a1588 commit 95a4adc
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 76 deletions.
34 changes: 1 addition & 33 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,29 +115,6 @@ jobs:
HOMEBREW_NO_INSTALL_CLEANUP=1 brew install gnu-getopt
brew link --force gnu-getopt
- name: Restore rocksdb from cache (Macos/Linux)
if: runner.os != 'Windows'
id: rocksdb-cache
uses: actions/cache@v4
with:
path: rocks-db-cache-${{ matrix.target.cpu }}
key: 'rocksdb-v2-${{ matrix.target.os }}-${{ matrix.target.cpu }}'

- name: Install rocksdb (Linux amd64)
# mysterious illegal instruction error if we build our own librocksdb
if: runner.os == 'Linux' && matrix.target.cpu == 'amd64'
run: |
sudo apt-get -q update
sudo apt-get install -y librocksdb-dev libpcre3-dev
- name: Build and install rocksdb (Macos)
if: runner.os == 'Macos'
run: |
HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1 brew install ccache
echo "/usr/local/opt/ccache/libexec" >> ${GITHUB_PATH}
curl -O -L -s -S https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_rocksdb.sh
bash build_rocksdb.sh rocks-db-cache-${{ matrix.target.cpu }}
- name: Restore llvm-mingw (Windows) from cache
if: runner.os == 'Windows'
id: windows-mingw-cache
Expand All @@ -154,7 +131,7 @@ jobs:
path: external/dlls-${{ matrix.target.cpu }}
# according to docu, idle caches are kept for up to 7 days
# so change dlls# to force new cache contents (for some number #)
key: dlls0-${{ matrix.target.cpu }}
key: dlls1-${{ matrix.target.cpu }}

- name: Install llvm-mingw dependency (Windows)
if: >
Expand All @@ -179,19 +156,10 @@ jobs:
steps.windows-dlls-cache.outputs.cache-hit != 'true' &&
runner.os == 'Windows'
run: |
if [[ '${{ matrix.target.cpu }}' == 'amd64' ]]; then
ROCKSDBSUB=x64
else
ROCKSDBSUB=x86
fi
DLLPATH=external/dlls-${{ matrix.target.cpu }}
mkdir -p external
curl -L "https://nim-lang.org/download/windeps.zip" -o external/windeps.zip
7z x -y external/windeps.zip -o"$DLLPATH"
# ROCKSDB
curl -L "https://github.com/status-im/nimbus-deps/releases/download/nimbus-deps/nimbus-deps.zip" -o external/nimbus-deps.zip
7z x -y external/nimbus-deps.zip
cp "./$ROCKSDBSUB/librocksdb.dll" "$DLLPATH/librocksdb.dll"
- name: Path to cached dependencies (Windows)
if: >
Expand Down
22 changes: 1 addition & 21 deletions .github/workflows/simulators.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ jobs:
id: versions
run: |
sudo apt-get -q update
sudo apt-get install -y librocksdb-dev libpcre3-dev
getHash() {
git ls-remote "https://github.com/$1" "${2:-HEAD}" | cut -f 1
}
Expand Down Expand Up @@ -66,20 +65,6 @@ jobs:
- name: Checkout code
uses: actions/checkout@v4

- name: Restore rocksdb from cache
id: rocksdb-cache
uses: actions/cache@v4
with:
path: rocks-db-cache
key: 'rocksdb-v2'

- name: Build and install rocksdb
run: |
HOMEBREW_NO_AUTO_UPDATE=1 HOMEBREW_NO_INSTALL_CLEANUP=1 brew install ccache
echo "/usr/local/opt/ccache/libexec" >> ${GITHUB_PATH}
curl -O -L -s -S https://raw.githubusercontent.com/status-im/nimbus-build-system/master/scripts/build_rocksdb.sh
bash build_rocksdb.sh rocks-db-cache
- name: Get latest nimbus-build-system commit hash
id: versions
run: |
Expand Down Expand Up @@ -131,20 +116,15 @@ jobs:
path: external/dlls
# according to docu, idle caches are kept for up to 7 days
# so change dlls# to force new cache contents (for some number #)
key: dlls0
key: dlls1

- name: Install DLLs dependencies
if: steps.windows-dlls-cache.outputs.cache-hit != 'true'
run: |
ROCKSDBSUB=x64
DLLPATH=external/dlls
mkdir -p external
curl -L "https://nim-lang.org/download/windeps.zip" -o external/windeps.zip
7z x -y external/windeps.zip -o"$DLLPATH"
# ROCKSDB
curl -L "https://github.com/status-im/nimbus-deps/releases/download/nimbus-deps/nimbus-deps.zip" -o external/nimbus-deps.zip
7z x -y external/nimbus-deps.zip
cp "./$ROCKSDBSUB/librocksdb.dll" "$DLLPATH/librocksdb.dll"
- name: Restore llvm-mingw from cache
id: windows-mingw-cache
Expand Down
28 changes: 16 additions & 12 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -203,21 +203,16 @@ update-from-ci: | sanity-checks update-test
+ "$(MAKE)" --no-print-directory deps-common

# builds the tools, wherever they are
$(TOOLS): | build deps
$(TOOLS): | build deps rocksdb
for D in $(TOOLS_DIRS); do [ -e "$${D}/$@.nim" ] && TOOL_DIR="$${D}" && break; done && \
echo -e $(BUILD_MSG) "build/$@" && \
$(ENV_SCRIPT) nim c $(NIM_PARAMS) -o:build/$@ "$${TOOL_DIR}/$@.nim"

# a phony target, because teaching `make` how to do conditional recompilation of Nim projects is too complicated
nimbus: | build deps
nimbus: | build deps rocksdb
echo -e $(BUILD_MSG) "build/$@" && \
$(ENV_SCRIPT) nim c $(NIM_PARAMS) -d:chronicles_log_level=TRACE -o:build/$@ "nimbus/$@.nim"

nimbus_rocksdb_static: | build deps rocksdb_static_deps
echo -e $(BUILD_MSG) "build/$@" && \
$(ENV_SCRIPT) nim c $(NIM_PARAMS) -d:enable_rocksdb_static_linking -d:chronicles_log_level=TRACE \
-o:build/$@ "nimbus/nimbus.nim"

# symlink
nimbus.nims:
ln -s nimbus.nimble $@
Expand All @@ -226,17 +221,26 @@ nimbus.nims:
libbacktrace:
+ $(MAKE) -C vendor/nim-libbacktrace --no-print-directory BUILD_CXX_LIB=0

# nim-rocksdb static dependencies
rocksdb_static_deps:
# nim-rocksdb

ifneq ($(USE_SYSTEM_ROCKSDB), 0)
ifeq ($(OS), Windows_NT)
rocksdb: fetch-dlls
else
rocksdb:
+ vendor/nim-rocksdb/scripts/build_static_deps.sh
endif
else
rocksdb:
endif

# builds and runs the nimbus test suite
test: | build deps
test: | build deps rocksdb
$(ENV_SCRIPT) nim test_rocksdb $(NIM_PARAMS) nimbus.nims
$(ENV_SCRIPT) nim test $(NIM_PARAMS) nimbus.nims

# builds and runs an EVM-related subset of the nimbus test suite
test-evm: | build deps
test-evm: | build deps rocksdb
$(ENV_SCRIPT) nim test_evm $(NIM_PARAMS) nimbus.nims

# Primitive reproducibility test.
Expand Down Expand Up @@ -334,7 +338,7 @@ t8n_test: | build deps t8n
$(ENV_SCRIPT) nim c -r $(NIM_PARAMS) -d:chronicles_default_output_device=stderr "tools/t8n/$@.nim"

# builds evm state test tool
evmstate: | build deps
evmstate: | build deps rocksdb
$(ENV_SCRIPT) nim c $(NIM_PARAMS) "tools/evmstate/$@.nim"

# builds and runs evm state tool test suite
Expand Down
12 changes: 6 additions & 6 deletions config.nims
Original file line number Diff line number Diff line change
Expand Up @@ -156,12 +156,9 @@ switch("warning", "LockLevel:off")
# disable nim-kzg's blst
switch("define", "kzgExternalBlst")

# RocksDB static linking is disabled by default
when defined(enable_rocksdb_static_linking):

when defined(windows):
{.fatal: "RocksDB static linking is not supported on Windows".}

# We lock down rocksdb to a particular version
# TODO self-build rocksdb dll on windows
when not defined(use_system_rocksdb) and not defined(windows):
switch("define", "rocksdb_static_linking")

# use the C++ linker profile because it's a C++ library
Expand All @@ -173,3 +170,6 @@ when defined(enable_rocksdb_static_linking):
switch("dynlibOverride", "rocksdb")
switch("dynlibOverride", "lz4")
switch("dynlibOverride", "zstd")

when defined(gcc):
switch("passc", "-Wno-error=incompatible-pointer-types")
1 change: 0 additions & 1 deletion nimbus/db/aristo/aristo_init/rocks_db.nim
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
## ...
##
{.push raises: [].}
{.warning: "*** importing rocks DB which needs a linker library".}

import
eth/common,
Expand Down
1 change: 0 additions & 1 deletion nimbus/db/kvt/kvt_init/rocks_db.nim
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
## ...
##
{.push raises: [].}
{.warning: "*** importing rocks DB which needs a linker library".}

import
eth/common,
Expand Down

0 comments on commit 95a4adc

Please sign in to comment.