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

Build mips-unknown-linux-musl from Debian instead of Ubuntu to have access to their packages #1332

Closed
wants to merge 1 commit into from

Conversation

sarfata
Copy link

@sarfata sarfata commented Sep 14, 2023

Ubuntu does not support mips, so we cannot add libraries with pre-build hooks like suggested in the documentation.

This changes the mipsel-unknown-linux-musl image to build from Debian stable.

You can now do:

pre-build = ["dpkg --add-architecture mipsel && apt-get update && apt-get install -y zlib1g-dev:mipsel && ln -s /usr/include/zlib.h /usr/local/mipsel-linux-muslsf/include"]

I realize this is not a benign change but it ended up being pretty small. I am curious if this is something you would consider merging - or if you would suggest a better approach.

Ubuntu does not support mips, so we cannot add libraries with pre-build
configuration in Cross.toml.

This changes the mipsel-unknown-linux-musl image to build from Debian
stable.

You can now use something like:

```
pre-build = ["dpkg --add-architecture mipsel && apt-get update && apt-get install -y zlib1g-dev:mipsel && ln -s /usr/include/zlib.h /usr/local/mipsel-linux-muslsf/include"]
```
@sarfata sarfata requested a review from a team as a code owner September 14, 2023 00:01
@sarfata sarfata changed the title fix: ubuntu does not provide mips packages Build mips-unknown-linux-musl from Debian instead of Ubuntu to have access to their packages Sep 14, 2023
Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea, but maybe we should keep the ubuntu based image just incase?

One issue I see is our override of sources here, should probably remove that for this target.

The ubuntu image can be kept as docker/Dockerfile.mipsel-unknown-linux-musl.ubuntu, it would then need to be added to targets.toml like we've done for centos

@@ -49,7 +49,7 @@ if_centos() {
}

if_ubuntu() {
Copy link
Member

@Emilgardis Emilgardis Sep 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably be renamed to if_aptitude I think

@@ -1,4 +1,4 @@
FROM ubuntu:20.04
FROM debian:bookworm
Copy link

@joseluisq joseluisq Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
FROM debian:bookworm
FROM debian:12.2-slim

@sarfata
Copy link
Author

sarfata commented Feb 7, 2024

Sorry - Keep meaning to get back to this but have not found the time. Will close the PR for now and will re-open when I have applied the requested changes.

@sarfata sarfata closed this Feb 7, 2024
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