Docker Adaptation for some windows devices #331
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #331 +/- ##
===========================================
- Coverage 76.53% 63.46% -13.08%
===========================================
Files 98 92 -6
Lines 8452 8107 -345
===========================================
- Hits 6469 5145 -1324
- Misses 1983 2962 +979
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
This PR adapts the Docker installation and CI workflows to work better with Docker containers and some Windows devices by removing sudo dependencies, simplifying installation scripts, and improving the Docker build/run process.
- Removes sudo requirements from installation scripts to make them Docker-compatible
- Simplifies the Docker installation documentation and adds a new automated script
- Updates CI workflows to use sudo for elevated permissions where needed
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| installation/Ubuntu/install_su2_with_mpi.sh | Adds parallel build flag and cleanup command for Docker optimization |
| installation/Ubuntu/install_pentagrow.sh | Removes sudo commands and error handling to work in Docker containers |
| installation/DOCKER_INSTALLATION.md | Simplifies Docker setup instructions to use new automated script |
| ceasiompy_docker | New automated Docker build and run script with cross-platform support |
| CEASIOMpy_docker_Installation | Minor Dockerfile improvements for package installation |
| .github/workflows/unittests.yml | Adds manual trigger and sudo to shell commands for proper permissions |
| .github/workflows/integrationtests.yml | Adds manual trigger and sudo to shell commands for proper permissions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -1,50 +1,49 @@ | |||
| #!/bin/bash | |||
| set -e | |||
There was a problem hiding this comment.
Adding set -e at the beginning makes the script exit on any error, but then removing all error handling patterns (like || { echo ...; exit 1; }) throughout the script could mask important failure information. Consider keeping some error messages for critical operations or document that failures will be silent.
| shell: sudo bash -l {0} | ||
| run: | | ||
| installation/Ubuntu/install_pentagrow.sh | ||
| - name: Run unit tests | ||
| shell: bash -l {0} | ||
| shell: sudo bash -l {0} |
There was a problem hiding this comment.
Using sudo bash -l {0} as a shell is unconventional and potentially problematic. The typical approach would be to run individual commands with sudo rather than elevating the entire shell. Consider using shell: bash -l {0} and adding sudo to specific commands that need elevation.
| shell: sudo bash -l {0} | ||
| run: | | ||
| installation/Ubuntu/install_pyavl.sh | ||
| - name: Install SU2 | ||
| shell: bash -l {0} | ||
| shell: sudo bash -l {0} | ||
| run: | | ||
| installation/Ubuntu/install_su2_without_mpi.sh | ||
| - name: Install Pentagrow | ||
| shell: bash -l {0} | ||
| shell: sudo bash -l {0} | ||
| run: | | ||
| installation/Ubuntu/install_pentagrow.sh | ||
| - name: Run integration tests | ||
| shell: bash -l {0} | ||
| shell: sudo bash -l {0} |
There was a problem hiding this comment.
Using sudo bash -l {0} as a shell is unconventional and potentially problematic. The typical approach would be to run individual commands with sudo rather than elevating the entire shell. Consider using shell: bash -l {0} and adding sudo to specific commands that need elevation.
Prevent duplicates by checking if path already exists Co-authored-by: Copilot <[email protected]>
2 participants
original commit