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

cmd/shfmt: official docker images #68

Closed
mmlb opened this issue Feb 22, 2017 · 42 comments
Closed

cmd/shfmt: official docker images #68

mmlb opened this issue Feb 22, 2017 · 42 comments

Comments

@mmlb
Copy link

mmlb commented Feb 22, 2017

I'd be great if you could provide a docker image on docker hub. My use case is to use it with drone.io, there are other CIs that can use docker images natively. Something like:

FROM alpine:latest
ENTRYPOINT ["/usr/bin/shfmt"]
ADD https://github.com/mvdan/sh/releases/download/v1.2.0/shfmt_v1.2.0_linux_amd64 /usr/bin/shfmt
RUN chmod +x /usr/bin/shfmt

would work for me.

And maybe the following for automated builds:

FROM golang:alpine
ENTRYPOINT ["shfmt"]
COPY . /go/src/app
WORKDIR /go/src/app
RUN set -ex \
       && apk add --no-cache --virtual .build-deps git \
       && go-wrapper download ./... \
       && go-wrapper install ./... \
       && apk del .build-deps 
@mvdan
Copy link
Owner

mvdan commented Feb 23, 2017

Usually people would build their own docker images with multiple tools, not just shfmt:

https://github.com/search?l=Dockerfile&q=shfmt&type=Code&utf8=%E2%9C%93

In that list, there are also some people making small images with just this tool.

Why should I make an "official" image? How is that better than just an official binary?

@mvdan
Copy link
Owner

mvdan commented Mar 9, 2017

reping @mmlb

@mvdan
Copy link
Owner

mvdan commented Mar 10, 2017

I'm going to close this, as I don't see how an official docker image adds anything on top of an official static binary.

@mvdan mvdan closed this as completed Mar 10, 2017
@mmlb
Copy link
Author

mmlb commented Mar 20, 2017

hey @mvdan

So sorry to not respond, I somehow disabled email notifications and wasn't checking them from github. Its cool if you don't want to implement this, my use case was to not have to manage my own docker image and update whenever you updated, nor to have to download the static binary on each ci run.

Its kinda nice that shellcheck has docker images that track master. It would require more work than I would want to do if I wanted to replicate. I would need to wire up ifft or something else to watch github's rss feed of this repo and update my docker image.

I'll probably just have my own image and update that with your releases/master from time to time.

@mvdan
Copy link
Owner

mvdan commented Mar 20, 2017

You should not always use shfmt from master :) Same goes for released versions, really. You should stick to a certain version and only update to newer versions after checking everything is still OK.

For example, I might add more simplification rules like the one that converts `foo` to $(foo).

If you're updating to a newer shfmt version manually, building a tiny docker image doesn't keep you from fully automating the process on its own.

@mmlb
Copy link
Author

mmlb commented Mar 20, 2017

flip side is to break things quickly. I'm ok with having shfmt/shellcheck running master and pinning to a version if something breaks or I want some time to come into compliance. I'd rather have the default be run latest and greatest (ahem Arch Linux user here btw) without having to worry about it, until I do where I'd favor fixing the code.

@mvdan
Copy link
Owner

mvdan commented Mar 21, 2017

Sure, but I don't want to encourage that even when I'm an Arch user too ;)

If we're talking about automation of Docker image building, why is it easier to do here than in a separate repo or place? Could you not watch for changes externally?

@mmlb
Copy link
Author

mmlb commented Mar 21, 2017

Its kind of a pain to do so. Docker needs to install hooks into the repo to do the builds, but since its not my repo I don't have perms to do so. The other option (which I started to toy with for something similar) was to hook up ifft or zapier to watch an rss feed and then trigger docker hub. It would watch the rss feed for the master branch for example. Looked doable, but definitely more work (and would be duplicated if anyone else wanted this setup) than you providing.

@mvdan
Copy link
Owner

mvdan commented Mar 21, 2017

Okay then - happy to have it part of this repo as long as you tell me what to do and submit a PR with the Dockerfile.

Also assuming this is about a docker image tracking master, and not tracking released versions. I don't know if you'd want these too, or if they could also be handled automagically.

@mvdan mvdan reopened this Mar 21, 2017
@mmlb
Copy link
Author

mmlb commented Mar 21, 2017

oh cool! Alright I'll work that out. I think both master and tagged release can be doable.

@mvdan mvdan changed the title docker image Official docker images Mar 22, 2017
@mvdan
Copy link
Owner

mvdan commented Apr 11, 2017

Bump @mmlb - do you still plan on working on this?

@mmlb
Copy link
Author

mmlb commented Apr 11, 2017

Hey mvdan, I do plan to do this just a bit swamped at work at the moment. I expect to have some time later in the week or early next.

@mvdan
Copy link
Owner

mvdan commented Apr 12, 2017

That's fine, there is no hurry - just wanted to make sure this wasn't a zombie issue :)

@mvdan mvdan changed the title Official docker images cmd/shfmt: official docker images Apr 23, 2017
@mmlb
Copy link
Author

mmlb commented May 25, 2017

hey @mvdan seems it turned into a zombie issue ;). Turns out the docker image is not quite as useful as I thought because of the way I'm using shfmt in ci:

shfmt -w $files
git diff | (! grep '.')

So unless you'd be ok with adding git to the image then there's no benefit for me. Alternatively, having shfmt -d to produce a diff would be even better IMO.

@mvdan
Copy link
Owner

mvdan commented May 25, 2017

I think adding a diff algorithm would be overkill for the tool. Composable tools and all that :)

It also raises the question as to where to stop. Do we also include something to replace grep? What about diffs that ignore whitespace, like diff -s?

I'm going to close this since, as you say, there's little benefit in an image as things are now. Thanks for your input!

@mvdan mvdan closed this as completed May 25, 2017
@mvdan
Copy link
Owner

mvdan commented May 25, 2017

Ah, I also forgot to say - you can always use shfmt -l $files instead. That will print the files whose current format differs, e.g. which would have a non-empty diff. That's what I use in CI myself, with an echo saying something like run shfmt on these files.

@mmlb
Copy link
Author

mmlb commented May 26, 2017

yeah I may try going with the -l option, but I think it'll be hard to lose the actual "errors" CI sees. I was basing my wish for shfmt -d on existence/parity with gofmt -d.

@mvdan
Copy link
Owner

mvdan commented May 26, 2017

Note, however, that gofmt -d just calls diff, so you'd still be needing to install extra programs in the Docker image.

@mmlb
Copy link
Author

mmlb commented Jun 1, 2017

oh I did not know that. I was thinking about using either golang or alpine, both have diff installed.

@mvdan
Copy link
Owner

mvdan commented Jun 1, 2017

Requiring the image to have diff is not very different from requiring it to have git :)

@mmlb
Copy link
Author

mmlb commented Jun 1, 2017

Not quite, diff already comes in alpine and golang so I don't have to do anything :D. And shfmt image should not have anything to do with the scm used for the files (if any even), otherwise I'd need to add bzr, git, hg, svn (and detect which to use 🤢 )

In the typical CI setup, source has already been fetched, and I think its better to have slim images instead of fat one ala travis.

@jamesmstone
Copy link
Contributor

sorry, going back to advantages over providing an official binary, I see a docker image more like providing an official package. Which I believe you do.
My preference is to have an official Docker image :D

-- sincerely a non arch user 😱

@fkromer
Copy link

fkromer commented Feb 27, 2018

@jamesmstone Thanks for your docker image jamesmstone/shfmt 👍 .

@mvdan
Copy link
Owner

mvdan commented Nov 20, 2018

Seems clear to me that users want this, so I'm fine with having an official image as long as it can be built and pushed automatically via https://docs.docker.com/docker-hub/builds/.

Assuming that obtaining https://hub.docker.com/_/shfmt/ would be difficult, we can just go with https://hub.docker.com/r/mvdan/shfmt/ for now.

@PeterDaveHello I'll take some ideas from your Dockerfile, since you seem to have the latest version, if that's ok.

@mvdan mvdan reopened this Nov 20, 2018
@PeterDaveHello
Copy link
Contributor

Here is my Docker image repository if anyone likes to try: peterdavehello/shfmt, which is built automatically on Docker Hub, and here is its GitHub repository: PeterDaveHello/dockerized-shfmt.

@mvdan I'm also a maintainer of the official node image, so I'd like to help make an official shfmt image if it's possible, just need to tell you that the official repository is not using automatic build, it needs pull request and review everytime to have a new release, not sure this is 100% you want, I can also invite/add you as collaborator of my Docker repository if it's easier for you, what do you think?

@mvdan
Copy link
Owner

mvdan commented Nov 20, 2018

so I'd like to help make an official shfmt image if it's possible

How easy is it to obtain an official repository?

it needs pull request and review everytime to have a new release

I'm curious - why is that? I'd be fine with a bit of overhead per release, if that means it can be an "official" Docker repository. But I don't think it's a dealbreaker, as I think automatic builds from the upstream git repo to a docker hub repository owned by the author should be good enough.

I can also invite/add you as collaborator of my Docker repository if it's easier for you

Thanks for the offer! I think it will be best if we keep the Dockerfile somewhere in this repository, to tie it with the rest of the upstream code. Unless there's a reason to specifically put it in a separate repo?

@PeterDaveHello
Copy link
Contributor

PeterDaveHello commented Nov 20, 2018

How easy is it to obtain an official repository?

I haven't done that yet, I'm not the first maintainer of node's Docker image.

I'm curious - why is that? I'd be fine with a bit of overhead per release, if that means it can be an "official" Docker repository. But I don't think it's a dealbreaker, as I think automatic builds from the upstream git repo to a docker hub repository owned by the author should be good enough.

I don't have a certain answer, maybe it's just to make sure its quality.

Thanks for the offer! I think it will be best if we keep the Dockerfile somewhere in this repository, to tie it with the rest of the upstream code. Unless there's a reason to specifically put it in a separate repo?

To have an official Docker hub repository may need a separated one, as there are also some scripts and Dockerfile related files being developed and they are not directly related with the application development. If you don't mind, I can try to make my original repository to be the source repository of it.

@mvdan
Copy link
Owner

mvdan commented Nov 20, 2018

some scripts and Dockerfile related files being developed and they are not directly related

Ah, I've looked at the docker-node repository and I agree that the file and directory structure would not fit well in this repository.

I can try to make my original repository to be the source repository of it

Sure, feel free to give it a go. I'll probably publish mvdan/shfmt as an automated build in the meantime, as we don't yet know if an official image will be possible.

mvdan added a commit that referenced this issue Nov 20, 2018
Pinning versions of the builder and final base image, so that its
behavior doesn't change over time.

Updates #68.
@mvdan mvdan closed this as completed in 6300770 Nov 22, 2018
@nemchik
Copy link

nemchik commented Nov 30, 2018

I wanted to point out a few things about 3f0ab69

I'm very excited to see you're working towards having an official docker image! I noticed https://hub.docker.com/r/peterdavehello/shfmt/tags/ seems to also have v3 alpha which is cool, and would be cool to see in an official capacity. More importantly though, there is no latest tag on https://hub.docker.com/r/mvdan/shfmt/tags/ so the docker image cannot currently be used by name without a tag (the tag must be specified).

Also to compare to @jamesmstone image I am able to run

docker run --rm -v ${VALIDATIONPATH}:${VALIDATIONPATH} jamesmstone/shfmt ${VALIDATIONPATH}/file.sh

Notice there is no tag (because latest is implied) and the image name seems to imply the use of the shfmt command as well.

By contrast this is what I have working with your official image

docker run --rm -v ${VALIDATIONPATH}:${VALIDATIONPATH} mvdan/shfmt:v2.6.1 shfmt ${VALIDATIONPATH}/file.sh

Note that the version tag was required in this case, and then also the command had to be included. The command being required isn't a huge issue, but it would be nice if it were not required as this is similar to many other images for tools like this.

Here is my specific usage for reference
https://github.com/nemchik/ShellSuite/blob/master/shellsuite.sh#L104-L116

Thanks!

@mvdan
Copy link
Owner

mvdan commented Nov 30, 2018

also have v3 alpha which is cool, and would be cool to see in an official capacity

v3's alpha0 was really just to kickstart the v3 Go module, it doesn't include anything else as a release. I'm not yet sure if I'll publish future alphas on Docker Hub, but I'd publish betas there for sure.

there is no latest

That was kind of on purpose. Seems to me like a "tag" that just follows the latest version isn't particularly useful in the long run. For example, once v3 is final, that might introduce breaking changes, meaning that latest might break some scripts or programs out there.

I realise this is all "use at your own risk", but I don't want to encourage that kind of usage. I'd prefer doing something like a v2 tag, which wouldn't have that problem. But then you lose the ability to leave out the tag. I also don't know if this could be automated.

Another option would be to have latest always be master. I guess that would more consistently give you an unstable version, but I wonder how useful that would be.

the command had to be included

I was thinking about this for a while, actually. This repository can hold multiple binaries - for example, there's a gosh one already. So I was trying to make it so that I can stick multiple binaries into that one Docker image.

Of course, the issue then is that mvdan/shfmt shfmt ... is a bit redundant. I could name the image mvdan/sh to reduce that problem, but it seems to me like "sh" as an image basename is too generic.

I think the packaging of shfmt has already settled on too many systems, so I think I'll have to stick with just using its name for package and image names. So that Docker image will likely only contain the one binary in the long run too. In which case I'm fine with dropping the requirement for the command name. I copied that design bit from @PeterDaveHello, so perhaps he has an opinion.

@nemchik
Copy link

nemchik commented Nov 30, 2018

I would think if you wanted to run gosh you would find an image built just for it, as I would expect the mvdan/shfmt image to be just for shfmt. Similarly koalaman/shellcheck is only for shellcheck and does not require to be run as koalaman/shellcheck shellcheck. The same is true for a number of others I use.

I would be all for the latest tag being master. The idea there would hopefully be that if master is updated and there are now new rules or breaking changes that would be the point when people would either adjust their builds with new arguments to meet what they need or specify a tag to keep them behind intentionally. I'm all for people staying current though. The same would happen if shfmt were in apt, dnf, or yum (the version would be upgraded when it happens in the repo) [granted I know it's not installed via those package managers, but I used that as an example to illustrate the point].

I still feel there's validity to versions being tagged, especially past versions, but also dev versions like v3 while it's in alpha. This gives people the ability to pin themselves to a specific build if they need or try future builds before they become latest. That's one of the many beauties of Docker actually.

@mvdan
Copy link
Owner

mvdan commented Dec 7, 2018

I'll update the Dockerfile to add shfmt as the entrypoint, thanks. Reopening so that I don't forget. I'll leave the already published image as-is, to not break anything.

I'll also look at adding a latest tag to track master. Of course, backwards and forwards compatibility will not be guaranteed with that tag.

@mvdan mvdan reopened this Dec 7, 2018
mvdan added a commit that referenced this issue Dec 8, 2018
Now the Dockerfile is version agnostic, so it can be used to build a
master version as well as any tagged version.

It also uses COPY instead of a git clone, so it's much faster and no
longer requires an internet connection.

These changes mean that we can no longer use Docker Hub's automated
builds. Which is fine, because they were painfully slow anyway.

While at it, add an ENTRYPOINT as suggested in #68.
@mvdan mvdan closed this as completed in a9fbef1 Dec 8, 2018
@mvdan
Copy link
Owner

mvdan commented Dec 8, 2018

Both fixed:

$ docker run mvdan/shfmt:latest -h
usage: shfmt [flags] [path ...]

@mvdan
Copy link
Owner

mvdan commented Dec 8, 2018

On second thought - I had forgotten that Travis secrets currently break their Windows builds.

Testing on Windows is vastly more important than pushing latest images, so I'm going to disable those docker builds until they can resolve this issue. See https://travis-ci.community/t/current-known-issues-please-read-this-before-posting-a-new-topic/264.

@mvdan mvdan reopened this Dec 8, 2018
@nemchik
Copy link

nemchik commented Dec 8, 2018

So for the moment we can use docker run mvdan/shfmt:v2.6.2 -h since you've got the entrypoint, but avoid latest until we see movement here?

@mvdan
Copy link
Owner

mvdan commented Dec 8, 2018

You can use either v2.6.2 or latest for now. The secrets issue on Travis is the biggest remaining issue with their Windows setup, so I'd expect that it will be fixed within the next weeks, likely before we ever do a v2.6.3 release.

@nemchik
Copy link

nemchik commented Dec 8, 2018

My builds look good, thanks for the updates!

@vittorius
Copy link

@mvdan Hey! What do you think about having 2 images, similarly to shellcheck approach:

  • binary-only image (example)
  • shell-based image providing the binary (example)

The reason for this request is that some CI tools (for me it's GitLab Runner) require the Docker images to have a shell installed in order to execute the script CI file entry.

@mvdan
Copy link
Owner

mvdan commented Jul 2, 2019

That's a bit of a niche requirement :) I guess I'm fine with it as long as it's automated. I'll take a look once v3 is released and once I've moved off of Travis.

@greenled
Copy link

@vittorius meanwhile you could use something like this:

shellfmt:
  image: docker:stable
  services:
    - docker:dind
  script:
    - docker run mvdan/shfmt:v2.6.4 --version
    - docker run -v $(pwd)/psu:/psu mvdan/shfmt:v2.6.4 -i 2 -ci -l psu

This DIND approach only works in CI products where you can actually use DIND. For example, it works in GitLab CI and Circle CI, but not in Drone CI.

Having an official image with a shell, as you propose, would be more efficient and more portable.

@mvdan
Copy link
Owner

mvdan commented Dec 16, 2019

Now that v3.0.0 is out, we're back with proper release and master images. Each tagged version is the same as the git tag here, and latest corresponds to master.

@vittorius the current image is based on busybox, which should contain a POSIX shell. I've just verified that with docker run -it --entrypoint=/bin/sh mvdan/shfmt:v3.0.0. If that's not enough for people, I can consider splitting the image into v3.0.0 and v3.0.0-alpine, where the former is from scratch, and the latter is based on a real distro.

@mvdan
Copy link
Owner

mvdan commented Feb 18, 2020

For what it's worth, once v3.0.2 is out, the -alpine image will be available. It's already available for master as latest-alpine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants