Docker overhaul #54
Docker overhaul #54
Conversation
|
I expected this to break some of the workflows (e.g. bumping smriprep will have an effect on workflow names) but since I'm about to implement some refactoring thought this was a good first step. |
eilidhmacnicol
force-pushed
the
docker/docker_overhaul
branch
from
aea81fe to
86947cc
Compare
| test "${INSTALLED_VERSION}" = "${THISVERSION}" | ||
| - name: Install in confined environment [setup.py - install] | ||
| run: | | ||
| python -m venv /tmp/setup_install |
There was a problem hiding this comment.
You can go ahead and get rid of setup.py - install. Setuptools does not recommend installing this way. You could replace with pip install .. You should not need to install dependencies manually first.
| - name: Install in confined environment [setup.py - develop] | ||
| run: | | ||
| python -m venv /tmp/setup_develop | ||
| source /tmp/setup_develop/bin/activate |
There was a problem hiding this comment.
Likewise, setup.py develop can be replaced with pip install -e . You should not need to install dependencies manually first.
|
There's something up with the Circle builds. You're getting: In the anatomical workflow. |
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
|
Thanks for the speedy review. I think the GHA fails are being investigated. Other than that, I think I'll come back to the RE circle builds: I think it's because there's no docker registry set up. I'll try to emulate fmriprep's code and come back with something soon. |
|
To fix the version, you need to remove things that are checked into git from your dockerignore: fmriprep-rodents/.dockerignore Lines 28 to 43 in d153551 |
|
Thanks - I was going to just comment out the check until versioneer was removed (i.e. something like nipreps/fmriprep@2191833 if pyproject.toml is upcoming) but this is much easier. 🤞🏼 |
eilidhmacnicol
force-pushed
the
docker/docker_overhaul
branch
from
327f140 to
32b7740
Compare
| RUN pip install build | ||
| RUN apt-get update && \ | ||
| apt-get install -y --no-install-recommends git | ||
| COPY . /src/fmriprep |
There was a problem hiding this comment.
DO NOT ACCEPT
@eilidhmacnicol @oesteban @mgxd One approach we could take to avoid annoying .dockerignore mismatches could be:
| COPY . /src/fmriprep | |
| COPY .git /src/fmriprep/.git | |
| RUN git -C /src/fmriprep reset --hard HEAD |
This would ensure that images only get built from commits, and not from dirty worktrees. I'm not sure whether we value being able to build from a dirty worktree... Personally, I just fmriprep-docker ... --patch fmriprep=$PWD/fmriprep when I want that.
|
Eilidh, for reference, I did the following to find the remaining things to remove from $ docker build --target=src -t fmriprep-rodents:test .
$ docker run --rm -it --entrypoint=bash fmriprep-rodents:test
> cd /src/fmriprep
> git status
On branch docker/docker_overhaul
Changes not staged for commit:
(use "git add/rm <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
deleted: .maint/contributors.json
deleted: .maint/developers.json
deleted: .maint/former.json
deleted: .maint/paper_author_list.py
deleted: .maint/update_changes.sh
deleted: .maint/update_zenodo.py
deleted: Makefile
no changes added to commit (use "git add" and/or "git commit -a") |
|
Great! The tests are still failing, but at least now they are failing like I expected them to do. Unless there's anything else, I think this might be ready to merge. Thanks for all your help today, @effigies! |
2 participants
Supercedes #53
Changes proposed in this pull request
Documentation that should be reviewed