Skip to content

Commit 19ae2be

Browse files
ekumphoolioh
andauthored
build(macOS): set the LC_ID_DYLIB for mac binaries to set correct name for linking (#1646)
# What does this PR do? for macOS builds we were not setting `LC_ID_DYLIB`, so the absolute path at build time was being used, which causes problems when you link against the dylib on another machine. This PR should correctly set `LC_ID_DYLIB` for macOS. Something similar was done for linux in #1258 The post-build fix logic has been removed since it doesn't seem to be used anywhere else anymore. # Motivation What inspired you to submit this pull request? # Additional Notes Anything else we should know when reviewing? # How to test the change? `cargo run --bin release --release -- --out /tmp/libdatadog-output` Prior to this change the output would have looked like: ``` cmd LC_ID_DYLIB cmdsize 184 name /Users/ec2-user/builds/.../libdatadog_profiling_ffi.dylib (offset 24) ``` With this change it now looks like: ``` cmd LC_ID_DYLIB cmdsize 64 name @rpath/libdatadog_profiling.dylib (offset 24) ``` Co-authored-by: hoolioh <107922352+hoolioh@users.noreply.github.com> Co-authored-by: julio.gonzalez <julio.gonzalez@datadoghq.com>
1 parent 277094c commit 19ae2be

6 files changed

Lines changed: 6 additions & 68 deletions

File tree

builder/src/arch/apple.rs

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
use std::ffi::OsStr;
5-
use std::os::unix::process::ExitStatusExt;
65
use std::process::Command;
76

87
use crate::utils::wait_for_success;
@@ -13,34 +12,13 @@ pub const PROF_DYNAMIC_LIB: &str = "libdatadog_profiling.dylib";
1312
pub const PROF_STATIC_LIB: &str = "libdatadog_profiling.a";
1413
pub const PROF_DYNAMIC_LIB_FFI: &str = "libdatadog_profiling_ffi.dylib";
1514
pub const PROF_STATIC_LIB_FFI: &str = "libdatadog_profiling_ffi.a";
16-
pub const REMOVE_RPATH: bool = true;
1715
pub const BUILD_CRASHTRACKER: bool = true;
18-
pub const RUSTFLAGS: [&str; 2] = ["-C", "relocation-model=pic"];
19-
20-
pub fn fix_rpath(lib_path: &str) {
21-
if REMOVE_RPATH {
22-
let lib_name = lib_path.split('/').next_back().unwrap();
23-
24-
let exit_status = Command::new("install_name_tool")
25-
.arg("-id")
26-
.arg("@rpath/".to_string() + lib_name)
27-
.arg(lib_path)
28-
.status()
29-
.expect("Failed to fix rpath using install_name_tool");
30-
match exit_status.code() {
31-
Some(0) => {}
32-
Some(rc) => panic!("Failed to fix rpath using install_name_tool: return code {rc}"),
33-
None => match exit_status.signal() {
34-
Some(sig) => {
35-
panic!("Failed to fix rpath using install_name_tool: killed by signal {sig}")
36-
}
37-
None => panic!(
38-
"Failed to fix rpath using install_name_tool: exit status {exit_status:?}"
39-
),
40-
},
41-
}
42-
}
43-
}
16+
pub const RUSTFLAGS: [&str; 4] = [
17+
"-C",
18+
"relocation-model=pic",
19+
"-C",
20+
"link-arg=-Wl,-install_name,@rpath/libdatadog_profiling.dylib",
21+
];
4422

4523
pub fn strip_libraries(lib_path: &str) {
4624
// objcopy is not available in macos image. Investigate llvm-objcopy

builder/src/arch/linux.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ pub const PROF_DYNAMIC_LIB: &str = "libdatadog_profiling.so";
1111
pub const PROF_STATIC_LIB: &str = "libdatadog_profiling.a";
1212
pub const PROF_DYNAMIC_LIB_FFI: &str = "libdatadog_profiling_ffi.so";
1313
pub const PROF_STATIC_LIB_FFI: &str = "libdatadog_profiling_ffi.a";
14-
pub const REMOVE_RPATH: bool = false;
1514
pub const BUILD_CRASHTRACKER: bool = true;
1615
pub const RUSTFLAGS: [&str; 4] = [
1716
"-C",
@@ -20,18 +19,6 @@ pub const RUSTFLAGS: [&str; 4] = [
2019
"link-arg=-Wl,-soname,libdatadog_profiling.so",
2120
];
2221

23-
pub fn fix_rpath(lib_path: &str) {
24-
if REMOVE_RPATH {
25-
let patchelf = Command::new("patchelf")
26-
.arg("--remove-rpath")
27-
.arg(lib_path)
28-
.spawn()
29-
.expect("failed to spawn patchelf");
30-
31-
wait_for_success(patchelf, "patchelf");
32-
}
33-
}
34-
3522
pub fn strip_libraries(lib_path: &str) {
3623
let rm_section = Command::new("objcopy")
3724
.arg("--remove-section")

builder/src/arch/musl.rs

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ pub const PROF_DYNAMIC_LIB: &str = "libdatadog_profiling.so";
1111
pub const PROF_STATIC_LIB: &str = "libdatadog_profiling.a";
1212
pub const PROF_DYNAMIC_LIB_FFI: &str = "libdatadog_profiling_ffi.so";
1313
pub const PROF_STATIC_LIB_FFI: &str = "libdatadog_profiling_ffi.a";
14-
pub const REMOVE_RPATH: bool = false;
1514
pub const BUILD_CRASHTRACKER: bool = true;
1615
pub const RUSTFLAGS: [&str; 4] = [
1716
"-C",
@@ -20,16 +19,6 @@ pub const RUSTFLAGS: [&str; 4] = [
2019
"link-arg=-Wl,-soname,libdatadog_profiling.so",
2120
];
2221

23-
pub fn fix_rpath(lib_path: &str) {
24-
if REMOVE_RPATH {
25-
Command::new("patchelf")
26-
.arg("--remove-rpath")
27-
.arg(lib_path)
28-
.spawn()
29-
.expect("failed to remove rpath");
30-
}
31-
}
32-
3322
pub fn strip_libraries(lib_path: &str) {
3423
let rm_section = Command::new("objcopy")
3524
.arg("--remove-section")

builder/src/arch/windows.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ pub const PROF_PDB: &str = "datadog_profiling.pdb";
1212
pub const PROF_DYNAMIC_LIB_FFI: &str = "datadog_profiling_ffi.dll";
1313
pub const PROF_STATIC_LIB_FFI: &str = "datadog_profiling_ffi.lib";
1414
pub const PROF_PDB_FFI: &str = "datadog_profiling_ffi.pdb";
15-
pub const REMOVE_RPATH: bool = false;
1615
pub const BUILD_CRASHTRACKER: bool = false;
1716
pub const RUSTFLAGS: [&str; 4] = [
1817
"-C",
@@ -21,7 +20,6 @@ pub const RUSTFLAGS: [&str; 4] = [
2120
"target-feature=+crt-static",
2221
];
2322

24-
pub fn fix_rpath(_lib_path: &str) {}
2523
pub fn strip_libraries(_lib_path: &str) {}
2624

2725
pub fn add_additional_files(lib_path: &str, target_path: &OsStr) {

builder/src/bin/release.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,6 @@ pub fn main() {
124124
let res = builder.build();
125125
match res {
126126
Ok(_) => {
127-
builder.sanitize_libraries();
128127
// Note: tar creation is handled by CI, not by this binary
129128
}
130129
Err(err) => panic!("{}", format!("Building failed: {err}")),

builder/src/builder.rs

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -123,19 +123,6 @@ impl Builder {
123123
.expect("Failed to create include directory");
124124
}
125125

126-
// TODO: maybe do this in module's build.rs
127-
pub fn sanitize_libraries(&self) {
128-
let datadog_lib_dir = Path::new(self.source_lib.as_ref());
129-
130-
let libs = fs::read_dir(datadog_lib_dir).unwrap();
131-
for lib in libs.flatten() {
132-
let name = lib.file_name().into_string().unwrap();
133-
if name.ends_with(".so") {
134-
arch::fix_rpath(lib.path().to_str().unwrap());
135-
}
136-
}
137-
}
138-
139126
pub fn add_cmake(&self) {
140127
let libs = arch::NATIVE_LIBS.to_owned();
141128
let cmake_dir: PathBuf = [&self.target_dir, "cmake"].iter().collect();

0 commit comments

Comments
 (0)