fix: copy libsteam_api to build output and improve installation docs#27
fix: copy libsteam_api to build output and improve installation docs#27unhappychoice merged 1 commit intomainfrom
Conversation
- build.rs: auto-copy libsteam_api shared library to target/<profile>/ so it is found at runtime via $ORIGIN rpath - README: remove cargo install section (cannot bundle .so), add FAQ for libsteam_api.so error - install.sh: warn if shared library is missing after installation
📝 WalkthroughWalkthroughThe PR refactors library handling for Steam API shared objects: build.rs now includes dedicated functions to copy the Steam API library to the output directory and configure rpath for runtime resolution. Documentation is updated to explain library requirements, and the install script adds verification warnings for missing libraries. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@build.rs`:
- Around line 50-53: The current silent ignore of std::fs::copy hides failures;
replace the unused-result call with proper error handling: call
std::fs::copy(&lib_src, &lib_dst) and match its Result, and on Err emit a
build-time hint via println!("cargo:warning=...") including the error message
and the lib_src/lib_dst paths (reference symbols: lib_src, lib_dst,
std::fs::copy, and the cargo:warning mechanism) so users see why the copy failed
during cargo build.
🧹 Nitpick comments (2)
build.rs (2)
24-26: Doc comment mentionscargo installbut it won't actually help there.The copy targets
target/<profile>/, not~/.cargo/bin/, socargo installusers won't benefit from this logic. Since the README already droppedcargo installas an installation method, consider updating this comment to avoid confusion.Suggested wording
-/// Copy libsteam_api shared library next to the output binary so it can be found at runtime. -/// This handles `cargo build` and `cargo install` cases where the library would otherwise -/// only exist deep inside the build directory. +/// Copy libsteam_api shared library next to the output binary so it can be found at runtime. +/// Without this, the library only exists deep inside the build directory (target/build/.../out/).
56-56: Prefer&Pathover&PathBuffor function parameters.Clippy's
ptr_arglint flags this —&Pathis the idiomatic borrow type and avoids requiring callers to hold aPathBuf.Proposed fix
-fn find_steam_api_lib(build_dir: &PathBuf, lib_name: &str) -> Option<PathBuf> { +fn find_steam_api_lib(build_dir: &std::path::Path, lib_name: &str) -> Option<PathBuf> {
| let lib_dst = profile_dir.join(lib_name); | ||
| if lib_src != lib_dst { | ||
| let _ = std::fs::copy(&lib_src, &lib_dst); | ||
| } |
There was a problem hiding this comment.
Silent copy failure hides the exact problem this PR aims to fix.
If std::fs::copy fails (e.g., permissions, disk full), the build silently succeeds but the library is missing at runtime — the same user-facing error this PR addresses. Emit a cargo:warning so users get a build-time hint.
Proposed fix
let lib_dst = profile_dir.join(lib_name);
if lib_src != lib_dst {
- let _ = std::fs::copy(&lib_src, &lib_dst);
+ if let Err(e) = std::fs::copy(&lib_src, &lib_dst) {
+ println!(
+ "cargo:warning=Failed to copy {} to {}: {}",
+ lib_src.display(),
+ lib_dst.display(),
+ e
+ );
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let lib_dst = profile_dir.join(lib_name); | |
| if lib_src != lib_dst { | |
| let _ = std::fs::copy(&lib_src, &lib_dst); | |
| } | |
| let lib_dst = profile_dir.join(lib_name); | |
| if lib_src != lib_dst { | |
| if let Err(e) = std::fs::copy(&lib_src, &lib_dst) { | |
| println!( | |
| "cargo:warning=Failed to copy {} to {}: {}", | |
| lib_src.display(), | |
| lib_dst.display(), | |
| e | |
| ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@build.rs` around lines 50 - 53, The current silent ignore of std::fs::copy
hides failures; replace the unused-result call with proper error handling: call
std::fs::copy(&lib_src, &lib_dst) and match its Result, and on Err emit a
build-time hint via println!("cargo:warning=...") including the error message
and the lib_src/lib_dst paths (reference symbols: lib_src, lib_dst,
std::fs::copy, and the cargo:warning mechanism) so users see why the copy failed
during cargo build.
1 participant
Summary
Fixes the
libsteam_api.so: cannot open shared object fileerror reported by users (e.g., on Fedora 43).Changes
libsteam_api.so/.dylib/.dllfromsteamworks-sysbuild output totarget/<profile>/so it sits next to the binary and is resolved by$ORIGINrpathcargo installinstallation method (cargo installonly copies the binary, not the shared library, and there is no way to fix this within Cargo's constraints). Add FAQ entry for thelibsteam_api.soerrorBackground
steamfetchdynamically links againstlibsteam_api.so(bundled bysteamworks-sys). The existing$ORIGINrpath means the library must be in the same directory as the binary. The install script and Homebrew handle this, butcargo buildleft the.sodeep insidetarget/build/steamworks-sys-*/out/— not next to the binary.cargo installis worse: it only copies the binary to~/.cargo/bin/, and there is no mechanism in Cargo to install additional files.Summary by CodeRabbit
Documentation
Improvements