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

Generate output files under configurable ID, e.g. as the current user #334

Merged
merged 2 commits into from
Apr 5, 2021

Conversation

davebaird
Copy link
Contributor

@davebaird davebaird commented Apr 4, 2021

Currently, the docker image generates output under the ID of the marp user created inside the image at build time. Usually this will be different from the current user, so it fails unless writing to a world writable location. Also, the file generated has a different owner from the current user.

This PR allows the user to specify a different ID to generate the output.

I suggest adding to the instructions on docker hub:

# Convert slide deck into HTML, specifying ownership of output file
docker run --rm -v $PWD:/home/marp/app/ -e LANG=$LANG -e MARPID="$(id -u):$(id -g)" marpteam/marp-cli slide-deck.md

Thanks for an excellent product, it's a lot of fun to use!

@yhatt yhatt self-requested a review April 4, 2021 20:24
Copy link
Member

@yhatt yhatt left a comment

Choose a reason for hiding this comment

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

I have delayed to handle this problem because Docker for Windows and Mac have no problems about file permission. This change will be useful for Linux Docker!

Dockerfile Outdated
@@ -31,6 +32,10 @@ RUN yarn add puppeteer-core@chrome-$(chromium-browser --version | sed -r 's/^Chr
RUN yarn install && yarn build && rm -rf ./src ./node_modules && yarn install --production && yarn cache clean \
&& node /home/marp/.cli/marp-cli.js --version

USER root

ENV MARPID marp:marp
Copy link
Member

Choose a reason for hiding this comment

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

I felt the env name MARPID is unclear and a bit obscure. How about using MARP_USER instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely better.

Copy link
Member

@yhatt yhatt left a comment

Choose a reason for hiding this comment

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

Great! Many thanks for your contribution 😄

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.

2 participants