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

Add license for bibliographic data and make Dockerfile more robust #1222

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Flowdalic
Copy link
Contributor

Second iteration after #1220


WORKDIR /src
RUN OUTDIR=/var/www/html/extensions/ make -j$NCORES $TARGETS
RUN --mount=target=/xeps make -C /xeps -j$NCORES $TARGETS OUTDIR=/var/www/html/extensions
Copy link
Contributor

Choose a reason for hiding this comment

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

This implies that I cannot do git operations (like switching branches) while the build is running, without corrupting the result, right?

That would be a blocker.

Copy link
Contributor Author

@Flowdalic Flowdalic Dec 16, 2022

Choose a reason for hiding this comment

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

You could do git operations, but those will be immediately be reflected in the container with all the consequences that may have.

I am not sure how why this makes this approach as non suitable? How often do you perform git actions while a build is running? Regardless of the existence of a docker layer used for the build or not. You wouldn't run cmake .. && ninja and simultaneously switch the git branch, would you?

And in addition, this changes simplifies the Dockerfile enormously, we do not longer have to explicitly COPY stuff into the docker build context.

Instead of copying individual files into /src, we simply bind mount
the build context, which is typically the XEPs repo, and invoke 'make'
with the givne targets on that.
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