-
Notifications
You must be signed in to change notification settings - Fork 464
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
Debug ci #1085
Debug ci #1085
Conversation
Accidentally based this PR off the wrong branch at first, then forced a repush with just the debug. In the first run however, the linux (bundled) run failed with:
A failure to fetch the upstream sources. I wonder if this is the same sort of flake that just happens to be hitting the Windows vcpkg build too (maybe more often if it is a static IP?). |
Looks like the vcpkg build of sdl-gfx is choking due to invalid character set translation happening in the somewhere in microsoft's toolchain.
codepage 65001 is utf-8 so this seems like a valid thing to warn about? |
an error:
|
This was fixed in upstream vcpkg already: microsoft/vcpkg#14348 Will try spinning with an updated vcpkg ref pointer. |
windows-latest passes at vcpkg HEAD, but ubuntu-latest is broken (missing symbols in link for brotli). |
Spent a good while today trying to understand why the ubuntu-latest build breaks when vcpkg is updated to HEAD. The root of the issue appears to be final link for the examples, where a the final link is:
The breakage happens because I spent a good amount of time trying to work out where that ordering comes from exactly, without any success. I've iterated through several variations in the CMake and automake as well as in the vcpkg port, but I still haven't lucked my way to re-ordering what is processed. I'm wondering if the ordering isn't just alphabetic sorted flat as the three libs are defined flat among each other in pkg-config, but I'm not very sure how these are getting sourced. |
Thanks, it's really appreciated! If ubuntu-latest fails now but didn't fail before, then it looks like the error is coming from vspkg itself, isn't it? In that case, perhaps we could try to somehow |
Yes the error is from vcpkg. Unfortunately, I tried both vcpkg HEAD above, as well as at the commit where the vs2019 fix is added, and both have the same link errors for brotli, implying that there doesn't exist a commit yet upstream where all three builds would work at the same commit. I spent more time digging into how the ordering comes about and indeed within cargo-vcpkg (specifically the vcpkg crate that is used by build.rs) the libs are discovered by path matching the installed package's manifest, which is listed alphabetically (so in the wrong --as-needed sort order). I think the solution is to fold the brotlicommon-static archive into both the brotlidec-static and brotlienc-static archives. Will file an issue against upstream and cook up a pull request with my suggested fix. |
This change allows downstream consumers of the archives to not worry about ordering the link correctly when using this package, which can be problematic when --as-needed is passed to the linker and the order isn't otherwise known. Keeping the brotlicommon-static archive as is allows for probing of the brotli libraries to remain symmetric between dynamic and static links. Fixes static linking issue discussed in microsoft#16991 which is blocking rust-sdl2 CI dependent upon vcpkg discussed here Rust-SDL2/rust-sdl2#1085 .
vcpkg pr sent for brotli, rust-sdl2 CI is now passing with the vcpkg pointing at the fix at my github. :D |
Honestly, I can't thank you enough, huge props to you for finding this out, it's really appreciated! Merging this! |
vcpkg-rs discovers libraries to link using the manifest files from vcpkg. While ports are handled corrrectly already in dependency order, multiple libraries defined by a single Port are otherwise not sorted. This can cause problems when there is an explicit ordering that should be obeyed. This is observed when building rust-sdl2 using the 'use-vcpkg' flag, as this transitively includes the broli port, which defines 3 libraries: libbrotlicommon, libbrotlidec and libbrotlienc. These libraries expect to be linked with libbrotlicommon last, as otherwise rustc's invocation of link will fail (as --as-needed is passed to the linker). See also: microsoft/vcpkg#16991 See also: Rust-SDL2/rust-sdl2#1085 This commit works around the issue by teaching vcpkg-rs the bare minimum required to parse (optional) .pc files if they are available in the vcpkg package, and uses them to infer the correct ordering that should be used. Closes mcgoo#26
vcpkg-rs discovers libraries to link using the manifest files from vcpkg. The manifest files do not indicate any ordering of library files. While ports are handled correctly already in dependency order, multiple libraries defined by a single Port are otherwise not sorted. This can cause problems when there is an explicit ordering that should be obeyed. This is observed when building rust-sdl2 using the 'use-vcpkg' flag, as this transitively includes the brotli port, which defines 3 libraries: libbrotlicommon, libbrotlidec and libbrotlienc. These libraries expect to be linked with libbrotlicommon last, as otherwise rustc's invocation of link will fail (as --as-needed is passed to the linker). See also: microsoft/vcpkg#16991 See also: Rust-SDL2/rust-sdl2#1085 This commit works around the issue by teaching vcpkg-rs the bare minimum required to parse (optional) .pc files if they are available in the vcpkg package, and uses them to infer the correct ordering that should be used. Closes #26
* Add verbose flag (-v) to `cargo vcpkg build` in CI * Fix CI so that it works for all platforms
Creating this PR to iterate on windows-latest vcpkg CI debug.