Well the latest tag doesnt work for me, it could be in fact a configuration issue but changing the tag to stable-3.1 works fine for me and since this is a relatively simple change without much consequences I think its worth It. In fact I see it as enhancement for development, even from point of View of testing the tags, its a nice little feature where one can just simply Change the tag on the fly. Similarly I think for the directory of patchwork, Its less error-prone using a predefined value, rather than hardcoding the Value everywhere, and it this ever changes? Then it has to be changed in Many places, instead of one. Even from the point of the if statement error Highlighted below when I change the if statement the error output need to Be changed. Last thing to point out I guess its actual linux configuration with Regards to script it fixes. The home or ~ character resolves differently on Different linux distros or even systems based on internal system configuration, And same applies for docker images. Lines below are error prone, because In the docker-compose we mount /home/patchwork/patchwork/… where as in the Scripts we use ~/patchwork/patchwork/…. To put this to real life, if my memory Serves me well, on ubuntu based systems, home directory for root resolves to /root where as for users to /home/username. Because user management calls Seem to cause issues on docker when used on Mac and many projects recommend To comment them out because of it, the use of relative paths in scripts can then cause Issues. I think its a good enhancement to add the possibility of change if the need arises, And also it fixes some bugs in the script.
> On 31 Oct 2024, at 21:56, Stephen Finucane <step...@that.guru> wrote: > > On Tue, 2024-10-29 at 19:09 +0000, Herakliusz Lipiec wrote: >> Currently to change the home directory for docker image it has >> to be changed in many places. Dockerfile allows for use of >> arguments and env variables, by using those now it can be changed >> by passing arguments to docker-compose. Also adding TAG argument >> for the same reason. > > Could I ask why you want to do this? Changing the tag shouldn't be necessary, > since 'latest' should always contain the runtimes needed to test Patchwork. > Similarly, there should be no need to customise the directory that Patchwork > runs under since those paths are relative to the container, not the host? I > don't have a Mac to test this on, but this feels like a local configuration > issue more so than anything else? We could probably remove the 'PROJECT_HOME' > configuration variable though since it's not used any more ('git log -S > PROJECT_HOME' shows I should have removed it in commit a0db473213 some time > back). > > Cheers, > Stephen > >> >> Signed-off-by: Herakliusz Lipiec <her...@icloud.com> >> --- >> docker-compose-pg.yml | 2 ++ >> docker-compose-sqlite3.yml | 2 ++ >> docker-compose.yml | 2 ++ >> tools/docker/Dockerfile | 10 ++++++---- >> tools/docker/entrypoint.sh | 9 +++++---- >> 5 files changed, 17 insertions(+), 8 deletions(-) >> >> diff --git a/docker-compose-pg.yml b/docker-compose-pg.yml >> index 44bc3ec..a9593bd 100644 >> --- a/docker-compose-pg.yml >> +++ b/docker-compose-pg.yml >> @@ -15,6 +15,8 @@ services: >> args: >> - UID >> - GID >> + - TAG >> + - PATCHWORK_DIR >> depends_on: >> - db >> volumes: >> diff --git a/docker-compose-sqlite3.yml b/docker-compose-sqlite3.yml >> index 900cb71..d26e74b 100644 >> --- a/docker-compose-sqlite3.yml >> +++ b/docker-compose-sqlite3.yml >> @@ -7,6 +7,8 @@ services: >> args: >> - UID >> - GID >> + - TAG >> + - PATCHWORK_DIR >> command: python3 manage.py runserver 0.0.0.0:8000 >> volumes: >> - .:/home/patchwork/patchwork/ >> diff --git a/docker-compose.yml b/docker-compose.yml >> index 73f080a..f6a59b2 100644 >> --- a/docker-compose.yml >> +++ b/docker-compose.yml >> @@ -20,6 +20,8 @@ services: >> args: >> - UID >> - GID >> + - TAG >> + - PATCHWORK_DIR >> depends_on: >> - db >> volumes: >> diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile >> index 0a55b54..4a18c44 100644 >> --- a/tools/docker/Dockerfile >> +++ b/tools/docker/Dockerfile >> @@ -1,12 +1,14 @@ >> -FROM ghcr.io/getpatchwork/pyenv:latest >> +ARG TAG="latest" >> +FROM ghcr.io/getpatchwork/pyenv:${TAG} >> >> ARG UID=1000 >> ARG GID=1000 >> - >> +ARG PATCHWORK_DIR="/home/patchwork/patchwork" >> ARG TZ="Australia/Canberra" >> + >> ENV DEBIAN_FRONTEND noninteractive >> ENV PYTHONUNBUFFERED 1 >> -ENV PROJECT_HOME /home/patchwork/patchwork >> +ENV PROJECT_HOME ${PATCHWORK_DIR} >> ENV DJANGO_SETTINGS_MODULE patchwork.settings.dev >> >> RUN groupadd --gid=$GID patchwork && \ >> @@ -38,4 +40,4 @@ COPY tools/docker/entrypoint.sh >> /usr/local/bin/entrypoint.sh >> ENTRYPOINT ["/usr/local/bin/entrypoint.sh"] >> CMD ["python3", "manage.py", "runserver", "0.0.0.0:8000"] >> USER patchwork >> -WORKDIR /home/patchwork/patchwork >> +WORKDIR $PATCHWORK_DIR >> diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh >> index c78c058..5a4085d 100755 >> --- a/tools/docker/entrypoint.sh >> +++ b/tools/docker/entrypoint.sh >> @@ -5,6 +5,7 @@ export DATABASE_HOST=${DATABASE_HOST:-} >> export DATABASE_PORT=${DATABASE_PORT:-} >> export DATABASE_USER=${DATABASE_USER:-patchwork} >> export DATABASE_PASSWORD=${DATABASE_PASSWORD:-password} >> +export PROJECT_HOME=${PROJECT_HOME:-/home/patchwork/patchwork/} >> >> case "${DATABASE_TYPE:-}" in >> postgres) >> @@ -40,13 +41,13 @@ test_database() { >> >> # check if patchwork is mounted. Checking if we exist is a >> # very good start! >> -if [ ! -f ~patchwork/patchwork/tools/docker/entrypoint.sh ]; then >> +if [ ! -f $PROJECT_HOME/tools/docker/entrypoint.sh ]; then To get this running, I need to change the above because ~patchwork doesnt resolve correctly >> cat << EOF >> The patchwork directory doesn't seem to be mounted! >> >> Are you using docker-compose? If so, you may need to create an SELinux rule. >> Refer to the development installation documentation for more information. >> -If not, you need -v PATH_TO_PATCHWORK:/home/patchwork/patchwork >> +If not, you need -v PATH_TO_PATCHWORK:$PROJECT_HOME Error here is outputs different value than the actual test without this patch. Even if the path is correct in the if statement and it has the missing slash, ~/patchwork/ will not always resolve to /home/patchwork I know its an edge case, but I actually think this is a quality change to the code. >> EOF >> exit 1 >> fi >> @@ -55,7 +56,7 @@ set +e >> >> # check if we need to rebuild because requirements changed >> for x in /opt/requirements-*.txt; do >> - if ! cmp $x ~/patchwork/$(basename $x); then >> + if ! cmp $x $PROJECT_HOME/$(basename $x); then >> cat << EOF >> A requirements file has changed. >> >> @@ -63,7 +64,7 @@ You may need to rebuild the patchwork image: >> >> docker-compose build web >> EOF >> - diff -u $x ~/patchwork/$(basename $x) >> + diff -u $x $PROJECT_HOME/$(basename $x) >> fi >> done >> >
_______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork