-
-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Windows build with dynamic linking to lib fails #45
Comments
Hi! Thanks for the report I am not sure if we are testing dynamic linking in CI so this might be a new use case. You can see how build script first attempts to find the dynamic libary by
Both of the approaches fail and then it reverts to vendored build. Vendored build fails with link (as you posted). Link is failing because there was no build done. Build was not done because nmake is not found, as far as I can tell: CMake Error at CMakeLists.txt:1 (project): So something is off with visual studio/build setup? The vendored build is tested in CI even. But to get it to link to the provided shared library/dll, we would like to get the smoke tests working
This is failure in linking phase so it seems libcec header is found but dll not. Have you found Microsoft documentation on the env variable to set? In linux this is controlled by LD_LIBRARY_PATH. It also seems that static assertion is not supported by your compiler? We can diagnose this after fixing the libcec linking error -- build script might need some fixing. Found good reference https://learn.microsoft.com/en-us/cpp/c-language/static-assert-c?view=msvc-170 |
Seems like PATH should influence the dll finding process on windows, see https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order Does that particular error disappear with PATH set to C:\Program Files (x86)\Pulse-Eight\USB-CEC Adapter\x64 ? |
Pinging @ok-nick from #15 (introduction of windows support) I noticed the following syntax from Microsoft documentation We are not passing library names to link to now in smoke test build, should we? Line 157 in f31b294
|
Nope, does not solve it unfortunately - it still fails the smoke test. I also attempted to modify the
Correct me if I am wrong, but this feels a bit unintuitive. I understand the idea of best effort when compiling libcec-sys, but it should only do what it's been told - and if not fullfillable - fail hard.
I am continuing research and ultimately targeting to help with simplifying the burden on the user (installing the rather large VS suite.. and a specific build of python etc.) by providing a CI-built repository that includes the different prebuilt flavors of libcec for Windows.
The repository or let's rather say the github workflow is Result: https://github.com/skraus-dev/libcec-vendor What's missing still is the commit of the binary files back into the repository, so the repo can be easily included as a submodule to consume the bins from there. PS: If this goes too much off-topic, happy to continue in #24. PPS: Why use prebuilts? I took the idea from https://github.com/ftdi-rs/libftd2xx-ffi/blob/main/build.rs which just has a quite minimalistic build.rs that does not attempt to build libs and rather just emits compiler directives (except for bindgen). Tbf, libftd2xx does not have the option to compile from source, as the manufacturer FTDI only provides binaries. |
Quick comments from my phone Re, vendored feature etc: I have followed the model from some of the major sys crates, should not be too surprising. I see the default feature set as "best effort" not really as "non-vendored" Re. Prebuilt libraries: I do not completely follow, would not the official libcec distribution included the needed binaries? Or are referring to needed libraries / artifacts for static linking? |
Re /link, good idea experimenting correct flags. Is it always complaining not finding the right libcec symbol? Do you agree that we should get that standalone C program build working first? This should be basic C/MSVC stuff, not related to rust at this level. Unfortunately, my understanding is limited |
I am referring to both, dynamic and static linking. This sums is up: https://learn.microsoft.com/en-us/cpp/build/linking-an-executable-to-a-dll?view=msvc-170 From my understanding, MSCV relies on a *.lib to link against either way, statically or dynamically. In libcec's case:
You would use the DLL in cases when defining
Bottom-line: Pulse-Eight's Libcec distribution for windows ONLY contains the DLL.
Sure!
It's solved now:
-> Manual build of libcec via cmdline in Developer Command Prompt worked from this point on. libcec_sys (vendored) is compiled also successfully, when started from "Developer command prompt". Next stepsEnsure the theory about *.lib files by experimenting with the linker directives in build.rs |
Hey, I switched to Linux a while ago so I'm not sure if I could be much help. If you have any other questions I will do my best to answer. |
@skraus-dev good info, today I learned sometving. It looks like it might be possible to generate .lib from .dll? https://stackoverflow.com/questions/9946322/how-to-generate-an-import-library-lib-file-from-a-dll Also, if there's a lot of changes, I propose to rebase on top ci-speedup branch, there are some heavier github CI and build.rs changes. I need to clean up the branch a bit still and merge to main (EDIT: now merged) Thanks @ok-nick , seems like we figured out the necessary steps |
Once again regarding preference of vendored vs non-vendored, not sure where I picked that up. Some examples openssl-sysI do see similar type of logic is used for static vs dynamic building in openssl-sys: https://github.com/sfackler/rust-openssl/blob/master/openssl-sys/build/main.rs#L351-L398 (if static is explicitly requested, we do static build. Otherwise try to do dynamic build). However, same crate uses vendored sources only if libgit2-syshttps://github.com/rust-lang/git2-rs/blob/master/libgit2-sys/build.rs Uses vendored when explicitly requested. Vendored can be also disabled with an env variable (LIBGIT2_NO_VENDOR) // Specify `LIBGIT2_NO_VENDOR` to force to use system libgit2.
// Due to the additive nature of Cargo features, if some crate in the
// dependency graph activates `vendored` feature, there is no way to revert
// it back. This env var serves as a workaround for this purpose. Even if user has not explicitly specified the bzip2-rshttps://github.com/alexcrichton/bzip2-rs/blob/master/bzip2-sys/build.rs With So yeah, as summary, practices seem to vary between popular crates... |
Ideally it would be nice to have the following
Thoughts? |
Yeah that sounds good, to not rely on Developer command prompt environment but rather source the environment ourselves, if found. this step should then also verify the existance of components
While it's possible to forge a Concerning DLL -> Import Library, the new rust 1.71.0 release introduced a nice feature, but that's on you to decide if it's feasible to implement. Allows defining linkage to a DLL inline in code. RFC: https://rust-lang.github.io/rfcs/2627-raw-dylib-kind.html
Introducing a distinct handling for static linking sounds good! Static linking the C runtime looks independent from the userspace-library linking. See: https://doc.rust-lang.org/reference/linkage.html#static-and-dynamic-c-runtimes
See answer to point 1 |
👍 minor correction: python should not be needed anymore: Line 109 in 07859ab
I see, got it now.
This sounds something worth investigation! 1.71.0 is quite new though...
Yes, "static" feature would only control libcec link, not the C runtime. |
Bug description
Attempting to build a project which depends on
libcec-sys
orcec-rs
yields error:LINK : fatal error LNK1181: cannot open input file 'cec.lib'
Smoke tests apparently all fail...
Will be happy to send a PR for more detailed README in that regard, if it can be narrowed down.
To Reproduce
Cargo.toml
main.rs
C:\Program Files (x86)\Pulse-Eight\USB-CEC Adapter\x64\cec.dll
C:\Program Files (x86)\Pulse-Eight\USB-CEC Adapter\x64
as that made a bit more sense to me, still, same result.C:\Program Files (x86)\Pulse-Eight\USB-CEC Adapter\include
Developer command prompt
or regularcmd
cargo build
orcargo build --release
Expected behavior
Build process of libcec-sys recognizing the
cec.dll
and dynamically linking with it.Environment
Additional context
Verbose output for
cargo build -vv
The text was updated successfully, but these errors were encountered: