Skip to content

realsense: tell the linker where nix actually put librealsense2 - #4054

Merged
spomichter merged 1 commit into
mainfrom
ivan/fix/realsense-link-search
Sep 10, 2026
Merged

spomichter merged 1 commit into
mainfrom
ivan/fix/realsense-link-search

Conversation

@leshy

@leshy leshy commented Sep 10, 2026

Copy link
Copy Markdown
Member

dimos run of anything with a RealSenseCamera fails its native build with

rust-lld: error: unable to find library -lrealsense2

nixpkgs' realsense2.pc names lib/x86_64-linux-gnu but the .so is in lib/.
build.rs already walked up to the real directory for the rpath; now it
emits it as a link-search path too, so `-lrealsense2` resolves and the
native module builds from a fresh nix store. Cargo.lock refreshed against
the dimos-module crate it points at.
@greptile-apps

greptile-apps Bot commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

Safe to merge: the native build script selects the directory containing librealsense2 and retains its existing fallback behavior.

What we checked:

  • Compared the prior Nix-style directory handling with the updated ancestor-directory lookup to verify how the script resolves linker search paths. T-Rex
  • Confirmed that when librealsense2.so exists in /lib, the updated logic emits /lib as both the native linker search path and the runtime rpath. T-Rex
  • Confirmed that when no ancestor contains the library, the script retains the supplied pkg-config path as its fallback. T-Rex
  • Compared the pre-change behavior to the post-change behavior: the library-containing /lib is emitted for both directives and the missing-library fallback remained unchanged. T-Rex

Summary

  • Updates the standalone RealSense build script so Cargo links against the directory that actually contains librealsense2.so when pkg-config reports a Nix-style subdirectory.
  • Keeps the reported pkg-config directory as a fallback when the library cannot be located in an ancestor directory.
  • Refreshes the standalone RealSense lockfile for the local dimos-module dependency.

No merge-blocking issues found.

Reviews (1) · Last reviewed commit: "realsense: tell the linker where nix act..."

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@            Coverage Diff             @@
##             main    #4054      +/-   ##
==========================================
- Coverage   78.22%   78.20%   -0.02%     
==========================================
  Files        1431     1428       -3     
  Lines      135204   135077     -127     
  Branches    11711    11708       -3     
==========================================
- Hits       105762   105643     -119     
+ Misses      26167    26112      -55     
- Partials     3275     3322      +47     
Components Coverage Δ
Tests 94.71% <ø> (+0.05%) ⬆️
Flag Coverage Δ
OS-ubuntu-24.04-arm 74.53% <ø> (-0.01%) ⬇️
OS-ubuntu-latest 75.10% <ø> (-0.01%) ⬇️
Py-3.10 75.10% <ø> (-0.01%) ⬇️
Py-3.11 75.09% <ø> (-0.01%) ⬇️
Py-3.12 75.10% <ø> (+<0.01%) ⬆️
Py-3.13 75.09% <ø> (-0.01%) ⬇️
Py-3.14 75.10% <ø> (-0.01%) ⬇️
Py-3.14t 75.10% <ø> (+<0.01%) ⬆️
SelfHosted-Large 30.76% <ø> (-0.04%) ⬇️
SelfHosted-Linux 35.57% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Sep 10, 2026
@spomichter
spomichter added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit 8b8b57e Sep 10, 2026
32 of 33 checks passed
@spomichter
spomichter deleted the ivan/fix/realsense-link-search branch September 10, 2026 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants