Skip to content

Adds step in Dockerfile to include freedesktop.org.xml #35

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

Closed
wants to merge 1 commit into from

Conversation

PhilCoggins
Copy link
Contributor

@PhilCoggins PhilCoggins commented Mar 25, 2021

👋 I saw the terraforming-rails talk in person, and more recently the Ruby on Whales blogpost has been crucial in helping me setup Docker at my company, thank you!

I wanted to give back, the Dockerfile used in the blogpost now requires freedesktop.org.xml to install the mimemagic gem as the shared-mime-info package is not installed on slim Docker images. It would be great to see a corresponding blog post update to help future Rails devs configure Docker for their Rails app without the mime headaches.

See also: docker-library/ruby#344

* This is now required to install the mimemagic gem, which is a transitive dependency of Active Storage.
&& apt-get clean \
&& rm -rf /var/cache/apt/archives/* \
&& rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* \
&& truncate -s 0 /var/log/*log

RUN cp /usr/share/mime/packages/freedesktop.org.xml ./ \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This step is excess because of image size will not be reduced at the end.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.

We should either move the first cp and the apt-get remove commands to the RUN above or add an independent RUN apt-get update ..... apt-get clean ... just for this functionality.

I prefer the second approach.

@bibendi @PhilCoggins Thoughts?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to just install the package and don't do any actions after :)

By the way, is this fix needed after the recent fixes at active-storage's dependencies?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, is this fix needed after the recent fixes at active-storage's dependencies?

Not sure, but I just found that my app doesn't work without it 😁

@palkan
Copy link
Member

palkan commented Mar 30, 2021

Merged 525e58d

Thanks!

@palkan palkan closed this Mar 30, 2021
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

Successfully merging this pull request may close these issues.

3 participants