allow specifying a dockerfile to use for a target and implement "pre-build"ing #678

Emilgardis · 2022-04-03T22:26:03Z

Add optional target.{target}.dockerfile[.file], target.{target}.dockerfile.context and target.{target}.dockerfile.build-args to invoke docker/podman build before using an image.

also adds target.{target}.pre-build to allow pre-building as per #565

In total, this PR allows a user to do the following:

[build]
pre-build = [
    "dpkg --add-architecture $CROSS_DEB_ARCH", 
    "apt-get update && apt-get --assume-yes install libssl-dev:$CROSS_DEB_ARCH"
    ]
[target.x86_64-unknown-linux-gnu]
dockerfile = { file = "./Dockerfile.x86_64_ssl", context = "..", build-args = { ARG1 = "foo" }}
image = "my-image:latest"
pre-build = ["echo 'Hello world!'"]

fixes #565

docs/cross_toml.md

Emilgardis · 2022-04-03T22:55:49Z

src/docker.rs

@@ -248,8 +250,51 @@ pub fn run(
        }
    }

+    let image = if let Some(dockerfile) = config.dockerfile(target)? {
+        let mut docker_build = docker_command("build")?;


Should maybe add a flag to allow specifying more flags for this command

We already have build-args, so we need anything else?

thinking like DOCKER_OPTS, I think a DOCKER_BUILD_OPTS is the best way to do it

Ah ok, that's a good suggestion. Maybe also consider then #770, since we handle more than just docker now? If we're introducing new environment variables, we don't need to worry about backwards compatibility.

src/docker.rs

Emilgardis · 2022-04-04T20:38:35Z

I think this is a nice addition to #635

Emilgardis · 2022-04-05T19:36:58Z

I've made this also implement #565

I think I personally like this approach better to #635

CHANGELOG.md

src/config.rs

src/cross_toml.rs

README.md

Emilgardis · 2022-06-08T11:25:59Z

We can also add a label to our images to signal that an image is intended for cross, and that way we can return those with the tools in #767

not sure what to label them though, org.cross-rs.for-cross-target=<target> ?
(i just bought that domain so we can use it :) )

README.md

src/docker.rs

Alexhuszagh · 2022-06-08T17:54:41Z

src/docker.rs

@@ -248,8 +250,51 @@ pub fn run(
        }
    }

+    let image = if let Some(dockerfile) = config.dockerfile(target)? {
+        let mut docker_build = docker_command("build")?;


We already have build-args, so we need anything else?

src/cross_toml.rs

src/config.rs

Alexhuszagh · 2022-06-08T18:17:08Z

src/docker.rs

+        }
+
+        match self {
+            Dockerfile::File { path, .. } => {


Would it be preferable to handle the FROM {{image}} ourselves? That way the users can just add the components they want afterwards? Not sure about this but I think it might be easier.

I don't understand this comment I think.

do you mean for pre-build, we do something like

FROM {image} {instructions}

where {instructions} is the entries newline separated?

Yeah, say we have a Dockerfile as follows for bindgen support:

RUN apt-get update && apt-get install --assume-yes --no-install-recommends \ libclang-3.9-dev clang-3.9

We want to use the same base Dockerfile for all images, so we'd rather not have to create a new Dockerfile for all targets. For the target armv7-unknown-linux-gnueabihf, this will automatically get transformed to:

FROM ghcr.io/cross-rs/armv7-unknown-linux-gnueabihf:main RUN apt-get update && apt-get install --assume-yes --no-install-recommends \ libclang-3.9-dev clang-3.9

And this way all we need to do is ensure the name is provided in our Cross.toml for all targets. Also, then we could concatenate the pre-build steps and this into a single file and avoid having to add the pre-build tag. If we also have pre-build hooks, this would then become:

FROM ghcr.io/cross-rs/armv7-unknown-linux-gnueabihf:main RUN apt-get update && apt-get install --assume-yes --no-install-recommends \ libclang-3.9-dev clang-3.9 ARG CROSS_CMD RUN eval "${{CROSS_CMD}}"

I fail to see the point, you could already do that with

pre-build = ["apt-get update && apt-get install --assume-yes --no-install-recommends libclang-3.9-dev clang-3.9"]

I see another solution though, make a target.<target>.dockerfile.provide-base that would do it instead with the image you provide. But I still don't see the benefit

That's just a simple example: if something requires the ENV keyword or similar (like, say we want vcpkg for providing dependencies of cross-compiled C/C++ libraries), we can't simply use pre-build hooks.

A more complex example would be:

RUN apt-get update && apt-get install --assume-yes --no-install-recommends \ libclang-3.9-dev clang-3.9 RUN git clone https://github.com/Microsoft/vcpkg.git && ./vcpkg/bootstrap-vcpkg.sh ENV PATH /path/to/vcpkg/bin/dir/:$PATH # do more stuff here, maybe add an ARG, etc. # everything is platform generic.

Maybe we could have pre-build hooks encompass this functionality (RUN isn't implicit anymore, and then all this can be easily encompassed there)? That way you can provide a complete Dockerfile, or just add extra commands to the existing one?

I've implemented this (i think, not fully tested), but with allowing the user to tell cross to insert FROM with target.TARGET.dockerfile.provide-base.

and removed with f98a6e6

instead we provide a build arg

ARG CROSS_BUILD_IMAGE that will be whatever cross would use

Alexhuszagh · 2022-06-08T18:26:28Z

We can also add a label to our images to signal that an image is intended for cross, and that way we can return those with the tools in #767

not sure what to label them though, org.cross-rs.for-cross-target=<target> ? (i just bought that domain so we can use it :) )

I think that's a good idea, and we can then add extracting the label if needed for the list/remove images commands.

Emilgardis

Would it make sense to allow the following

[target.aarch64-unknown-linux-gnu.pre-build]
lines = ["/lib.sh"]

[[target.aarch64-unknown-linux-gnu.pre-build.copy]]
files = ["scripts/lib.sh"]
location = "/"

src/docker/shared.rs

src/config.rs

Emilgardis · 2022-06-15T11:44:03Z

this should be ready now, I'm happy with the functionality.

Alexhuszagh

Minor comments, otherwise looks great.

docs/cross_toml.md

Alexhuszagh · 2022-06-15T18:55:07Z

src/docker/shared.rs

+            content: format!(
+                r#"
+    FROM {image}
+    ARG CROSS_DEB_ARCH=


This (CROSS_DEB_ARCH) should probably be added to the wiki?

I've added it to the readme, will definitely add it to the wiki

scratch that, I haven't... wiki seems good

src/docker/shared.rs

src/file.rs

src/extensions.rs

src/cross_toml.rs

src/docker/shared.rs

Emilgardis · 2022-06-15T21:50:43Z

all done!

Alexhuszagh · 2022-06-15T21:56:59Z

Went over all the changes from the comments, and it looks good. Once this merges I'll be adding additional documentation to the wiki.

bors r+

bors · 2022-06-15T22:19:47Z

Build succeeded:

conclusion

Emilgardis requested a review from a team as a code owner April 3, 2022 22:26

Emilgardis force-pushed the dockerfile branch from be84a9d to cdc4e29 Compare April 3, 2022 22:31

Emilgardis added a commit to Emilgardis/cross that referenced this pull request Apr 3, 2022

add changelog for cross-rs#678

b5c061f

Emilgardis added a commit to Emilgardis/cross that referenced this pull request Apr 3, 2022

add changelog for cross-rs#678

97ec7a0

Emilgardis force-pushed the dockerfile branch from b5c061f to 97ec7a0 Compare April 3, 2022 22:33

Emilgardis added a commit to Emilgardis/cross that referenced this pull request Apr 3, 2022

add changelog for cross-rs#678

220a32f

Emilgardis force-pushed the dockerfile branch from 97ec7a0 to 220a32f Compare April 3, 2022 22:36

Emilgardis commented Apr 3, 2022

View reviewed changes

Emilgardis force-pushed the dockerfile branch 3 times, most recently from afd44c2 to 14c7ff8 Compare April 4, 2022 10:16

Emilgardis mentioned this pull request Apr 5, 2022

Add pre-build hooks #635

Closed

Emilgardis changed the title ~~allow specifying a dockerfile to use for a target~~ allow specifying a dockerfile to use for a target and implement "pre-build"ing Apr 5, 2022

Emilgardis force-pushed the dockerfile branch from cb38ad7 to d2a78ba Compare April 5, 2022 19:35

Emilgardis force-pushed the dockerfile branch from d2a78ba to 0a57e56 Compare April 5, 2022 19:40

Emilgardis commented Apr 5, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src/config.rs Outdated Show resolved Hide resolved

src/cross_toml.rs Outdated Show resolved Hide resolved

Emilgardis commented Apr 6, 2022

View reviewed changes

src/cross_toml.rs Outdated Show resolved Hide resolved

Emilgardis force-pushed the dockerfile branch 3 times, most recently from 2bf937d to 876fcbd Compare April 7, 2022 19:33

Emilgardis commented Apr 7, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Emilgardis mentioned this pull request May 21, 2022

glibc compatibility / make x86_64-unknown-linux-gnu ubuntu again and provide image for glibc compatibility #680

Closed

Emilgardis mentioned this pull request May 31, 2022

Unable to find libclang.so #731

Closed

Emilgardis force-pushed the dockerfile branch from e7b5816 to 94f52ea Compare June 8, 2022 07:48

Emilgardis marked this pull request as draft June 8, 2022 14:46

Alexhuszagh requested changes Jun 8, 2022

View reviewed changes

Emilgardis requested a review from Alexhuszagh June 12, 2022 19:21

Emilgardis force-pushed the dockerfile branch 2 times, most recently from f198b74 to e56d9be Compare June 12, 2022 20:57

Emilgardis commented Jun 13, 2022

View reviewed changes

Emilgardis force-pushed the dockerfile branch 2 times, most recently from ba7a22c to 92db711 Compare June 14, 2022 21:25

Emilgardis added the enhancement label Jun 14, 2022

Emilgardis force-pushed the dockerfile branch from 92db711 to 7c0fe6d Compare June 15, 2022 00:26

Emilgardis commented Jun 15, 2022

View reviewed changes

src/docker/shared.rs Show resolved Hide resolved

src/config.rs Outdated Show resolved Hide resolved

Emilgardis force-pushed the dockerfile branch 2 times, most recently from 1a0a942 to 3c7f900 Compare June 15, 2022 11:32

Emilgardis mentioned this pull request Jun 15, 2022

Make config vectors optional. #801

Merged

Alexhuszagh reviewed Jun 15, 2022

View reviewed changes

Emilgardis force-pushed the dockerfile branch 2 times, most recently from d113ac1 to 0bfc338 Compare June 15, 2022 21:38

allow specifying a dockerfile to use for a target

8b6ea17

Emilgardis force-pushed the dockerfile branch from 0bfc338 to 8b6ea17 Compare June 15, 2022 21:50

Alexhuszagh approved these changes Jun 15, 2022

View reviewed changes

bors bot merged commit b921db8 into cross-rs:main Jun 15, 2022

Emilgardis deleted the dockerfile branch June 15, 2022 22:51

Emilgardis added this to the v0.2.2 milestone Jun 16, 2022

This was referenced Jul 2, 2022

Wishlist: Add a feature to support installing packages in the Docker image when cross runs #281

Closed

Support path to Dockerfile in Cross.toml #196

Closed

This was referenced Nov 6, 2022

Can't use custom docker image? #109

Closed

Add support for custom docker images #42

Closed

uncomfyhalomacro mentioned this pull request Apr 10, 2023

using cross affects ci workflow JuliaLang/juliaup#614

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow specifying a dockerfile to use for a target and implement "pre-build"ing #678

allow specifying a dockerfile to use for a target and implement "pre-build"ing #678

Emilgardis commented Apr 3, 2022 •

edited

Loading

Emilgardis Apr 3, 2022

Alexhuszagh Jun 8, 2022

Emilgardis Jun 9, 2022

Alexhuszagh Jun 9, 2022

Emilgardis commented Apr 4, 2022

Emilgardis commented Apr 5, 2022

Emilgardis commented Jun 8, 2022

Alexhuszagh Jun 8, 2022

Alexhuszagh Jun 8, 2022

Emilgardis Jun 8, 2022 •

edited

Loading

Alexhuszagh Jun 8, 2022 •

edited

Loading

Emilgardis Jun 8, 2022

Alexhuszagh Jun 8, 2022

Alexhuszagh Jun 8, 2022

Alexhuszagh Jun 8, 2022

Emilgardis Jun 11, 2022

Emilgardis Jun 12, 2022

Alexhuszagh commented Jun 8, 2022

Emilgardis left a comment •

edited

Loading

Emilgardis commented Jun 15, 2022

Alexhuszagh left a comment

Alexhuszagh Jun 15, 2022 •

edited

Loading

Emilgardis Jun 15, 2022

Emilgardis Jun 15, 2022

Emilgardis commented Jun 15, 2022

Alexhuszagh commented Jun 15, 2022

bors bot commented Jun 15, 2022

allow specifying a dockerfile to use for a target and implement "pre-build"ing #678

allow specifying a dockerfile to use for a target and implement "pre-build"ing #678

Conversation

Emilgardis commented Apr 3, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emilgardis commented Apr 4, 2022

Emilgardis commented Apr 5, 2022

Emilgardis commented Jun 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emilgardis Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Alexhuszagh Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Alexhuszagh commented Jun 8, 2022

Emilgardis left a comment • edited Loading

Choose a reason for hiding this comment

Emilgardis commented Jun 15, 2022

Alexhuszagh left a comment

Choose a reason for hiding this comment

Alexhuszagh Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Emilgardis commented Jun 15, 2022

Alexhuszagh commented Jun 15, 2022

bors bot commented Jun 15, 2022

Emilgardis commented Apr 3, 2022 •

edited

Loading

Emilgardis Jun 8, 2022 •

edited

Loading

Alexhuszagh Jun 8, 2022 •

edited

Loading

Emilgardis left a comment •

edited

Loading

Alexhuszagh Jun 15, 2022 •

edited

Loading