semver-tool icon indicating copy to clipboard operation
semver-tool copied to clipboard

feat: wrap the script in a Docker container to simplify its usage

Open mdelapenya opened this issue 2 years ago • 3 comments

What does this PR does?

It adds a Dockerfile and a few Make goals to build the docker image that wraps the execution of the shell script, making it possible to run it in a cross-platform manner.

How to test it?

Simply running the following command will do the trick:

make build
docker run --rm docker.io/fsaintjacques/semver-tool:3.4.0 bump minor "1.1.0"

It should produce the 1.2.0 output

Follow-ups

Not sure how you want to release the Docker image, that's why I'm not adding anything to the Github action. Probably a dedicated one listening for tags, using secrets for the Docker user/password?

mdelapenya avatar Feb 15 '23 17:02 mdelapenya

I have to admit that I created this code in May 2018, waiting since there in my laptop 🤦

mdelapenya avatar Feb 15 '23 17:02 mdelapenya

Thanks for the contribution! I've been thinking about it since it landed in my inbox. Now I have a bit of time to comment.

Note: I will send a second comment regarding code specifics. This comment addresses the more general question.

The question: "What problem does this PR (having a Docker image/executable of semver) solve?"

The PR comment hints at one problem: ". . . making it possible to run it in a cross-platform manner." My first thought was "Oh, yeah, Windows". For *nix, it's not an issue. For Mac's, it's true that Apple is moving away from Bash because it can't use the latest version (licenses?) .. but Bash is still there and will remain there long into the future (and the shebang will continue to work without any user config for default shells, etc.). Besides Windows, there are various tools/frameworks--especially CI--that allow commands to be plugged-in. These can be rather tricky!

Are there other advantages? Problems solved? Send them in!

So, Windows and frameworks. For frameworks, my guess is that trying to run via a Docker engine is going to cause more problems than running a simple command. And if the framework is run under Docker? I guess that Docker-in-Docker works these days, but this sounds more like "because we can" rather than "makes life easier". For Windows interactive use and when not using some "feels like unix" environment (and with Docker set up): I guess this would work (but see below). For Windows non-interactive and not in a framework (Powershell scripting?): running via Docker might be a use case. But I'd like to see an example!

But take a step back: semver has no real use except in automation: mostly CI/CD. Interactive use is needed only to debug shell pipelines or whatnot. Nobody types in semver get major 1.2.1 to discover that "1" in the major version. So, when does having a lightweight/standalone Docker image with semver installed in /usr/local/bin help? When (i.e. what sort of environment) would semver relying on Bash be a problem, when, the rest of environment wouldn't be a problem? Like, what shell are you using to invoke semver or docker run ... Don't say Bash!

I could be missing something! I'd be very interested! I'd learn something because I obviously don't see it now!

As well, adding the Docker image in the build adds a fair amount of complexity in terms of maintenance. And, two release variants to manage: github and Dockerhup. The Makefile is 20% larger and we have a new Dockerfile. And a new account on Dockerhub to maintain.

Docker is great for baking in the dependencies at a specific version and reducing the risk of the specifics of a particular host config breaking things. So, it is seductive to think that "docker run ... :3.4.0" insures we are running the specific version of semver we need (for repeatable builds). However, which version of Bash? Which version of alpine? Note you can always install semver as semver-3.4.0 if you want to. I think it better to pay attention to compatibility upstream rather than relying on version specifics: that's what semver has done (we could do more: testing against different Bash versions!). There is a certain promise that semver will work with any Bash version that you'd be likely to find. I think it better to maintain this rather than spend effort insuring it works with whatever Docker version or configuration or dockerhub setup might emerge.

In short, without a convincing use case, I'd not be in favor of accepting the PR.

ranger6 avatar Feb 28 '23 09:02 ranger6

Now, a comment about specifics and details of this PR. See previous comment regarding the bigger picture.

  1. I really appreciate the details of the build with identifying metadata.
  2. I don't understand why CMD and ENTRYPOINT are defined, with CMD being a zero length string.
  3. The Makefile has an extra quote on line 41. make push fails.
  4. Defining the DOCKER_PASSWORD in the environment seems awkward and insecure. No way to get prompted for interactive use and for scripts, the PW needs to be somewhere else so the env. variable can be set. It's always a tricky problem.
  5. The Docker image does not have a "latest" tag: you can't run it without knowing the version unless you do a push. The README file example fails without a push.
  6. The README file example fails always: the ENTRYPOINT already define running semver, so "semver --version" results in trying to run "semver semver --version"
  7. There are no tests provided. If there were tests, perhaps the above bugs would not have been included in the PR!
  8. Is "Manuel de la Peña" really the proposed MAINTAINER?
  9. The version of semver is defined with the image, but not the versions of other artifacts: alpine, bash, docker ... This is generally not a problem, but if one idea is to use Docker to nail down config and get repeatable behaviour, then one should go all the way. E.g. using "latest" in stable builds is not a good idea.
  10. It's true that you don't want a push on each build and the plan needs to be worked out, but once build artifacts are being pushed to other places, then the build process for this needs to be nailed down. We could still use a well defined (i.e. scripted) process for the github releases!

ranger6 avatar Feb 28 '23 10:02 ranger6