`docker-compose`-style `context` and `dockerfile` config #446

alsuren · 2020-07-25T13:07:01Z

I was following along with https://www.collabora.com/news-and-blog/blog/2020/06/23/cross-building-rust-gstreamer-plugins-for-the-raspberry-pi/ and found the custom docker image workflow quite awkward (see also #149 )

In this PR, I add support for context and dockerfile options, based on the build config options that you can supply when using docker-compose (see https://docs.docker.com/compose/compose-file/#build for details)

I have been using this in https://github.com/alsuren/mijia-homie (the related files have recently been changed in this PR alsuren/mijia-homie#102) to cross-compile to a raspberry pi and a few other devices around my house.

I would be very interested in getting feedback from @gdesmott on this because I have never used podman, and don't know whether the same tricks can be used.

support other docker-compose build options?

docker-compose specifies a bunch of other options in the build section. Should we think about supporting those as well, or is that too much scope creep?

(edit: I decided that I would implement "args" because I have used this in docker-compose, but not implement any of the other options that I've never used)

toml namespaces?

should we add [target.armv7-unknown-linux-gnueabihf.docker.build] section to our toml, or is it okay to keep everything flat under the [target.armv7-unknown-linux-gnueabihf] namespace? (I think I'm going to keep it flat)

code structure, verbosity~, and running docker build multiple times~?

In order to get the sha id of the docker image, I run docker build --quiet and strip off the trailing newline. Anyone who is used to building docker images will be expecting to see a bunch of output when docker build is running. By specifying --quiet, we suppress this output, so people will just observe the cross command hanging. If they specify --verbose then they will see the output they're expecting.

I'm actually not that fussed about calling docker build lots of times, because it's pretty cheap when everything is cached (I think it just checksums all of the files that are COPYed from the build context and then noops). If anyone else has a better way to do this, please shout.

Also, even if you are happy with how I'm interacting with docker, please suggest refactors to my code. I feel like there are a bunch of layering violations, but my brain is not working well enough to unpick them at the moment.

Failing tests

~~Are these failing tests normal? They don't seem to have anything to do with my change.~~

rvolgers · 2020-07-26T16:57:11Z

src/docker.rs

+        // then run it again with --quiet to get the image sha.
+        docker.run(verbose)?
+    }
+    let mut image_id = docker.arg("--quiet").run_and_get_stdout(verbose)?;


You should add the --iidfile option to the first docker build to write the image id to a temp file, instead of running it again and trying to parse the human-readable output.

That seems to work. Thanks. Pushed.

rvolgers

(Note: I'm just a random person who is interested in using this, I have no authority to accept or request changes. Just adding my 2 cents.)

Personally I prefer this style of using docker (just using image ids which are generated by re-running the build, instead of named images/tags) as it means you always know exactly what you're using and it will always be up to date.

alsuren · 2020-07-26T18:33:13Z

src/docker.rs

+    }
+    let mut image_id = std::fs::read_to_string(&image_id_filename)?;
+    image_id.retain(|c| c != '\n');
+    std::fs::remove_file(&image_id_filename).unwrap_or_else(


I kind-of want to do this within a ScopeGuard, or import and use https://docs.rs/tempfile/3.1.0/tempfile/struct.NamedTempFile.html . What is cross's policy on adding new dependencies?

alsuren · 2020-08-02T13:47:04Z

docker/Dockerfile.armv7-unknown-linux-gnueabihf

@@ -1,28 +1,31 @@
+# This was built from ./Dockerfile.context
+FROM alsuren/cross-context:latest as context
+
 FROM ubuntu:16.04


This idea turns out to be really powerful when creating custom images, because it allows you to provide context as an empty directory and dockerfile as something that uses this pattern to fetch build scripts etc.

In https://github.com/lcsfelix/reading-xiaomi-temp/pull/1/files I was previously using context = "../cross/docker" which is probably not something you want to check into your git repo, because it requires all developers to keep a copy of the cross repo in a specific location.

TODO:

upload this cross-context docker image somewhere more official-sounding, and make sure that it gets uploaded and tagged with each release.

revert the changes to this file, and produce a dedicated "example custom build image" dockerfile that uses this pattern.

alsuren · 2020-08-02T13:55:18Z

@rvolgers I would be interested in getting experience reports. If you have time, could you install from my branch and tell me how you get on.

cargo install --git=https://github.com/alsuren/cross --branch=docker-build-context

alsuren · 2020-10-24T22:12:27Z

docker/Dockerfile.context

+# It has been uploaded to `alsuren/cross-context` so it can be used like so:
+
+## Fetch this image and use it as `context`:
+# FROM alsuren/cross-context:latest as context


What's the process for getting this uploaded somewhere more official?

Our CI has access to dockerhub, see

https://github.com/rust-embedded/cross/blob/2af06e2a871f4cf467eb241a40d6ebef0cc43a8f/azure-pipelines.yml#L123

For example

Hrm. The more I think about it, maybe this Dockerfile.context thing should be punted to another PR.

It isn't strictly necessary for my use-case (cross-compiling something that links against dbus). You can get away with something like:

FROM rustembedded/cross:armv7-unknown-linux-gnueabihf-0.2.1 RUN dpkg --add-architecture armhf && \ apt-get update && \ apt-get install -y libssl-dev:armhf libdbus-1-dev:armhf ENV PKG_CONFIG_PATH=/usr/lib/arm-linux-gnueabihf/pkgconfig

adding Dockerfile.context means that all of your build-scripts are suddenly exposed like public API, and are easy to break accidentally.

Testing the context image is fiddly and intrusive, and requires more thought.

Feels wrong somehow.

This reverts commit 2741964.

This reverts commit 3e03356.

This reverts commit eb49ad0.

alsuren · 2020-12-22T21:11:13Z

Okay. I have backed out all of the questionable Dockerfile.context bits. I think I'm happy with this.

I've also been using it for a while. The thing that's most questionable is the fact that we fall back to building on the host if we fail to build/find a docker image. This means that if you mess up the Dockerfile syntax and don't pass --verbose then you get output like this (and it's easy to miss the first line and wonder why everything is broken further down).

Warning: `"/usr/local/bin/docker" "build" "./docker" "--iidfile" "/var/folders/kp/k8982sg57h7cq12ylw459f180000gn/T/50928.iid" "-f" "./docker/Dockerfile.dbus.armv7-unknown-linux-gnueabihf"` failed with exit code: Some(1) Falling back to `cargo` on the host.
   Compiling libc v0.2.81
   Compiling syn v1.0.54
   Compiling cfg-if v0.1.10
   Compiling log v0.4.11
...

I think that this should be fixed by adding a flag to forbid falling back to building on the host. Do we want this in this PR or a follow-up?

Emilgardis · 2020-12-24T10:59:26Z

src/docker.rs

+    let mut image_id = std::fs::read_to_string(&image_id_filename)?;
+    image_id.retain(|c| c != '\n');
+    std::fs::remove_file(&image_id_filename).unwrap_or_else(
+        |e| println!(


Suggested change

|e| println!(

|e| eprintln!(

Good point. Fixed.

I also pushed da74ad3, because I realised that I had introduced some dead code as part of an earlier change.

Emilgardis

Looks good, going to wait with merge myself as I want to try it out myself, if any other code owner reviews feel free to merge

alsuren · 2021-07-27T17:18:18Z

I realised that I have not used this functionality since I added a script to build my images.

I'm going to close this now, so there is one less thing for the new maintainer to deal with.

alsuren requested review from Dylan-DPC-zz and a team as code owners July 25, 2020 13:07

alsuren mentioned this pull request Jul 26, 2020

POC Raspberry Pi cross-compliation lcsfelix/reading-xiaomi-temp#1

Closed

rvolgers reviewed Jul 26, 2020

View reviewed changes

rvolgers suggested changes Jul 26, 2020

View reviewed changes

alsuren commented Jul 26, 2020

View reviewed changes

alsuren commented Aug 2, 2020

View reviewed changes

alsuren force-pushed the docker-build-context branch from 6ad4c08 to 1d39940 Compare October 24, 2020 18:51

alsuren commented Oct 24, 2020

View reviewed changes

alsuren added 8 commits December 22, 2020 15:27

docker-compose-style context and dockerfile options

6a8230b

use docker --iidfile

04069db

implement docker build args

e5a7565

update docs

dca7bf2

alsuren/cross-context docker image

8dc1a70

forbid specifying image and context together for now

494dc0a

document Dockerfile.context

908033b

add lib.sh to Dockerfile.context

8efb3ec

alsuren force-pushed the docker-build-context branch from 3a9984f to 908033b Compare December 22, 2020 15:32

alsuren added 8 commits December 22, 2020 15:40

try adding Dockerfile.context to build matrix

eb49ad0

skip test.sh for Dockerfile.context

3e03356

squash unused variable warning

cd158ba

have a go at testing Dockerfile.context

2741964

Feels wrong somehow.

Revert "have a go at testing Dockerfile.context"

2ae429d

This reverts commit 2741964.

remove Dockerfile.context

36c7f5c

Revert "skip test.sh for Dockerfile.context"

c0b7862

This reverts commit 3e03356.

Revert "try adding Dockerfile.context to build matrix"

26b088e

This reverts commit eb49ad0.

remove dead code

da74ad3

Emilgardis requested changes Dec 24, 2020

View reviewed changes

alsuren added 2 commits December 24, 2020 12:12

fix code review comment

e49de58

docker compose spec has moved and been reworded

22b8b15

Emilgardis approved these changes Dec 26, 2020

View reviewed changes

Emilgardis requested a review from a team December 26, 2020 00:15

alsuren closed this Jul 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`docker-compose`-style `context` and `dockerfile` config #446

`docker-compose`-style `context` and `dockerfile` config #446

alsuren commented Jul 25, 2020 •

edited

Loading

rvolgers Jul 26, 2020

alsuren Jul 26, 2020

rvolgers left a comment

alsuren Jul 26, 2020

alsuren Aug 2, 2020 •

edited

Loading

alsuren commented Aug 2, 2020

alsuren Oct 24, 2020 •

edited

Loading

Emilgardis Oct 25, 2020

alsuren Dec 22, 2020 •

edited

Loading

alsuren commented Dec 22, 2020

Emilgardis Dec 24, 2020

alsuren Dec 24, 2020

Emilgardis left a comment

alsuren commented Jul 27, 2021

docker-compose-style context and dockerfile config #446

docker-compose-style context and dockerfile config #446

Conversation

alsuren commented Jul 25, 2020 • edited Loading

rvolgers Jul 26, 2020

Choose a reason for hiding this comment

alsuren Jul 26, 2020

Choose a reason for hiding this comment

rvolgers left a comment

Choose a reason for hiding this comment

alsuren Jul 26, 2020

Choose a reason for hiding this comment

alsuren Aug 2, 2020 • edited Loading

Choose a reason for hiding this comment

alsuren commented Aug 2, 2020

alsuren Oct 24, 2020 • edited Loading

Choose a reason for hiding this comment

Emilgardis Oct 25, 2020

Choose a reason for hiding this comment

alsuren Dec 22, 2020 • edited Loading

Choose a reason for hiding this comment

alsuren commented Dec 22, 2020

Emilgardis Dec 24, 2020

Choose a reason for hiding this comment

alsuren Dec 24, 2020

Choose a reason for hiding this comment

Emilgardis left a comment

Choose a reason for hiding this comment

alsuren commented Jul 27, 2021

`docker-compose`-style `context` and `dockerfile` config #446

`docker-compose`-style `context` and `dockerfile` config #446

alsuren commented Jul 25, 2020 •

edited

Loading

alsuren Aug 2, 2020 •

edited

Loading

alsuren Oct 24, 2020 •

edited

Loading

alsuren Dec 22, 2020 •

edited

Loading