fix(build): export dynamic symbols from the Lua hosting binaries - #7710
Merged
Conversation
Lua C modules pulled in by a profile's require() are dlopen'ed at runtime and resolve their lua_* symbols against the hosting executable. Since the vcpkg migration in #7487 Lua is linked statically, so those symbols are absent from the dynamic symbol table and loading fails with "undefined symbol: lua_gettop". CMake only passes the platform export flag, -rdynamic on ELF toolchains, for targets marked ENABLE_EXPORTS once CMP0065 is NEW, which it is here via cmake_minimum_required(VERSION 3.18). Set the property on osrm-extract and osrm-contract, the only two binaries that embed Lua. This replaces the CMAKE_EXE_LINKER_FLAGS="-rdynamic" workaround from the bug report with the mechanism CMake documents for executables that load plugins. Guarded with NOT MSVC, where ENABLE_EXPORTS instead produces an import library and is not needed. Refs #7700 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0152BLSwmtKX4pQmiLpd8wKN
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7710 +/- ##
==========================================
+ Coverage 94.78% 94.79% +0.01%
==========================================
Files 526 526
Lines 42085 42085
==========================================
+ Hits 39891 39896 +5
+ Misses 2194 2189 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the first half of #7700.
Problem
Lua C modules pulled in by a profile's
require()aredlopen'ed at runtime and resolve theirlua_*symbols against the hosting executable. Since the vcpkg migration in #7487 Lua comes from vcpkg and thex64-linuxtriplet links it statically, so those symbols never reach the dynamic symbol table:Before #7487
ENABLE_CONANdefaulted to OFF and the build used the system Lua, a sharedliblua, so adlopen'ed module resolved these symbols from the already loaded.so. This is a regression.Fix
CMake historically added
-rdynamicto every executable on ELF platforms. Under CMP0065, NEW here because ofcmake_minimum_required(VERSION 3.18), it does so only for targets withENABLE_EXPORTS:So the property is set on
osrm-extractandosrm-contract, the only two binaries linking${LUA_LIBRARIES}. This is the documented mechanism for executables that load plugins viadlopen, and replaces the reporter's-DCMAKE_EXE_LINKER_FLAGS="-rdynamic"workaround.Guarded with
NOT MSVC, whereENABLE_EXPORTSinstead produces an import library and is not needed.Verification
No Linux machine was available here, so the policy behaviour was confirmed directly with a minimal project pinned to the same
cmake_minimum_required(VERSION 3.18), using a sentinel in place of the platform export flag, which is empty on Apple:ENABLE_EXPORTS ONLocally
osrm-extractandosrm-contractconfigure, link, and run. Worth a check on Linux against the reporter'slua-rediscase before merging.Not covered
The second half of #7700, system Lua module paths, is unaddressed. vcpkg builds Lua with upstream's
LUA_ROOTof/usr/local/, so Debian's/usr/share/lua/5.5is never searched. Separately,luaAddScriptFolderToLoadPath(include/util/lua_util.hpp:25) extendspackage.pathbut neverpackage.cpath, so profile local C modules cannot be loaded on any platform. Both need a portability decision and are left for a follow up.🤖 Generated with Claude Code