Conversation
|
This script uses only one case of ubuntu/rolling for now. |
|
What's the status on this one, @JAuriac, @benoitmartin88 ? |
benoitmartin88
left a comment
There was a problem hiding this comment.
I am not sure that we need such a script.
It looks very static: the image is fixed and it must use podman (probably better to use docker to be more generic, then its up to the user to alias docker to podman).
My suggestion is that this should be added to PDI's documentation.
|
there used to be this https://github.com/pdidev/pdi/blob/1.7.1-gh/tools/build_scripts/test_in_docker that interpreted and ran the |
|
@JAuriac can we close this ? |
|
For the indentation, adding podman is a good idea, but why not just add it to |
| # Set SRCDIR only if not provided | ||
| if [[ -z "${SRCDIR:-}" ]]; then | ||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| SRCDIR="$(cd "$SCRIPT_DIR/.." && pwd)" | ||
| fi |
There was a problem hiding this comment.
Please add podman support in addition to docker in bin/test_indent
There was a problem hiding this comment.
Does it have to be so complex?
In my case a simple:
IMAGE_NAME=ghcr.io/pdidev/spack/latest/clang/openmpi/all:v4
podman/docker run --rm -ti -v ${PWD}:/src:ro ${IMAGE_NAME} /src/bin/build_and_run_all_testsdoes the trick. Do we need more?
There was a problem hiding this comment.
The key phrase is "in my case", as all pdi contributors maybe shouldn't have to know about containers' directory mounting and the behaviour of the online CI. Two scripts (placed in the bin folder or a hidden folder for example, the second one being the indent script) could enable to test a local change, independently of the local environment, with an additional option to specify the image used.
As seen together, the following should be environment-agnostic :
IMAGE_NAME=ghcr.io/pdidev/spack/latest/clang/openmpi/all:v4
podman run --rm -ti -e CMAKE_FLAGS="-DBUILD_DOCUMENTATION=OFF" -v ${PWD}:/src:ro ${IMAGE_NAME} /src/bin/build_and_run_all_tests
There was a problem hiding this comment.
totally agree that a script makes sense. I just think it can be simpler
There was a problem hiding this comment.
Correction, the previous commands are not sufficient on their own, a chmod is needed, as mentioned by Iole :
chmod -R o+rX .
IMAGE_NAME=ghcr.io/pdidev/spack/latest/clang/openmpi/all:v4
podman run --rm -ti -e CMAKE_FLAGS="-DBUILD_DOCUMENTATION=OFF" -v ${PWD}:/src:ro ${IMAGE_NAME} /src/bin/build_and_run_all_tests
There was a problem hiding this comment.
ideally we shouldn't modify the user filesystem
but I don't know any way to make the user data accessible without running as root in the image. do you?
Add a script to run the CI locally.
Made for use on laptop, using podman.
Only one configuration case of ubuntu/rolling is used for now.
We may want to loop over the all configurations as defined in .github/workflows/tests.yml, or we may want to keep it lightweight as is (or we can add both through an option on this script).
Requires :
and, if they exists, clear the cache files at the root "pdi/" :
Fix #596
List of things to check before making a PR
Before merging your code, please check the following:
.clang-format;Fix #issuekeyword to autoclose the issue when merged.