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

Use of wget to pull source code in Dockerfile #145

Open
geraldwuhoo opened this issue Feb 9, 2022 · 3 comments
Open

Use of wget to pull source code in Dockerfile #145

geraldwuhoo opened this issue Feb 9, 2022 · 3 comments

Comments

@geraldwuhoo
Copy link

In the Dockerfile, use of wget to get the latest release from the GitHub releases is pretty strange, and IMO an anti-pattern. IMO, the Dockerfile should be put in the root of the project directory, and copy the source code directly from the project with COPY . ., instead of pulling the latest release from the web.

Imagine a case where someone wishes for whatever reason to build a Docker image for an older version of agate. They would download the desired release from the releases page, and then build the Dockerfile. The Dockerfile would query the Internet for the latest release, and then download it. They would get the latest release of agate, not the version they specifically cloned to build! This would be solved by the proposed change.

Additionally, imagine a case where someone wishes to build a Docker image for a "development" version of the code straight from the latest git commit, rather than the latest stable release. The above case would apply again.

This change would also futureproof against a possible future CI pipeline, in which the Docker image may be automatically built. This should certainly be built against the current state of the code, not whatever the latest stable release on GitHub is.

I am more than happy to open a PR with the above proposed changes, should the primary developer(s) agree with the sentiment. Open to discussion as well.

@Johann150
Copy link
Collaborator

Please note the tools/ directory is mostly contributed by others. This also applies to the Dockerfile. Since I have neither experience with nor interest to get to know Dockerfiles I can only do a cursory review anyway (see also last paragraph of MIT license). Your description seems sound so a PR would be welcome. 👍

@geraldwuhoo
Copy link
Author

I see, the tools directory is more like a contrib directory. Unfortunately, Dockerfiles only have context into their working directory, and not any parent directories, which means the above proposed change would not work from within tools.

It may be inappropriate to move the Dockerfile out to the root of the project, if it is meant to be contained in an effectively community-maintained contrib directory. I am willing to maintain this Dockerfile in the root of the project, but I can understand if you are hesitant towards this change if you (and the other primary developers) are not familiar with Docker. If you're fine with this change, I can update my existing PR.

@Johann150
Copy link
Collaborator

Its fine to put the Dockerfile in the root directory.

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

No branches or pull requests

2 participants