-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Change owner while copy #7552
Change owner while copy #7552
Conversation
Copy all files and chown them in a 2nd layer leads to a larger image. See layer 22 and 26 of https://hub.docker.com/layers/snipe/snipe-it/v4.7.8/images/sha256-67c865d91df1b90cef1112f12bbc9c64402dfeafde0bdb160c4f07e785ee0bcc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small question, but once that's answered it should be good to go! Thanks for the change!
Dockerfile
Outdated
@@ -62,8 +62,6 @@ WORKDIR /var/www/html | |||
# COPY docker/*.php /var/www/html/app/config/production/ | |||
COPY docker/docker.env /var/www/html/.env |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems great! Would we need to also do it here, though? The later chown
seems like it would grab this .env
file as well?
Let me know about that (or add it, if needed) and I'll get it approved for you right away!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add a commit to the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi there! Would you still be able to make this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent; looks great to me! Thank you!
If you like, I could refactor the whole Dockerfile, e.g. use multistage build to get rid of all the composer dependencies (git, svn, ...) in the final image. |
I think that would be great, after version 5.0 is released - I'd love to shrink that all down, it'll make everyone much happier. |
Cool! After the merge of this PR, I'll look at it. |
Congrats on merging your first pull request! 🎉🎉🎉 |
CRAAAP. I didn't realize this PR was against master instead of develop. :( :( |
Hi! Should I change something? |
Nah, it's already merged - I just gotta make sure it doesn't get eaten in the develop -> master merge :) |
Cool! If I find some time, I'll have a look at the Dockerfiles for some more optimizations. |
* Change owner while copy Copy all files and chown them in a 2nd layer leads to a larger image. See layer 22 and 26 of https://hub.docker.com/layers/snipe/snipe-it/v4.7.8/images/sha256-67c865d91df1b90cef1112f12bbc9c64402dfeafde0bdb160c4f07e785ee0bcc * Copy docker.env as user docker
Copy all files and chown them in a 2nd layer leads to a larger image.
See layer 22 and 26 of https://hub.docker.com/layers/snipe/snipe-it/v4.7.8/images/sha256-67c865d91df1b90cef1112f12bbc9c64402dfeafde0bdb160c4f07e785ee0bcc