Skip to content
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

Expand PPM platform support #1728

Merged
merged 4 commits into from
Oct 17, 2023
Merged

Expand PPM platform support #1728

merged 4 commits into from
Oct 17, 2023

Conversation

toph-allen
Copy link
Contributor

@toph-allen toph-allen commented Oct 17, 2023

Fixes #1720

I set out to add support for getting Rocky Linux 9 binaries from Package Manager instances, as it's binary compatible with CentOS/RHEL binaries. This was initially motivated by adding binary support for Connect instances hosted on Rocky. Along the way I went to check out /etc/os-release files from other supported distros, and made some other additions/fixes.

Connect's supported distros are listed here, and Package Manager's are available here.

Changes I made:

  • Added functions to detect Rocky Linux and AlmaLinux, two CentOS/RHEL-compatible distributions, as centos8/rhel9.
  • Added support for Debian. (It isn't supported by Connect but is by PPM, so it's nice for renv to support it.) Its platform function is like Ubuntu.
  • Fixed support for OpenSUSE. Previously, the renv_ppm_platform_suse() function would return something like opensuse15, but the minor part of the version needs to be appended, e.g. opensuse154. Also, OpenSUSE /etc/os-release files don't always contain suse as a standalone word, so the I changed the regex used to identify this distro.
  • Added support for SLES, which has different /etc/os-release files from OpenSUSE.
  • Added tests for a cross-section distros (not all minor versions).

Other changes I'm considering, and would appreciate feedback on whether to include or not:

  • Adding support for detecting Amazon Linux 2 as centos7. It seems to be mostly compatible (StackOverflow, Amazon docs), and I think this is the basis of Connect listing it as supported, but I'm not sure.

Code style questions:

  • It looks like we previously erred on the side of giving each distro a separate function, even if for now the logic is identical to other functions. I followed this pattern with the new distros. But we could also just call e.g. renv_ppm_platform_rhel for AlmaLinux and Rocky.
  • We could look for e.g. \\bsuse\\b in the ID_LIKE field instead of directly looking for the ID field.

Copy link
Contributor

@aronatkins aronatkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is the /etc/os-issue from an amazonlinux2 container

docker run -it --rm amazonlinux:2 cat /etc/os-release
NAME="Amazon Linux"
VERSION="2"
ID="amzn"
ID_LIKE="centos rhel fedora"
VERSION_ID="2"
PRETTY_NAME="Amazon Linux 2"
ANSI_COLOR="0;33"
CPE_NAME="cpe:2.3:o:amazon:amazon_linux:2"
HOME_URL="https://amazonlinux.com/"
SUPPORT_END="2025-06-30"

@aronatkins
Copy link
Contributor

Does this change (and the one in #1716) need NEWS?

Copy link
Collaborator

@kevinushey kevinushey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kevinushey kevinushey merged commit 18f8fed into main Oct 17, 2023
1 of 12 checks passed
@kevinushey
Copy link
Collaborator

It looks like we previously erred on the side of giving each distro a separate function, even if for now the logic is identical to other functions. I followed this pattern with the new distros. But we could also just call e.g. renv_ppm_platform_rhel for AlmaLinux and Rocky.

I'd prefer keeping these separate just for clarity, even if it means some code is duplicated here.

@toph-allen
Copy link
Contributor Author

I'd prefer keeping these separate just for clarity, even if it means some code is duplicated here.

Yeah, agreed 👍🏻

@toph-allen toph-allen deleted the toph-rocky-linux-ppm-support branch October 17, 2023 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

renv_ppm_platform() should detect Rocky Linux as Red Hat.
3 participants