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

Fix issue for building docker container #91

Merged
merged 10 commits into from
Dec 13, 2024
13 changes: 7 additions & 6 deletions .github/workflows/build-container.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,25 @@ jobs:
name: 'Build amc2moodle container'
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v2
- uses: actions/checkout@main
- id: docker-tag
uses: yuya-takeyama/docker-tag-from-github-ref-action@v1
uses: yuya-takeyama/docker-tag-from-github-ref-action@main
with:
latest-branches: 'main,master'
- name: "Build:checkout"
uses: actions/checkout@v2
uses: actions/checkout@main
- name: Set up Docker Buildx
id: buildx
uses: docker/setup-buildx-action@v1
uses: docker/setup-buildx-action@main
- name: Login to Github Packages
uses: docker/login-action@v1
uses: docker/login-action@main
with:
registry: ghcr.io
username: ${{ github.repository_owner }}
password: ${{ secrets.PAT }}
- name: 'Build:dockerimage'
uses: docker/build-push-action@v2
id: docker_build
uses: docker/build-push-action@main
with:
# relative path to the place where source code with Dockerfile is located
context: ./docker
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci-mac-os.yml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
run: |
# xstring is just required for testing --includestyles
sudo tlmgr update --self
sudo tlmgr install collection-fontsrecommended bophook xstring --verify-repo=none
sudo tlmgr install --verify-repo=none collection-fontsrecommended bophook xstring

- name: Test latexml
run: |
Expand Down
34 changes: 21 additions & 13 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,47 @@ FROM texlive/texlive:latest
# labels
LABEL AUTHOR='Luc Laurent'
LABEL MAINTAINER='Luc Laurent & Benoit Nennig'
LABEL org.opencontainers.image.source = "https://github.com/nennigb/amc2moodle/"
LABEL org.opencontainers.image.source="https://github.com/nennigb/amc2moodle/"

# declare useful directories using environement
ENV MONITOR_DIR=/tmp/work
ENV LOG_DIR=/tmp/daemon
ENV INSTALL_DIR_A2M=/tmp/amc2moodle
ENV VENVDIR=/tmp/venv
SHELL ["/bin/bash", "-c"]

# install debian packages
RUN apt update && \
apt install -yy wget ghostscript imagemagick libtext-unidecode-perl latexml xmlindent python3-pip git && \
apt clean
apt install -yy wget \
ghostscript \
imagemagick \
libtext-unidecode-perl \
latexml \
xmlindent \
python3-pip \
python3-venv \
git && \
apt clean &&\
rm -rf /var/lib/{apt,dpkg,cache,log}/

Copy link
Owner

Choose a reason for hiding this comment

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

better to use apt-get (to avoid WARNING: apt does not have a stable CLI interface. Use with caution in scripts.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

# move policy file
RUN mv /etc/ImageMagick-6/policy.xml /etc/ImageMagick-6/policy.xml.off
Copy link
Owner

@nennigb nennigb Dec 4, 2024

Choose a reason for hiding this comment

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

On my tests, docker install imageMagick 7 thus the path is wrong.
In interactive session it seems to work without this line. The default policy has changed in v7, perhaps we could just comment this line.
To see the policy in v7

magick identify -list policy

https://imagemagick.org/script/security-policy.php

Normally it should be catch in the unittests with the tinymonk.pdf file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed


# install pip and Python pkg
# RUN pip3 install -U pip && \
# pip install amc2moodle
WORKDIR /tmp
RUN git clone https://github.com/nennigb/amc2moodle.git -b master ${INSTALL_DIR_A2M}
WORKDIR ${INSTALL_DIR_A2M}
RUN pip3 install -U pip && \
pip3 install .
RUN mkdir -p ${VENVDIR} && \
python -m venv ${VENVDIR}
Copy link
Owner

Choose a reason for hiding this comment

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

Why installing the package in a venv ?
No problem with that, I am just curious ;-)
Doesn't seems to create problem.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in order to avoid "error: externally-managed-environment" when use the system python/pip

Copy link
Owner

Choose a reason for hiding this comment

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

ok :-)

ENV PATH="${VENVDIR}/bin:$PATH"
RUN pip install -U pip && \
pip install '.[test]'

# check if amc2moodle and moodle2amc work
WORKDIR /
ENV TEXINPUTS=.:${INSTALL_DIR_A2M}/amc2moodle/moodle2amc/test:$TEXINPUTS
ENV TEXINPUTS=.:${INSTALL_DIR_A2M}/amc2moodle/tests/payload_test_moodle2amc/:${TEXINPUTS}
RUN echo ${TEXINPUTS}
RUN python -m amc2moodle.amc2moodle.test && \
python -m amc2moodle.utils.test && \
python -m amc2moodle.moodle2amc.test


RUN python -m pytest --pyargs amc2moodle
Copy link
Owner

Choose a reason for hiding this comment

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

CI will fail until

[project.optional-dependencies]
test = ["pytest", "pytest-cov[all]"]

will be merged into the master branch used for docker build.
We could merge by the way.


# create dir
RUN mkdir ${MONITOR_DIR} && \
Expand Down
4 changes: 4 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ dependencies = [
]
requires-python='>=3.8'

[project.optional-dependencies]
test = ["pytest", "pytest-cov[all]"]
lint = [ "black", "flake8"]
Copy link
Owner

Choose a reason for hiding this comment

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

By default
https://hatch.pypa.io/1.13/config/internal/static-analysis/
hatch use ruff: https://github.com/astral-sh/ruff, 99.9% compatible with black.
Not sure we need this line: lint = [ "black", "flake8"]
Perhaps we could also configure ruff or have a discussion on rules

hatch default is line 120, which looks good for me. It could be put in the pyproject.toml to avoid changes

[tool.ruff]
# Allow lines to be as long as 120.
line-length = 120

I prefer simple quotes which is the present convention and looks less bulky.

[tool.ruff.format]
quote-style = "single"

If we choose ruff, we need to modify the readme.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I add it


[project.urls]
Homepage = "https://github.com/nennigb/amc2moodle"
Repository = "https://github.com/nennigb/amc2moodle"
Expand Down
Loading