It looks like we don't compute the project version using this line when building via Docker, so the validator version is always 0.1.0-SNAPSHOT.

This makes the ENTRYPOINT stable and works well for local testing, but creates a problem for the ones using the actual docker image. This implementation doesn't allow users to know the real validator version, which is usually displayed in the reports, which I think is very valuable. Any thoughts on this @isabelle-dr?

If the shadow jar was created using the correct project version, we could use the file name in the ENTRYPOINT. @bdferris Any thoughts on this? Do you have an idea why the axion-release plugin would not work here?

I believe if you update the actions/checkout@v2 action in the docker.yml file to include tag info then the version number will be computed correctly. See https://github.com/MobilityData/gtfs-validator/blob/master/.github/workflows/package_installers.yml#L33 for an example.

Thanks, that's helpful. Do you know if there is a way to do this locally as well? Currently, building with Docker does not calculate the version. We wouldn't need to hardcode the jar file name if it did.

Assuming I understand your question correctly, local builds should typically work by default, because you are typically working from a full Git repo where all tag info is already present.

That's what I thought too. Currently, when I run this command

docker build . -t gtfs-validator:latest

I end up building a shadow jar named gtfs-validator-0.1.0-SNAPSHOT-cli.jar, which seems also to be the case on Chris' end since the entry point was hardcoded as such. Though, by running ./gradlew shadowJar --no-daemon locally, the jar file is built with the correct tag version.

Ideally, we would create an entry point using the tag detected locally and on GitHub. So I'm looking to find out why the axion-release plugin is not detected in the docker build locally. So far, I have had no luck with the documentation of the axion-release plugin on what might be the issue.

@maximearmstrong my original version of this PR replicated what the GitHub workflow for building docker did, passing in $VERSION_TAG and using that to generate a version-infused entrypoint.

I removed that in 6aa301a because the jar build process stopped respecting the $versionTag input and started building gtfs-validator-0.1.0-SNAPSHOT-cli.jar regardless of input.

I found that b93dc1b, which got merged after I started my PR, removed that functionality so I updated this branch accordingly. We can just revert 6aa301a if the preference is to restore $versionTag

If the desire is to make the versions be driven by Git, I recommend chaining builds off of GitHub's release event and using the release metadata to generate and label the build (rather than driving it off raw Git tags or branch pushes)

Apologies, I'm new to Docker, took me a second to figure out what's going on.

1. Why wasn't the Docker build resolving the version number correctly in local builds? Recall that the axion-release plugin uses Git tags in the cloned repo to resolve the current version. Since .git was included in the .dockerignore file, none of the Git repro metadata was being copied over with the Docker build context (aka the COPY --chown=gradle:gradle . /build directive). By removing .git from .dockerignore, I was able to build local Docker images that correctly resolved the release version number. @themightychris did you add .git to .dockerignore for a specific reason? I couldn't think of any reasons, but it doesn't mean they don't exist :)

2. The upshot of (1) is that gradle is now building a jar like gtfs-validator-3.1.1-SNAPSHOT-cli.jar. How to specify the entry point now that the version number is no longer fixed? I'd use something like the following:

COPY --from=build /build/main/build/libs/gtfs-validator-*-cli.jar /gtfs-validator-cli.jar
...
ENTRYPOINT [ "java", "-jar", "gtfs-validator-cli.jar" ]

Here, we are copying the gtfs-validator-VERSION-cli.jar (@themightychris note how I didn't copy *.jar, since that picks up main-VERSION.jar as well) and rename it to a file without a version number. That way we can get a consistent reference in the ENTRYPOINT specifier.

I've verified the combo of (1) and (2) works for me on local Docker builds. I think it should also work for the docker.yml release builds, assuming we make the action/checkout tweak I mentioned.

I'll admit that I'm not entirely sure what would happen if you attempted docker build https://github.com/MobilityData/gtfs-validator.git#some_tag. Namely, I'm not sure if Docker will preserve enough git metadata to let version resolution work. Worse-case scenario, it'll fail to resolve the version number, it will default back to 0.1.0-SNAPSHOT, but the ENTRYPOINT should still work correctly.

Thanks a lot @bdferris-v2! I tested it locally and it works for me too. The correct version of the validator is displayed in the report. @themightychris If you can do these changes, we will be ready to merge.

@bdferris-v2 It's a bit of a bad practice to pull .git/ into the Docker build context—it can lead to extra or even sensitive content accidentally getting built into the container, and it's a criss-crossing of concerns to have your Dockerfile build process using a git client and knowledge of your release process to figure things out about the release itself.

A preferred approach is to let the system invoking the build determine the designated version and pass that in via a --build-arg. We can see that workflow happening previously in 6aa301a when gradle shadowJar could accept versionTag as an environment variable.

If I understand correctly, a plugin being used within the gradle workflow is handling extracting the current version from a local .git/ repo? If that's the case, it would be ideal if that could be made to be overridden environmentally, such that a CI system can pass one in and deactivate the build process trying to inspect .git/—Would that be difficult? That's a less quick fix, but this sort of separation of build and release concerns is important for anyone who needs to run their own builds and keep their forks minimal. And it's fine if the gradle build layer has some smarts when its run directly as long as they can be overridden in CI via environment

High-level: I hear what you are saying. That said, I do wonder if we'll just be introducing complexity elsewhere for the sake of purity in the Docker file. But more on that later.

For context, the versioning changes were relatively recent in b93dc1b to better support X.Y.Z version numbers, as used in app packaging, version checks, etc. This moved us away from the previous scheme of extracting version info from $GITHUB_REF in GitHub workflows. One nice by-product of this change is that version info was now correctly computed for local builds as well.

So how would we make this work in a Docker build without access to .git?

First, it is possible to override the version from the Gradle command-line. Per the axion-release plugin docs, you can force the version on the command-line in the following way:

./gradlew -Prelease.forceVersion=3.0.0

Second, it's possible to output the current version as deduced from .git using the Gradle currentVersion command as well:

./gradlew cV -q -Prelease.quiet

So put that all together:

1. In the docker workflow, we'd compute the current version as described above.
2. We'd pass that as a --build-arg to the Docker build context.
3. We'd pass that further into the actual gradle build to override the version.

Kind of roundabout, but I think it technically works? My only question is if it's possible to gracefully fallback for local docker build such that the user doesn't have to remember to manually pass in the version argument.

The release version in Gradle is now expected to be the form X.Y.Z (with an optional -SNAPSHOT suffix). I think it would be best if you got rid of version resolution via $GITHUB_REF and use the ./gradle currentVersion mechanism I described in my previous comment.

I've seen approaches like this run into problems in the past: developers are free to push tags without any restrictions GitHub can implement, and it's nondeterministic which of multiple tags for the current HEAD such client-side tooling will select. When it comes to GitHub actions being triggered by the release events though, the maintainer creating the GitHub release selected a specific tag to use/create and GitHub is providing that to the workflow as a singular input via GITHUB_REF. The real-world failure case here is developers pushing pre-release tags for testing, and then the GitHub release flow arbitrarily picking one of those to tag a release with rather than what the maintainer creating the official release tag selected.

Besides, this was already how the current workflow was sourcing the Docker image tag to apply and these changes are just also routing that same designation into the container build. So now they're both chained off the same input and it would be a broader scope of change to source that version designation differently. @bdferris-v2 I'd be happy to take that change on within this PR though if that's still preferred given the gotcha I described above.

The one substantial change I DID make here was dropping the v prefix when reading the Git tag. While it is a git convention to prefix version number tags with v, the convention with Docker image tags is to drop it. The -Prelease.forceVersion= option was also expecting it without the v prefix, so I did it upstream here to drive both the Docker tag and the Gradle version. So as of right now if this got merged, future docker container image publishings would drop thev—which seems like a good move to me this early in publishing one to bring in line with convention.

I wouldn't describe it as non-deterministic. The axios-release-plugin has a defined strategy for resolving the current version from the Git commit history. It's smart enough to ignore arbitrary tags like i-love-tacos but it is true that it will complain if you create a version tag like v1.tacos.

So I'm agreed that this requires us to use consistent vX.Y.Z version tags when tagging releases, but that doesn't seem too terrible. More importantly, this wasn't introduced arbitrarily. The jpackage app packaging phase specifically fails if passed a non-X.Y.Z version number, which is what led to this upstream change in the first place. We are going to need some consistency in our version tags one way or another. If there are other approaches that play well with the free-wheeling nature of Git versioning, I'm interested in learning more about them.

But if we stick with what we've got, then I'd argue that you should drop the code to resolve the version from GITHUB_REF and instead use the currentVersion approach I discussed earlier so that we get consistent version numbers whether we are building from a release tag, a PR, etc. I'll admit it's a hack (thus my inclination to just include the .git metadata in the Docker build itself, but I digress).

Unfortunately, while playing around with forceVersion behavior, I discovered that if you pass in 1.2.3-SNAPSHOT as the forced version, the plugin will happily append an additional -SNAPSHOT to make the version 1.2.3-SNAPSHOT-SNAPSHOT. So there's probably an argument for making the version resolution command:

$(./gradlew cV -q -Prelease.quiet | sed 's/-SNAPSHOT//')