Skip to content
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

Meta: Use dynamic vcpkg linkage for release builds #1528

Merged
merged 9 commits into from
Nov 6, 2024

Conversation

trflynn89
Copy link
Contributor

@trflynn89 trflynn89 commented Sep 25, 2024

No description provided.

@ADKaster
Copy link
Member

ADKaster commented Nov 5, 2024

I rebased this and bumped the simdutf version, hopefully this satisfies the github action cache enough to let this move forward and unblock Alex's GPU work

@trflynn89
Copy link
Contributor Author

The cache action thing was basically avoided by the commit to bump to the October release. The ongoing issue is that skia doesn't build.

@ADKaster
Copy link
Member

ADKaster commented Nov 5, 2024

Pushed a workaround for microsoft/vcpkg#41971

This is a fix to the vcpkg package itself to fix some exported symbols.
ADKaster and others added 7 commits November 5, 2024 14:04
Using install(IMPORTED_RUNTIME_ARTIFACTS), we can re-export the
shared library and frameworks we imported from outside the build
tree into our install treer. This is required for simdutf as it
is a dependency of the AK library, and we need liblagom-ak.so to
be loadable when doing fuzzer and cross-compile builds for the
Lagom tools.
The reason for this change is that CMake/vcpkg are unable to detect a
change to VCPKG_LIBRARY_LINKAGE. So when we switch to dynamic builds,
the switch would be non-functional, and every developer would have to
remove their Build and vcpkg cache directories manually. By changing
these directories, vcpkg is able to detect it must rebuild.
In addition to changing the build-type dependent build directories, we
can take this opportunity to move the vcpkg cache directory to the Build
folder itself. This probably isn't 100% needed, but it ensures that no
leftover artifacts are used from non-dynamic vcpkg builds, and it's also
generally nice to have all build artifacts under Build.
By using static linkage, we are opening ourselves up to ODR violations
that result in runtime crashes (rather than build-time link errors). For
example, we've seen this happen while trying to use Skia context in both
LibGfx and LibWeb.
With luck, this will blast the old vcpkg cache for simdutf in
github actions and allow us to move forward with the new dynamic
linking support.
This works around an issue in upstream skia where a debug build with
dynamic libraries includes an extra file in the skparagraph module
that is not compilable on macOS.
@ADKaster ADKaster marked this pull request as ready for review November 5, 2024 21:26
@ADKaster
Copy link
Member

ADKaster commented Nov 6, 2024

Let's go with this and deal with the fallout. :marge:

@ADKaster ADKaster merged commit 80d9949 into LadybirdBrowser:master Nov 6, 2024
6 checks passed
@trflynn89 trflynn89 deleted the vcpkg_shared branch November 6, 2024 17:52
@linusg
Copy link
Contributor

linusg commented Nov 8, 2024

This seems to have broken the artifact created by ladybird-js-artifacts.yml (affecting any esvu-installed js(1), e.g. https://github.com/boa-dev/data/actions/runs/11734525189/job/32690696943). https://www.google.com/search?q=%22libsimdutf.so.11%22 suggests that I can install it by hand only on FreeBSD and Alpine so it should probably be shipped alongside liblagom.

./bin/js: error while loading shared libraries: libsimdutf.so.11: cannot open shared object file: No such file or directory
$ cd ladybird-js-Linux-x86_64
$ ls lib
liblagom-ak.so             liblagom-coreminimal.so.0.0.0  liblagom-js.so.0         liblagom-syntax.so           liblagom-unicode.so.0.0.0
liblagom-ak.so.0           liblagom-crypto.so             liblagom-js.so.0.0.0     liblagom-syntax.so.0         liblagom-url.so
liblagom-ak.so.0.0.0       liblagom-crypto.so.0           liblagom-line.so         liblagom-syntax.so.0.0.0     liblagom-url.so.0
liblagom-core.so           liblagom-crypto.so.0.0.0       liblagom-line.so.0       liblagom-textcodec.so        liblagom-url.so.0.0.0
liblagom-core.so.0         liblagom-filesystem.so         liblagom-line.so.0.0.0   liblagom-textcodec.so.0      
liblagom-core.so.0.0.0     liblagom-filesystem.so.0       liblagom-regex.so        liblagom-textcodec.so.0.0.0  
liblagom-coreminimal.so    liblagom-filesystem.so.0.0.0   liblagom-regex.so.0      liblagom-unicode.so          
liblagom-coreminimal.so.0  liblagom-js.so                 liblagom-regex.so.0.0.0  liblagom-unicode.so.0        
$ ldd bin/js | grep '=> not found' | grep -v 'libstdc++'
	libsimdutf.so.11 => not found
	libicui18n.so.74 => not found
	libicuuc.so.74 => not found
$

@linusg
Copy link
Contributor

linusg commented Nov 8, 2024

@trflynn89
Copy link
Contributor Author

Should be fixed by #2225

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants