<html><head><style>pre,code,address {
margin: 0px;
}
h1,h2,h3,h4,h5,h6 {
margin-top: 0.2em;
margin-bottom: 0.2em;
}
ol,ul {
margin-top: 0em;
margin-bottom: 0em;
}
blockquote {
margin-top: 0em;
margin-bottom: 0em;
}
</style></head><body style="overflow-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;"><div>On Fri, 2024-11-01 at 00:46 +0000, Herakliusz Lipiec wrote:</div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><br id="lineBreakAtBeginningOfMessage"></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Well the latest tag doesnt work for me, it could be in fact a configuration </span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">issue but changing the tag to stable-3.1 works fine for me and since this</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">is a relatively simple change without much consequences I think its worth</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">It.</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"></span></div></blockquote><div><br></div><div>But that's the point: it's not. The stable-3.2 tag (which FYI just got created yesterday in order to fix build failures on that branch) is missing Python 3.13, which is part of the test matrix for current main, and has Python 3.8, which is not. The stable-3.1 tag is even older and is missing Python 3.12 but has Python 3.7. By using these older tags, you don't have the ability to run the full test matrix (i.e. running `tox` without arguments will fail on e.g. the `py313-*` testenvs). That kind of defeats the point of having that Dockerfile, since it exists solely to provide us with all the Python runtimes needed for our test matrix.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">In fact I see it as enhancement for development, even from point of</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">View of testing the tags, its a nice little feature where one can just simply</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Change the tag on the fly.</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"></span></div></blockquote><div><br></div><div>This shouldn't ever be necessary though. There should never be any tags except for `latest` and various `stable-*` tags, and there's a 1:1 mapping between tags for the Docker image and branches for the git repo (effectively `latest:main` and `stable-X.Y:stable/X.Y`). If we need to change the tag, it should be because we have created a new stable branch. See [1] for example.</div><div><br></div><div>[1] <a href="https://github.com/getpatchwork/patchwork/commit/0da30213d42e8ec5067eb598cd0ebffc0cae74a1">https://github.com/getpatchwork/patchwork/commit/0da30213d42e8ec5067eb598cd0ebffc0cae74a1</a></div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Similarly I think for the directory of patchwork,</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Its less error-prone using a predefined value, rather than hardcoding the</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Value everywhere, and it this ever changes? Then it has to be changed in</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Many places, instead of one.</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"></span></div></blockquote><div><br></div><div>Eh. 🤷</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>❯ sed -i 's/\/home\/patchwork\/patchwork/\/home\/foo\/bar/' tools/docker/Dockerfile docker-compose*</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>❯ sed -i 's/~\/patchwork/\/home\/foo\/bar/' tools/docker/Dockerfile docker-compose* tools/docker/entrypoint.sh</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Even from the point of the if statement error</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Highlighted below when I change the if statement the error output need to</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Be changed. Last thing to point out I guess its actual linux configuration with</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Regards to script it fixes. The home or ~ character resolves differently on</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Different linux distros or even systems based on internal system configuration,</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">And same applies for docker images. Lines below are error prone, because</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">In the docker-compose we mount /home/patchwork/patchwork/… where as in the</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Scripts we use ~/patchwork/patchwork/…. To put this to real life, if my memory</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Serves me well, on ubuntu based systems, home directory for root resolves to</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">/root where as for users to /home/username.</span></div><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"></span></div></blockquote><div><br></div><div>fwiw, I'm fine replacing `~/patchwork` with `/home/patchwork/patchwork`. In fact, I'll go do that right now.</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Because user management calls</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Seem to cause issues on docker when used on Mac and many projects recommend</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">To comment them out because of it, the use of relative paths in scripts can then cause</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Issues. I think its a good enhancement to add the possibility of change if the need arises,</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">And also it fixes some bugs in the script.</span></div></blockquote><div><br></div><div>I'm afraid I think this is a case of You're Not Gonna Need It (YAGNI), and I think it would be better to diagnose the issue you're having with Docker. Are you able to pull the images outside of docker-compose? Each of the below should work just fine for you:</div><div><br></div><div style="margin-left: 3ch;">❯ docker pull ghcr.io/getpatchwork/pyenv:stable-3.1</div><div style="margin-left: 3ch;">❯ docker pull ghcr.io/getpatchwork/pyenv:stable-3.2</div><div style="margin-left: 3ch;"><div>❯ docker pull ghcr.io/getpatchwork/pyenv:latest</div></div><div><br></div><div>If they're not, that's an issue you should look into because it suggests a local configuration issue that is almost certainly going to affect you elsewhere: pulling images shouldn't require any special permissions. However, if that works, you should test if you're? For example:</div><div><br><span class="Apple-tab-span" style="white-space:pre"> </span>❯ docker run -it ghcr.io/getpatchwork/pyenv:latest python3.13</div><div><br></div><div>That should open the Python REPL. If that works then it confirms the issue lies elsewhere. I wasn't able to find anything from a quick web search RE: permissions issue but my money is on the fact that we mount the local directory as a volume. Have you tried for example cloning the repo to '/tmp' and running docker-compose from there? That would confirm if its file permission issue related to running from your home directory. Also, are you using docker-tools or Docker for Mac? Have you tried podman?</div><div><br></div><div>Hope this helps,</div><div>Stephen</div><div><br></div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><font color="#000000"><span style="caret-color: rgb(0, 0, 0);"><br></span></font><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>On 31 Oct 2024, at 21:56, Stephen Finucane <stephen@that.guru> wrote:</div><div><br class="Apple-interchange-newline"></div><div><div>On Tue, 2024-10-29 at 19:09 +0000, Herakliusz Lipiec wrote:<br><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div>Currently to change the home directory for docker image it has<br>to be changed in many places. Dockerfile allows for use of<br>arguments and env variables, by using those now it can be changed<br>by passing arguments to docker-compose. Also adding TAG argument<br>for the same reason.<br></div></blockquote><br>Could I ask why you want to do this? Changing the tag shouldn't be necessary,<br>since 'latest' should always contain the runtimes needed to test Patchwork.<br>Similarly, there should be no need to customise the directory that Patchwork<br>runs under since those paths are relative to the container, not the host? I<br>don't have a Mac to test this on, but this feels like a local configuration<br>issue more so than anything else? We could probably remove the 'PROJECT_HOME'<br>configuration variable though since it's not used any more ('git log -S<br>PROJECT_HOME' shows I should have removed it in commit a0db473213 some time<br>back).<br><br>Cheers,<br>Stephen<br><br><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><br>Signed-off-by: Herakliusz Lipiec <hero24@icloud.com><br>---<br> docker-compose-pg.yml | 2 ++<br> docker-compose-sqlite3.yml | 2 ++<br> docker-compose.yml | 2 ++<br> tools/docker/Dockerfile | 10 ++++++----<br> tools/docker/entrypoint.sh | 9 +++++----<br> 5 files changed, 17 insertions(+), 8 deletions(-)<br><br>diff --git a/docker-compose-pg.yml b/docker-compose-pg.yml<br>index 44bc3ec..a9593bd 100644<br>--- a/docker-compose-pg.yml<br>+++ b/docker-compose-pg.yml<br>@@ -15,6 +15,8 @@ services:<br> args:<br> - UID<br> - GID<br>+ - TAG<br>+ - PATCHWORK_DIR<br> depends_on:<br> - db<br> volumes:<br>diff --git a/docker-compose-sqlite3.yml b/docker-compose-sqlite3.yml<br>index 900cb71..d26e74b 100644<br>--- a/docker-compose-sqlite3.yml<br>+++ b/docker-compose-sqlite3.yml<br>@@ -7,6 +7,8 @@ services:<br> args:<br> - UID<br> - GID<br>+ - TAG<br>+ - PATCHWORK_DIR<br> command: python3 manage.py runserver 0.0.0.0:8000<br> volumes:<br> - .:/home/patchwork/patchwork/<br>diff --git a/docker-compose.yml b/docker-compose.yml<br>index 73f080a..f6a59b2 100644<br>--- a/docker-compose.yml<br>+++ b/docker-compose.yml<br>@@ -20,6 +20,8 @@ services:<br> args:<br> - UID<br> - GID<br>+ - TAG<br>+ - PATCHWORK_DIR<br> depends_on:<br> - db<br> volumes:<br>diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile<br>index 0a55b54..4a18c44 100644<br>--- a/tools/docker/Dockerfile<br>+++ b/tools/docker/Dockerfile<br>@@ -1,12 +1,14 @@<br>-FROM ghcr.io/getpatchwork/pyenv:latest<br>+ARG TAG="latest"<br>+FROM ghcr.io/getpatchwork/pyenv:${TAG}<br><br> ARG UID=1000<br> ARG GID=1000<br>-<br>+ARG PATCHWORK_DIR="/home/patchwork/patchwork"<br> ARG TZ="Australia/Canberra"<br>+<br> ENV DEBIAN_FRONTEND noninteractive<br> ENV PYTHONUNBUFFERED 1<br>-ENV PROJECT_HOME /home/patchwork/patchwork<br>+ENV PROJECT_HOME ${PATCHWORK_DIR}<br> ENV DJANGO_SETTINGS_MODULE patchwork.settings.dev<br><br> RUN groupadd --gid=$GID patchwork && \<br>@@ -38,4 +40,4 @@ COPY tools/docker/entrypoint.sh /usr/local/bin/entrypoint.sh<br> ENTRYPOINT ["/usr/local/bin/entrypoint.sh"]<br> CMD ["python3", "manage.py", "runserver", "0.0.0.0:8000"]<br> USER patchwork<br>-WORKDIR /home/patchwork/patchwork<br>+WORKDIR $PATCHWORK_DIR<br>diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh<br>index c78c058..5a4085d 100755<br>--- a/tools/docker/entrypoint.sh<br>+++ b/tools/docker/entrypoint.sh<br>@@ -5,6 +5,7 @@ export DATABASE_HOST=${DATABASE_HOST:-}<br> export DATABASE_PORT=${DATABASE_PORT:-}<br> export DATABASE_USER=${DATABASE_USER:-patchwork}<br> export DATABASE_PASSWORD=${DATABASE_PASSWORD:-password}<br>+export PROJECT_HOME=${PROJECT_HOME:-/home/patchwork/patchwork/}<br><br> case "${DATABASE_TYPE:-}" in<br> postgres)<br>@@ -40,13 +41,13 @@ test_database() {<br><br> # check if patchwork is mounted. Checking if we exist is a<br> # very good start!<br>-if [ ! -f ~patchwork/patchwork/tools/docker/entrypoint.sh ]; then<br>+if [ ! -f $PROJECT_HOME/tools/docker/entrypoint.sh ]; then<br></div></blockquote></div></div></blockquote><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">To get this running, I need to change the above because ~patchwork doesnt resolve correctly</span><br><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div> cat << EOF<br> The patchwork directory doesn't seem to be mounted!<br><br> Are you using docker-compose? If so, you may need to create an SELinux rule.<br> Refer to the development installation documentation for more information.<br>-If not, you need -v PATH_TO_PATCHWORK:/home/patchwork/patchwork<br>+If not, you need -v PATH_TO_PATCHWORK:$PROJECT_HOME<br></div></blockquote></div></div></blockquote><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Error here is outputs different value than the actual test without this patch.</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">Even if the path is correct in the if statement and it has the missing slash,</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">~/patchwork/ will not always resolve to /home/patchwork</span><br style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);"><span style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0);">I know its an edge case, but I actually think this is a quality change to the code.</span><br><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div><div><blockquote type="cite" style="margin:0 0 0 .8ex; border-left:2px #729fcf solid;padding-left:1ex"><div> EOF<br> exit 1<br> fi<br>@@ -55,7 +56,7 @@ set +e<br><br> # check if we need to rebuild because requirements changed<br> for x in /opt/requirements-*.txt; do<br>- if ! cmp $x ~/patchwork/$(basename $x); then<br>+ if ! cmp $x $PROJECT_HOME/$(basename $x); then<br> cat << EOF<br> A requirements file has changed.<br><br>@@ -63,7 +64,7 @@ You may need to rebuild the patchwork image:<br><br> docker-compose build web<br> EOF<br>- diff -u $x ~/patchwork/$(basename $x)<br>+ diff -u $x $PROJECT_HOME/$(basename $x)<br> fi<br> done<br><br></div></blockquote><br></div></div></blockquote></div><div><br></div></blockquote><div><br></div><div><span></span></div></body></html>