-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Debian 12 docker-compose installation #112
base: trunk
Are you sure you want to change the base?
Conversation
Writing a pr that will not include python venv. Docker-compose v2 installation with binaries would be sufficient |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You removed a generic installation solution in the catch all for a debian specific installation solution. Please revert and put debian specific steps in its own case.
esac | ||
fi | ||
|
||
# docker-compose | ||
if ! command_exists docker-compose; then | ||
case $(uname -m) in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it helps, this case is for different architectures, we try to use a generic solution when available, but for when the generic solution isn't working we add a case statement.
case $(uname -m) in | ||
x86_64|amd64) curl -fsSL "$compose_url/docker-compose-$(uname -s)-$(uname -m)" -o /usr/local/bin/docker-compose;; | ||
*) | ||
case "$os_release" in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The os_release variable contains the distro information, which was extracted from /etc/os-release. This is where you will find what ever Debian's defaults to, it should be consistent across all versions of debian
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the information from my bookworm machine:
PRETTY_NAME="Debian GNU/Linux 12 (bookworm)"
NAME="Debian GNU/Linux"
VERSION_ID="12"
VERSION="12 (bookworm)"
VERSION_CODENAME=bookworm
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we use id, then you can use "debian" as the case key
- What I did
Raising for issue #89. Would like review from someone who can test on Debian 12 system. From what I remember, @cpswan discussed a different usage of venv rather than the way proposed here in PR.
- How I did it
Local edits
- How to verify it
Test installation on Debian 12 machine
- Description for the changelog
fix: Debian 12 docker-compose installation