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

Modified the distribution check to accept operating systems like Pop_os that are ubuntu/debian based and mention it differently in the /etc/os-release file #464

Closed
wants to merge 2 commits into from

Conversation

aserper
Copy link

@aserper aserper commented Jan 27, 2023

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Modified the distribution check to accept operating systems like Popthat are ubuntu/debian based and mention it differently in the /etc/os-release file

  • What is the current behavior? (You can also link to an open issue here)
    installer will fail on pop_os

  • What is the new behavior (if this is a feature change)? If possible add a screenshot.
    Installer will work on pop_os while still supporting the old scenario

  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    no

  • Other information:

Modified the distribution check to accept operating systems like Popthat are ubuntu/debian based and mention it differently in the /etc/os-release file
@aserper aserper changed the title Modified the distribution check to accept operating systems like Popthat are ubuntu/debian based and mention it differently in the /etc/os-release file Modified the distribution check to accept operating systems like Pop_os that are ubuntu/debian based and mention it differently in the /etc/os-release file Jan 27, 2023
@@ -145,7 +145,7 @@ if grep -q -i wsl /proc/version; then
fi

# distribution check
if ! grep -q "ID_LIKE=debian" /etc/os-release 2>/dev/null ; then
if ! grep -q "ID_LIKE=" /etc/os-release | grep -q "ubuntu\|debian" /etc/os-release 2>/dev/null ; then
Copy link
Member

Choose a reason for hiding this comment

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

is there a copy error? Should this be something like (untested):

if ! grep -q "ID_LIKE=debian\|ID_LIKE=ubuntu" /etc/os-release

Could you provide a /etc/os-release file?

Copy link
Author

Choose a reason for hiding this comment

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

Here is the content of /etc/os-release on my system:
NAME="Pop!_OS"
VERSION="22.04 LTS"
ID=pop
ID_LIKE="ubuntu debian"
PRETTY_NAME="Pop!_OS 22.04 LTS"
VERSION_ID="22.04"
HOME_URL="https://pop.system76.com"
SUPPORT_URL="https://support.system76.com"
BUG_REPORT_URL="https://github.com/pop-os/pop/issues"
PRIVACY_POLICY_URL="https://system76.com/privacy"
VERSION_CODENAME=jammy
UBUNTU_CODENAME=jammy
LOGO=distributor-logo-pop-os

Copy link
Member

@m-1-k-3 m-1-k-3 Jan 28, 2023

Choose a reason for hiding this comment

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

would this regex fit?

grep -E "ID_LIKE=(\")?(ubuntu)?( )?(debian)?"

Looks not too bad in my testset:

image

The first two are os-release files without debian or ubuntu entries. Then we have the popos and finally a ubuntu

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it looks good. I was too lazy to write a regex I guess

@m-1-k-3
Copy link
Member

m-1-k-3 commented Jan 27, 2023

Thank you for your contribution. Could you please check my comment?

@m-1-k-3 m-1-k-3 added the Installation Installation issues label Jan 28, 2023
@aserper
Copy link
Author

aserper commented Jan 28, 2023 via email

@m-1-k-3
Copy link
Member

m-1-k-3 commented Feb 8, 2023

Could you please finalize your PR. Would be really cool to get support for PopOS to EMBA

@m-1-k-3
Copy link
Member

m-1-k-3 commented Feb 15, 2023

Please re-open it if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Installation Installation issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants