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

#111 Pass tool version to dockerfile #112

Merged
merged 9 commits into from
Jan 21, 2025

Conversation

kkazmierczyk
Copy link
Collaborator

@kkazmierczyk kkazmierczyk commented Jan 16, 2025

Fixes #111.
See Contributing.md how to test it.

The patch is built on top of #110 .

@kkazmierczyk kkazmierczyk added enhancement New feature or request good first issue Good for newcomers labels Jan 16, 2025
@kkazmierczyk kkazmierczyk changed the title 111--pass-tool-version-to-dockerfile #111 - Pass tool version to dockerfile Jan 16, 2025
@kkazmierczyk kkazmierczyk changed the title #111 - Pass tool version to dockerfile #111 Pass tool version to dockerfile Jan 16, 2025
Signed-off-by: Krzysztof Kaźmierczyk <[email protected]>
@tjanasiewicz
Copy link
Collaborator

@kkazmierczyk is there a reason why you used CMD (to run the analyzer) before using RUN to build and configure the image?
https://www.docker.com/blog/docker-best-practices-choosing-between-run-cmd-and-entrypoint/

@kkazmierczyk
Copy link
Collaborator Author

kkazmierczyk commented Jan 17, 2025

@kkazmierczyk is there a reason why you used CMD (to run the analyzer) before using RUN to build and configure the image? https://www.docker.com/blog/docker-best-practices-choosing-between-run-cmd-and-entrypoint/

@tjanasiewicz are you asking why I used CMD and RUN instead of different ones or rather why in that order?

I agree that ENTRYPOINT is better than CMD. I have corrected it already in 0c6dc8f

Order of the commands:
Basically for dockerfile the instructions are not run in the order how they are written in the file. But the order matters when you modify the file. If you make any modification in the file, then all the layers since given line must be rebuilt. Therefore it is better to put more frequently modified lines on the bottom of the file to allow more caching.

More about the recommendations:
https://medium.com/@esotericmeans/optimizing-your-dockerfile-dc4b7b527756
https://docs.docker.com/build/building/best-practices/

@kkazmierczyk
Copy link
Collaborator Author

After further reading of https://docs.docker.com/build/building/best-practices/#entrypoint and https://www.docker.com/blog/docker-best-practices-choosing-between-run-cmd-and-entrypoint/ I believe that CMD is more applicable than ENTRYPOINT. Setting back line 17 to CMD.

@tjanasiewicz
Copy link
Collaborator

tjanasiewicz commented Jan 20, 2025

@kkazmierczyk is there a reason why you used CMD (to run the analyzer) before using RUN to build and configure the image? https://www.docker.com/blog/docker-best-practices-choosing-between-run-cmd-and-entrypoint/

@tjanasiewicz are you asking why I used CMD and RUN instead of different ones or rather why in that order?

I agree that ENTRYPOINT is better than CMD. I have corrected it already in 0c6dc8f

Order of the commands: Basically for dockerfile the instructions are not run in the order how they are written in the file. But the order matters when you modify the file. If you make any modification in the file, then all the layers since given line must be rebuilt. Therefore it is better to put more frequently modified lines on the bottom of the file to allow more caching.

More about the recommendations: https://medium.com/@esotericmeans/optimizing-your-dockerfile-dc4b7b527756 https://docs.docker.com/build/building/best-practices/

I am asking because from what I understand and what I found in the shared documentation those commands (RUN, CMD) are for different purposes [1]:
a) RUN:
is an image build step, the state of the container after a RUN command will be committed to the container image. A Dockerfile can have many RUN steps that layer on top of one another to build the image.

b) CMD [2]:
is the command the container executes by default when you launch the built image. A Dockerfile will only use the final CMD defined. The CMD can be overridden when starting a container with docker run $image $other_command.

Also, the example you referred [3] shows the same order. It means first build using RUN (add all references), and after that execute using CMD.

[1] https://docs.docker.com/reference/dockerfile/#run
[2] https://docs.docker.com/reference/dockerfile/#cmd
[3] https://medium.com/@esotericmeans/optimizing-your-dockerfile-dc4b7b527756

Signed-off-by: Krzysztof Kaźmierczyk <[email protected]>
@kkazmierczyk
Copy link
Collaborator Author

@kkazmierczyk is there a reason why you used CMD (to run the analyzer) before using RUN to build and configure the image? https://www.docker.com/blog/docker-best-practices-choosing-between-run-cmd-and-entrypoint/

@tjanasiewicz are you asking why I used CMD and RUN instead of different ones or rather why in that order?
I agree that ENTRYPOINT is better than CMD. I have corrected it already in 0c6dc8f
Order of the commands: Basically for dockerfile the instructions are not run in the order how they are written in the file. But the order matters when you modify the file. If you make any modification in the file, then all the layers since given line must be rebuilt. Therefore it is better to put more frequently modified lines on the bottom of the file to allow more caching.
More about the recommendations: https://medium.com/@esotericmeans/optimizing-your-dockerfile-dc4b7b527756 https://docs.docker.com/build/building/best-practices/

I am asking because from what I understand and what I found in the shared documentation those commands (RUN, CMD) are for different purposes [1]: a) RUN: is an image build step, the state of the container after a RUN command will be committed to the container image. A Dockerfile can have many RUN steps that layer on top of one another to build the image.

b) CMD [2]: is the command the container executes by default when you launch the built image. A Dockerfile will only use the final CMD defined. The CMD can be overridden when starting a container with docker run $image $other_command.

Also, the example you referred [3] shows the same order. It means first build using RUN (add all references), and after that execute using CMD.

[1] https://docs.docker.com/reference/dockerfile/#run [2] https://docs.docker.com/reference/dockerfile/#cmd [3] https://medium.com/@esotericmeans/optimizing-your-dockerfile-dc4b7b527756

The first two links are references to CMD and RUN instructions. I didn't find any information about order. The 3rd one is pretty interesting. It mentions:
#1 Place static instructions higher in the order. Instructions like, but not limited to, EXPOSE, VOLUME, CMD, ENTRYPOINT, and WORDIR whose value is not going to change once it is set.
And the first example shows CMD before RUN. But the next example on that page shows another order.

IMO, the profit from placing CMD before RUN is tiny (CMD layer has 0B if I run podman history javacore-analyser). We can keep the order to have CMD at the end of the file.

I rolled it back.

@tjanasiewicz tjanasiewicz merged commit 77896aa into main Jan 21, 2025
6 checks passed
@tjanasiewicz tjanasiewicz deleted the 111--pass-tool-version-to-dockerfile branch January 21, 2025 08:25
kkazmierczyk added a commit that referenced this pull request Jan 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pass tool version to dockerfile
2 participants