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

Future3/lift 64bit restriction #2425

Conversation

mittler-works
Copy link

Closes #2402

This PR removes the check for 64bit OS in the installer script.

This PR extends the check for raspian OS to an alternate condition, which is the presence of /etc/apt/sources.list.d/raspi.list as some installations of raspian displaying debian instead of raspian in /etc/os-release.

Related:
#2402
#2041
#2313

Copy link
Collaborator

@s-martin s-martin left a comment

Choose a reason for hiding this comment

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

Thanks for providing this PR!

The PR basically removes all checks. From my point of view this could be ok as I don’t see any other use cases right now.

RPiOS: 32-bit = raspbian, 64-bit = debian
Copy link
Collaborator

@AlvinSchiller AlvinSchiller left a comment

Choose a reason for hiding this comment

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

Hi, thanks for your work!
While i was reviewing your changes and did some reading, i came to a different solution for the raspbian check i would like to discuss:

  • While checking for the "raspi.list" source list seems to be sufficient, this could be added to any linux system and does not really point to a Raspbian Pi OS. Also on viewing the usage of the function "is_raspbian" it occured, it is not really a check for Raspberry Pi OS, but rather a precheck to resolve the debian version number later on.
    So checking for "raspbian" or "debian" would be more accurate imo, as we just want to make sure we have a debian based OS (raspbian is derived from debian). Also these function(s) could be removed (there are no other uses) and be integrated in "_get_boot_file_path" directly.
_get_boot_file_path() {
    local filename="$1"
    local os_release_id=$( . /etc/os-release; printf '%s\n' "$ID"; )
    if [[ "$os_release_id" == *"raspbian"* ]] || [[ "$os_release_id" == *"debian"* ]]; then
        local debian_version_number=$( . /etc/os-release; printf '%s\n' "$VERSION_ID"; )
        ...
    else
    ...
}

What do you think?

PS: i have a commit ready and could push it, if you approve. So no need for changes on your side.

@mittler-works
Copy link
Author

Hi Alvin,
that's right - but I'm not aware of any way to make 100% sure to be on a RaspberryPi OS system.
If the setup also runs properly on any debian, the check you suggest is perfectly sufficient and I would prefer it.
I also like your suggestion to consolidate and unclutter the code.
From my point of view, go for it! :)

@AlvinSchiller AlvinSchiller merged commit 32798da into MiczFlor:future3/develop Nov 7, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement future3 Relates to future3 development
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 | bookworm: check for raspian not working future3 | Install on 64bit not working (Wrong OS type)
3 participants