Closed robo9k closed 3 years ago
After staring at that Windows build failure from runs/3969370194 for a while, I believe these are the problems:
magic.lib(magic.obj) : error LNK2019: unresolved external symbol __imp_PathRemoveFileSpecA referenced in function _w32_get_magic_relative_to
Due to the vcpkg
triple "x64-windows-static-md" MSVC is instructed to statically build and link "magic.lib". However as can be seen from the error message, that means we need to link to shlwapi
for PathRemoveFileSpecA
.
Yet when we inspect the output of vcpkg-rs
in target\debug\build\magic-sys-*\output
:
cargo:rustc-link-lib=magic
cargo:rustc-link-lib=tre
cargo:rustc-link-lib=getopt
Those seem to be coming from microsoft/vcpkg ports/libmagic/vcpkg.json
Looking at the link.exe
invocation we also see:
"kernel32.lib"
"advapi32.lib"
"ws2_32.lib"
"userenv.lib"
"msvcrt.lib"
No shlwapi
.
The build can be "fixed" by adding the following to build.rs
:
println!("cargo:rustc-link-lib=shlwapi");
But that is not the right place.
Looking at file/file src/Makefile.am we see:
if MINGW
MINGWLIBS = -lgnurx -lshlwapi
else
MINGWLIBS =
endif
libmagic_la_LIBADD = $(LTLIBOBJS) $(MINGWLIBS)
The libmagic.pc
generated by vcpkg/autotools contains the following:
Libs.private: -ltre -lkernel32 -luser32 -lgdi32 -lwinspool -lshell32 -lole32 -loleaut32 -lcomdlg32 -ladvapi32
Thus I believe there are 2 upstream issues:
file/file
would need to fix their Makefile.am
to include -lshlwapi
for Windows in general, not just MinGWmcgoo/vcpkg-rs
would have to use the private libs from pkg-config
to build the requested package as a static libraryI believe this didn't surface earlier with vcpkg since there was no test with all linked symbols. cargo build
does not care about the missing symbols.
According to https://repology.org/project/file/cves the libmagic
5 versions without a CVE at time of writing 2021-10-22 are:
That might be motivation to drop support for old (API) versions sooner than later.
Regarding feature flag names, see gdesmott/system-deps#18 Essentially they'd have to be named "v*" (instead of "libmagic-abi-v5??").
Feature names could be changed later, but that'd be a semver breaking change.
This adds crate features named "libmagic-abi-v5??" for each change in the v5
libmagic
API/ABI. In general, the API seems to be backwards compatible by only adding new functionality.The use case for this is supporting both old and new versions of the
libmagic
API, depending on the user setup.Currently no effort is made to identify the version of
libmagic
that is being linked against. If symbols are undefined, a linker error will occur. If linking works, there might still be slight API differences.It would probably be possible to indirectly identify versions by their symbols and defines, but this seems unclean. Upstream seems to have switched to
autotools
and added a#define MAGIC_VERSION
in v5.13. Thelibmagic.pc
was only added in v5.39, see #1Slightly unrelated changes:
cargo fmt
, no enforcement via CIcargo clippy
have been fixed, no enforcement via CI#define
s have mostly been added as constants, so they can be used from e.g. themagic
cratecargo test
tests were added to ensure linking with the feature gated symbols worksMagic
struct has been fixed according to the 'nomicon and now mostly matches whatbindgen
would emitLeftover TODOs for myself:
docs.rs
libmagic
needs to be (any) version 5, update doc for #5magic
cratemagic-sys
crate release (candidate?)Misc notes:
file-5.00
seems to be from 2009-02-03, that should be sufficiently old to support even ancient systemsfile-5.41
is the latest considered version from 2021-10-18 and according to e.g. https://repology.org/project/file/badges has not even made it to all package managersdefault
feature should probably select a little older API version, given that otherwise crate compilation/linkage might fail on most systems currently, see previous bullet point