Add denoland/deno:bin image

[felipecrs]

This simplifies the build process, and also users who want to install deno in their own base images. This pattern is also followed by the Docker Buildx: https://hub.docker.com/r/docker/buildx-bin

For now, some additional complexities were added to the ci.yaml (the entire build-ci) stage. But I plan to also propose a simplification in another PR using the Docker Bake Action and Docker Metadata Action, thus allowing we to build everything in a single job.

Plus, some additional changes were made:

• Format Dockerfiles to use 2 spaces as indent delimiter
• Format ci.yaml with vscode-yaml

[hayd]

This seems like two independent changes:

1. bin release that just exposes the binary (and other dockerfiles copy from that)

This seems more complicated to me. Creates an additional dependency that didn't exist before. Also worth noting the binary won't work with (vanilla) alpine / some other base images. I think buildx pattern might make more sense when the build is more complex than a single "download via curl" step.

2. using ARG instead of ENV for deno version

There has been some discussion of this in the past but I don't recall a compelling argument being made (also, there was a competing deno docker image which used that but it was less popular).

Perhaps I am missing something...

[felipecrs]

1. bin release that just exposes the binary (and other dockerfiles copy from that)

This seems more complicated to me. Creates an additional dependency that didn't exist before. Also worth noting the binary won't work with (vanilla) alpine / some other base images. I think buildx pattern might make more sense when the build is more complex than a single "download via curl" step.

Indeed, having to call docker build twice is not better than before. But:

1. You don't have to call it twice if you are just playing with the distros based Dockerfiles (as long as denoland/deno:bin was already pushed to Docker Hub once).

2. I forgot to add to the PR description this link: https://github.com/docker/buildx/tree/master#from-dockerfile

3. Regarding not working with vanilla alpine images, that's not something that can be solved at our side, but only in the main project -- perhaps publishing an additional deno binary with the rest of the dependencies statically compiled. What we can do is to document the requirements for an alpine image to use it. Should I do it?

4. Ultimately, the entire build process can be further optimized by collapsing all the Dockerfiles into a single one -- leveraging docker multi staged builds, and a bit of Bake perhaps. See chore(docker): add Dockerfile to build deno from source deno#11827 as an inspiration. That way, docker will resolve (and build -- leveraging build cache) the dependencies for each target without having to call multiple docker builds.

[felipecrs]

2. using ARG instead of ENV for deno version

There has been some discussion of this in the past but I don't recall a compelling argument being made (also, there was a competing deno docker image which used that but it was less popular).

Oh, I just used it in the download layer of bin.dockerfile and not in the final image (which still is an ENV). The ARG here is only used to prevent having to hardcode the DENO_VERSION=1.13.2 in multiple stages. At the end, the ARG is converted to an ENV, so there are no breaking changes here.

[hayd]

My recommendation is to publish something on docker hub to play with... That way the community can play with it - and give some feedback. Personally I don't feel qualified/confident in best practices here... and would just go with the simpler solution. 🤷 (Note: I am not a maintainer anymore so not up to me.)

RE the deno PR, similar advice, I suspect they'll be some push back against a new top-level file (let alone several), also - more importantly - deno is already a very complex build matrix (and carefully optimised/cached for performance). My suspicion is a better way could be to create a new repo which includes deno as a submodule and build from that.... In that way you could create version binaries (or daily, similar to how daily deno binaries were built by the community)... this could be a route to something like that being included in mainline deno (as happened with daily deno binaries).

[wperron]

So I played around a bit with this PR: Something that I find interesting is that it does shelves off a few megabytes (at least on Ubuntu). The reason being that while we do remove the packages installed by apt required to download the executable, there are some artifacts left on the filesystem. For example, the ca-certificates package installs a /etc/ca-certificates.conf file that isn't removed from the filesystem when removing the package.

This could easily be solved by using a multi-stage build to isolate the download phase and avoid carrying over these artifacts left over by package managers, but this approach has the nice added advantage of leaving a single point where we have to download the artifact as opposed to the current approach where each Dockerfile installs the packages needed to do that step.

I was also worried about Alpine, but having tested it locally it does seem to work properly since the Alpine image is still based off of frolvlad/alpine-glibc. @hayd if you have some tests that you used to use locally with known issues on Alpine I'd interested, just to make sure that it is indeed correct.

Beyond that there's a few comments that I dropped, in addition to those I'd say I would prefer calling this image scratch instead of bin -- I think that's more in line with the Docker terminology. Also, the bin image uses Alpine to download the binary -- I would feel more confident if it was based on Debian, Ubuntu or Centos as they're distros that I (and probably the rest of the team) am more familiar with and can get better support for; Since the final image is based off of scratch it shouldn't change the final output.

[hayd]

but now the indents don't match 😭

[felipecrs]

@hayd I thought it was a mistake. Do you want me to revert to 1 space?

[hayd]

I was also worried about Alpine, but having tested it locally it does seem to work properly since the Alpine image is still based off of frolvlad/alpine-glibc.

Sorry, that was related to the README instruction to include the binary in your own image. I think that may lead to confused users.

[hayd]

in addition to those I'd say I would prefer calling this image scratch instead of bin

If we do that, we could have FROM scratch as last stage (that just copies the binary from the top layer).

[felipecrs]

in addition to those I'd say I would prefer calling this image scratch instead of bin

If we do that, we could have FROM scratch as last stage (that just copies the binary from the top layer).

Regarding the name of the image, the only example that I found was docker/buildx-bin. The convention about using the base image name as the tag (if base is ubuntu, tag is ubuntu) makes sense, but I don't know at which extent the scratch can be considered a base image. :)

There could be more images in the future which also use scratch as base (as its purpose is just to hold files). The bin contains the deno binary. One called src could contain the deno src package unpackaged, and so.

[wperron]

in addition to those I'd say I would prefer calling this image scratch instead of bin

If we do that, we could have FROM scratch as last stage (that just copies the binary from the top layer).

Regarding the name of the image, the only example that I found was docker/buildx-bin. The convention about using the base image name as the tag (if base is ubuntu, tag is ubuntu) makes sense, but I don't know at which extent the scratch can be considered a base image. :)

I think Andy was saying that the bin image should use FROM scratch as the last build stage, but it's already doing that.

[hayd]

I think Andy was saying that the bin image should use FROM scratch as the last build stage, but it's already doing that.

Yes! Oops. Sorry

[wperron]

So I thought a bit more about this change over the day; I'm in favor of adding this bare Docker layer and even publish it as a scratch image -- I think it can be pretty useful, makes the other images slightly nicer and makes it so we only have one canonical script to download the artifact. Really the only two blockers for me are:

• Removing the dependency on buildx as I don't think it's needed (unless I'm missing something?)
• Making the base image for the download stage something more mainstream like Debian or Ubuntu

[felipecrs]

@wperron Great, I will work on these points.

[felipecrs]

@wperron Done.

1. I used alpine to download because downloading is a very straightforward operation so I just wanted to make it as fast as possible. But I do use Ubuntu for everything otherwise here as well. :)
2. I had to remove build-push-action also so.

[felipecrs]

https://github.com/felipecrs/deno_docker/actions/runs/1164191039

@wperron CI is passing finally. I switch from ubuntu-latest-xl to ubuntu-latest for testing here. Should I switch back to ubuntu-latest-xl? Using a custom runner makes it a bit more difficult for contributors to test their changes, as the CI will not be able to assign a runner in their fork. And I don't know what are the specific needs that ubuntu-latest-xl solves...

[wperron]

https://github.com/felipecrs/deno_docker/actions/runs/1164191039

@wperron CI is passing finally. I switch from ubuntu-latest-xl to ubuntu-latest for testing here. Should I switch back to ubuntu-latest-xl? Using a custom runner makes it a bit more difficult for contributors to test their changes, as the CI will not be able to assign a runner in their fork. And I don't know what are the specific needs that ubuntu-latest-xl solves...

Please do; they run much faster than the regular runner. I agree it's a bit of a bummer when developing on a fork only, but it works as soon as you have a PR open. You can always open PRs as draft in the future, they usually stay unreviewed when in that state.

[felipecrs]

I just did, @wperron.

[wperron]

LGTM, Thanks a lot for the contribution! 🚀