-
-
Notifications
You must be signed in to change notification settings - Fork 48
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
🔨 Fix docker dav1d
shared library linking
#408
🔨 Fix docker dav1d
shared library linking
#408
Conversation
@@ -2,7 +2,7 @@ | |||
# Frontend Build Stage | |||
# ------------------------------------------------------------------------------ | |||
|
|||
FROM node:20.0.0-alpine3.16 as frontend | |||
FROM node:20.0.0-alpine3.16 AS frontend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just to appease the linting
docker/Dockerfile
Outdated
ninja install; \ | ||
ln -s /usr/local/lib/x86_64-linux-gnu/libdav1d.so.7.0.0 /usr/local/lib/libdav1d.so.7; \ | ||
ln -s /usr/local/lib/x86_64-linux-gnu/libdav1d.so.7.0.0 /usr/local/lib/libdav1d.so.7.0; \ | ||
ln -s /usr/local/lib/x86_64-linux-gnu/libdav1d.so.7.0.0 /usr/local/lib/libdav1d.so.7.0.0; \ | ||
echo "/usr/local/lib" >> /etc/ld.so.conf.d/mylibs.conf \ | ||
&& ldconfig |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could probably be done a lot more elegantly and generically but this seems to work so I'd be interested to see how I could do it better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I won't have time to test this until after work, but this won't account for arm64 builds
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'm happy to look at it again tomorrow to work out how to make it more generic (like using TARGETARCH
or something). Defs just a hack for now to get it to work for me
If you have an idea of how to improve I'll get to it in the morning but it's late over here so gonna call it for this attempt
dav1d
shared library linking dav1d
shared library linking
RUN cargo prisma generate --schema ./core/prisma/schema.prisma; \ | ||
./scripts/release/utils.sh -w; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change allows me to build locally for testing but I'm happy to revert if needing
I had to overwrite the entrypoint on my docker compose to get this to successfully run: |
The change that I just pushed allowed me to just run the image without changing the |
dav1d
shared library linking dav1d
shared library linking
set -ex; \ | ||
./scripts/release/utils.sh -p; \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that I actually had to remove the prisma.rs
text replacements because it was actually generated in the Dockerfile
above so was already correct
docker/Dockerfile
Outdated
COPY --from=builder /usr/local/lib /usr/local/lib | ||
COPY --from=builder /etc/ld.so.conf.d/mylibs.conf /etc/ld.so.conf.d/mylibs.conf |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
COPY --from=builder /usr/local/lib /usr/local/lib <- this is necessary
COPY --from=builder /etc/ld.so.conf.d/mylibs.conf /etc/ld.so.conf.d/mylibs.conf <- this can probs be removed because I do it in `entrypoint.sh` now
@wolffshots I made a few changes, since nightly is broken I wanted to make sure at the very least a working image goes out sooner rather than later. I appreciate your fixes! I translated them slightly to work for the arm64 build and then tweaked the After some discussions on Discord, once this fix is merged I will likely revert the AVIF-specific additions until another time when a more batteries-included solution can be put together. |
Great stuff! Glad to have been able to help and thanks for all the hard work |
I had to remove it, with prisma generation it stalled after 4+ hours on the arm64 build. The build without it completed both amd64 and arm64 in just under 2 😅 But I added some parameters you can use to generate it in the docker build. You can do something like: PLATFORMS='linux/amd64' TAG='local' RUN_PRISMA_GENERATE=true ./docker/build.sh |
Fair that makes sense
Great stuff, that's all good by me |
Adding explicit linking to the process appears to fix the error that I was experiencing in Docker builds that I mentioned in #406:
The Dockerfile still does not build locally despite adding a step to generate
prisma.rs
because of this error but that seems to be a separate issue: