rust-pcap / pcap

Rust language pcap library
Apache License 2.0
620 stars 144 forks source link

Feature flag to link statically with libpcap.a #195

Closed snir closed 2 years ago

snir commented 2 years ago

Fixes #183

Stargateur commented 2 years ago

oh nice, I don't have a lot of knowledge about linking but it's should Closes https://github.com/ebfull/pcap/issues/183 I guess. That say I think the feature should be rename to the openssl style, I believe it's more classic than static feature

snir commented 2 years ago

I think "static" is more accurate. Using "vendored" might be a bit misleading since it's used in Openssl to build and statically link openssl, while in this case we're simply statically linking against an existing libpcap.

stappersg commented 2 years ago

On Sun, Dec 12, 2021 at 05:47:32AM -0800, Antoine wrote:

... That say I think the feature should ..., I believe it's more ...

Please convert that into a merge request. Don't worry about how poor the merge request is, it is anyway better as "think" and "believe".

(merge request? Yes merge request, as in git merge. )

Wojtek242 commented 2 years ago

@snir - can you confirm that you tested this locally? If so, looks good to merge once I resolve the issue I mention below.

I just noticed I need to fix a clippy warning on master. Once I do that I will ping you to rebase to see if all the CI checks pass on this branch.

snir commented 2 years ago

Tested it locally - you can test with cargo build --features static --examples and see libpcap.so is removed from ldd examples/... Only slight issue is you need to set LIBPCAP_LIBDIR to build. I contemplated setting rustc-link-search=/usr/lib/x86_64-linux-gnu/ in build.rs if LIBPCAP_LIBDIR is empty but it seems a bit unidiomatic.

Stargateur commented 2 years ago

we should probably do some work on build.rs using pkg-config in another PR.

Wojtek242 commented 2 years ago

@snir Apologies for the delay, Christmas got in the way :-). Anyway, all clippy issues are resolved on master so please rebase so that the tests can run on this PR. If you could also update the CHANGELONG and the README about this change, that would be appreciated.

n-determinate commented 2 years ago

Looks like this is ready to have ci/workflows run again @Wojtek242 :)

Wojtek242 commented 2 years ago

Hi @snir! Sorry for coming back to this so late! I was trying your changes in a local container (since I wanted to make sure it all worked with the latest CI fixes on master) and I can't get it to compile against libpcap.a. I get

root@2942281523a1:/opt/pcap# LIBPCAP_LIBDIR=/opt/libpcap cargo build --features static --examples
   Compiling memchr v2.4.1
   Compiling libc v0.2.123
   Compiling cfg-if v1.0.0
   Compiling regex-syntax v0.6.25
   Compiling remove_dir_all v0.5.3
   Compiling libloading v0.6.7
   Compiling aho-corasick v0.7.18
   Compiling rand v0.4.6
   Compiling errno v0.2.8
   Compiling tempdir v0.3.7
   Compiling regex v1.5.5
   Compiling pcap v0.9.1 (/opt/pcap)
error: failed to run custom build command for `pcap v0.9.1 (/opt/pcap)`

Caused by:
  process didn't exit successfully: `/opt/pcap/target/debug/build/pcap-6a9a9fc8e6314d3a/build-script-build` (exit status: 101)
  --- stdout
  cargo:rerun-if-env-changed=LIBPCAP_LIBDIR
  cargo:rerun-if-env-changed=LIBPCAP_VER
  cargo:rustc-link-search=native=/opt/libpcap

  --- stderr
  thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: DlOpen { desc: "/opt/libpcap/libpcap.so: cannot open shared object file: No such file or directory" }', build.rs:140:52
  note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Am I forgetting something?

Wojtek242 commented 2 years ago

The directory /opt/libpcap contains a freshly compiled libpcap.a:

root@2942281523a1:/opt/pcap# ls -lha ../libpcap/libpcap.*
-rw-r--r--. 1 root root 2.5M Apr 14 21:48 ../libpcap/libpcap.a
-rw-r--r--. 1 root root  487 Apr 14 21:48 ../libpcap/libpcap.pc
-rw-r--r--. 1 root root  484 Apr 14 21:46 ../libpcap/libpcap.pc.in
-rwxr-xr-x. 1 root root 1.2M Apr 14 21:48 ../libpcap/libpcap.so.1.11.0-PRE-GIT
Wojtek242 commented 2 years ago

Ok, so first - we shouldn't assume libpcap.so is there which build.rs will unless LIBPCAP_VER is defined. Once I set LIBPCAP_VER I get:

  = note: /usr/bin/ld: /opt/pcap/target/debug/deps/libpcap-43746a11895c662d.rlib(pcap-dbus.o): in function `dbus_write':
          (.text+0x103): undefined reference to `dbus_message_demarshal'
          /usr/bin/ld: (.text+0x119): undefined reference to `dbus_connection_send'
          /usr/bin/ld: (.text+0x122): undefined reference to `dbus_connection_flush'
          /usr/bin/ld: (.text+0x12a): undefined reference to `dbus_message_unref'
          /usr/bin/ld: (.text+0x178): undefined reference to `dbus_error_free'
          /usr/bin/ld: /opt/pcap/target/debug/deps/libpcap-43746a11895c662d.rlib(pcap-dbus.o): in function `dbus_read':          (.text+0x1c3): undefined reference to `dbus_connection_pop_message'
          /usr/bin/ld: (.text+0x1e1): undefined reference to `dbus_connection_pop_message'
          /usr/bin/ld: (.text+0x1f6): undefined reference to `dbus_connection_read_write'
          /usr/bin/ld: (.text+0x262): undefined reference to `dbus_message_is_signal'
          /usr/bin/ld: (.text+0x27f): undefined reference to `dbus_message_marshal'
          /usr/bin/ld: (.text+0x2e3): undefined reference to `dbus_free'
          /usr/bin/ld: /opt/pcap/target/debug/deps/libpcap-43746a11895c662d.rlib(pcap-dbus.o): in function `dbus_cleanup':
          (.text+0x350): undefined reference to `dbus_connection_unref'
          /usr/bin/ld: /opt/pcap/target/debug/deps/libpcap-43746a11895c662d.rlib(pcap-dbus.o): in function `dbus_activate':
          (.text+0x3fa): undefined reference to `dbus_connection_open'
          /usr/bin/ld: (.text+0x412): undefined reference to `dbus_bus_register'
          /usr/bin/ld: (.text+0x505): undefined reference to `dbus_bus_add_match'
          /usr/bin/ld: (.text+0x50d): undefined reference to `dbus_error_is_set'
          /usr/bin/ld: (.text+0x53b): undefined reference to `dbus_bus_get'
          /usr/bin/ld: (.text+0x56c): undefined reference to `dbus_error_free'
          /usr/bin/ld: (.text+0x57c): undefined reference to `dbus_bus_add_match'
          /usr/bin/ld: (.text+0x584): undefined reference to `dbus_error_is_set'
          /usr/bin/ld: (.text+0x5b9): undefined reference to `dbus_error_free'
          /usr/bin/ld: (.text+0x5c5): undefined reference to `dbus_connection_unref'
          /usr/bin/ld: (.text+0x61e): undefined reference to `dbus_bus_get'
          /usr/bin/ld: (.text+0x65a): undefined reference to `dbus_error_free'
          /usr/bin/ld: (.text+0x675): undefined reference to `dbus_connection_set_max_received_size'
          /usr/bin/ld: (.text+0x686): undefined reference to `dbus_connection_unref'
          /usr/bin/ld: (.text+0x704): undefined reference to `dbus_error_free'
          collect2: error: ld returned 1 exit status

Seems like some issues with linking against dbus. I guess it's trying to link libpcap.a to a libdbus, but can't find it. I do have libdbus-1.a on my system though.

Wojtek242 commented 2 years ago

I tried a few more things with no success :(. I added , link(name = "dbus-1", kind = "static") next to the line about static pcap linking and it just complained about systemd. So instead I commented out the entire rfmon block to see if I can get it to compile, but no dice - it then can't even seem to link against pcap_close... at which point I gave up.

If anyone can reproduce a successful compilation against a static libpcap.a in an Ubuntu docker container I'd be grateful.

Wojtek242 commented 2 years ago

I read up a bit on linking and I managed to resolve the build issues and link statically with pcap by explicitly telling the compiler to dynamically link with dbus. I managed to successfully compile by adding the following two lines to build.rs:

        println!("cargo:rustc-link-lib=static=pcap");
        println!("cargo:rustc-link-lib=dylib=dbus-1");

The first one is not necessary if libpcap.so is not present. If it is, it seems to be preferred. The second line tells the compiler to use the dynamic dbus lib. Otherwise, the compiler does not seem to search for any version, static or dynamic of dbus-1.

With that, I'm not sure what's the right way forward. Even if this is put behind an option it dictates how to link against non-pcap libraries. Perhaps documentation to add correct link directives in a user's build.rs would be enough.

At the same time, I think this shows that a user crate could on its own decide how to link with pcap without any changes required in our build.rs. Perhaps this is the best option. Simply document how to link statically by the end-user.

Thoughts? Opinions?

Wojtek242 commented 2 years ago

At the same time, I think this shows that a user crate could on its own decide how to link with pcap without any changes required in our build.rs. Perhaps this is the best option. Simply document how to link statically by the end-user.

I just did an experiment and created a new binary crate which executed the same code as the getdevices example. I was able to make it link statically by making it use a build.rs file with the following contents:

fn main() {
    println!("cargo:rustc-link-lib=static=pcap");
    println!("cargo:rustc-link-lib=dylib=dbus-1");
}

As such I am no longer convinced that a feature flag in pcap is needed for this at all. I'll leave this PR open for a some time in case somebody can outline a use case where a feature flag would be better.

dilawar commented 1 year ago

It'd help to add https://github.com/rust-pcap/pcap/pull/195#issuecomment-1100223874 in the README.md file where static linking is discussed.

I found this by searching through issues. New rust users who have not suffered with C++ will appreciate more hand-holding here.

dilawar commented 1 year ago

On OpenSUSE-15.4 and Tumbleweed, following worked.

// build.rs
fn main() {
     println!("cargo:rustc-link-lib=static=pcap");
     println!("cargo:rustc-link-lib=dylib=nl-3");
     println!("cargo:rustc-link-lib=dylib=nl-genl-3");
     println!("cargo:rustc-link-lib=dylib=dbus-1");
}