-
Notifications
You must be signed in to change notification settings - Fork 53
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 build on x86_64-unknown-linux-musl because of missing zlib. #99
base: master
Are you sure you want to change the base?
Conversation
@Douile Can you take a look when you have time? |
Could you link to a log where a build fails because of zlib please. |
The failure was: ``` = note: /usr/lib/gcc/x86_64-alpine-linux-musl/13.2.1/../../../../x86_64-alpine-linux-musl/bin/ld: cannot find -lz: No such file or directory collect2: error: ld returned 1 exit status ```
Yes: https://github.com/orium/cargo-rdme/actions/runs/11489666325/job/31978894158 I've also updated the PR to use rust 1.82. |
@Douile Hi. Can you take a look at the log/PR when you have time? |
It seems this PR does not fix the issue (https://github.com/orium/cargo-rdme/actions/runs/11713088614/job/32625153289). I'll look at this soon. |
Found the issue. The current PR does solve the problem: for some reason even though I pointed to orium/rust-build.action it still used the original docker image. The problem is the |
If its for a dependency of a crate I don't think it should go in the base image: a pre-build script can be used to install crate dependencies. |
That's what I did in orium/cargo-rdme@79b8d25. One one hand I tend to agree that dependencies on system libraries that crates have are out of scope. On the other hand there should be at least a minimum system installation crates should count on. I would say something like libz may arguably be part of that minimum installation. I think at least rust-build should offer a simple way to install extra packages that do not require a pre-build script. I'm thinking an environment variable where people can define a set of packages to install. What do you think? I'm happy to change this PR to add a |
The failure was:
This PR also updates rust and alpine.
For my future reference. To test builds locally do
Closes #100