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

🐛 | bookworm: check for raspian not working #2402

Closed
mittler-works opened this issue Jul 13, 2024 · 13 comments · Fixed by #2425
Closed

🐛 | bookworm: check for raspian not working #2402

mittler-works opened this issue Jul 13, 2024 · 13 comments · Fixed by #2425
Labels
bug future3 Relates to future3 development needs triage

Comments

@mittler-works
Copy link

Version

3.5.3

Branch

future3/main

OS

Raspberry Pi OS Bookworm Lite

Pi model

Zero 2W

Hardware

No response

What happened?

When running the installer script, it fails saying

ERROR: '<something>' found more than once in 'unknown'

Where <something> is not always the same thing.

I tracked it down, the problem is the check for Raspian here.

Newer releases of Raspberry Pi OS (bookworm) do not display raspian in /etc/os-release no more, but just debian. So there is no way to really tell if we are on Raspberry Pi OS.
I have found articles that propose to check the existence of /etc/apt/sources.list.d/raspi.list to determine if we are on Raspberry Pi OS. Although this is obviously not perfect either, it might be a better solution than consulting /etc/os-release for the future.

Logs

No response

Configuration

No response

More info

No response

@mittler-works mittler-works added bug future3 Relates to future3 development needs triage labels Jul 13, 2024
@mittler-works
Copy link
Author

This only seems to affect bookworm 64bit. bookworm 32 bit shows raspian, as usual.

@s-martin
Copy link
Collaborator

At this time 64bit is not supported (there's also a check for it).

@mittler-works
Copy link
Author

mittler-works commented Jul 15, 2024

I know that (I had to bypass the check 😄 ), but I'm always interested in trying things out. ;-)
Basically, bookworm/64bit is already running quite decent. It's just little things; but I think it's important to track them, even if the software is currently not supporting it. That might save a bit of work for future later. 😉

@s-martin
Copy link
Collaborator

Basically, bookworm/64bit is already running quite decent. It's just little things; but I think it's important to track them, even if the software is currently not supporting it. That might save a bit of work for future later. 😉

I agree.

We also happily discuss possible changes for a working 64bit implementation, so if anybody opens a PR with necessary changes interested people could review it.
I know @alesrosina did also make it work (see #2313)

@alesrosina
Copy link

Basically, bookworm/64bit is already running quite decent. It's just little things; but I think it's important to track them, even if the software is currently not supporting it. That might save a bit of work for future later. 😉

I agree.

We also happily discuss possible changes for a working 64bit implementation, so if anybody opens a PR with necessary changes interested people could review it. I know @alesrosina did also make it work (see #2313)

Yes, basically I just removed all checks in setup script. And then it's running as it should on 64bit.

I probably should take the time to make a PR, but probably not before autumn, since in summer time i prefer to be outside in my free time ;)

@mittler-works
Copy link
Author

I can prepare a PR to lift the 64 bit restriction. It would only be a change in the install-jukebox.sh (removing _check_os_type) and the thing with the Raspian check. There I would then use an or-condition in terms of "Either the os-release property ID contains raspian or the file /etc/apt/sources.list.d/raspi.list exists and contains corresponding raspian sources".

I've found no other issues with 64 bit. The GPIO Problems with kernel 6.6 upwards mentioned in the other issue are not specific to 64 bit, but also affect 32 bit, right? So that wouldn't be a show stopper for lifting 64 bit restriction, I think.

@alesrosina
Copy link

I can prepare a PR to lift the 64 bit restriction. It would only be a change in the install-jukebox.sh (removing _check_os_type) and the thing with the Raspian check. There I would then use an or-condition in terms of "Either the os-release property ID contains raspian or the file /etc/apt/sources.list.d/raspi.list exists and contains corresponding raspian sources".

I've found no other issues with 64 bit. The GPIO Problems with kernel 6.6 upwards mentioned in the other issue are not specific to 64 bit, but also affect 32 bit, right? So that wouldn't be a show stopper for lifting 64 bit restriction, I think.

Yes, from my experience that's correct 👍

@harrydoddnoble
Copy link

@mittler-works did you manage to create a PR for this?

@mittler-works
Copy link
Author

Sorry, that got lost in the daily grind. 😉
I have just created the PR. 👋

@harrydoddnoble
Copy link

harrydoddnoble commented Sep 16, 2024 via email

@s-martin s-martin linked a pull request Sep 16, 2024 that will close this issue
@mittler-works
Copy link
Author

mittler-works commented Sep 17, 2024

With pleasure :)

But one other thing I would like to discuss: the error message you get when you are not on Raspberry Pi OS is not meaningful at all (see OP of this issue).

The problem origins in getting the boot and cmdline file paths. When the raspbian check (maybe it should be Raspberry Pi OS Check today? Thats also the reason why there is not always "raspbian" in /etc/os-release) returns false, then _get_boot_file_path just returns unknown as valid file path instead of stopping the execution with a failure. So the routine continues as usual and if you want to write or do something with the returned path, it fails with curious error messages.

My proposal would be, that the function should write a meaningful error message to stderr and exit with non-zero exit code. I don't know if there is any kind of logger and/or exception handling implemented in the installer scripts. If not, it would be as easy as echoing to stderr and exit.

@s-martin
Copy link
Collaborator

My proposal would be, that the function should write a meaningful error message to stderr and exit with non-zero exit code. I don't know if there is any kind of logger and/or exception handling implemented in the installer scripts. If not, it would be as easy as echoing to stderr and exit.

yes, we do.

please check here

print_lc() {
local message="$1"
echo -e "$message" | tee /dev/fd/3
}
# Function to log to logfile only
log() {
local message="$1"
echo -e "$message"
}
# Function to run a command where the output will be logged to both console and logfile
run_and_print_lc() {
"$@" | tee /dev/fd/3
}
# Function to log to console only
print_c() {
local message="$1"
echo -e "$message" 1>&3
}
# Function to clear console screen
clear_c() {
clear 1>&3
}
# Generic emergency error handler that exits the script immediately
# Print additional custom message if passed as first argument
# Examples:
# a command || exit_on_error
# a command || exit_on_error "Execution of command failed"
exit_on_error () {
print_lc "\n****************************************"
print_lc "ERROR OCCURRED!
A non-recoverable error occurred.
Check install log for details:"
print_lc "$INSTALLATION_LOGFILE"
print_lc "****************************************"
if [[ -n $1 ]]; then
print_lc "$1"
print_lc "****************************************"
fi
log "Abort!"
exit 1
}

@s-martin
Copy link
Collaborator

s-martin commented Nov 7, 2024

Fixed in #2425

@s-martin s-martin closed this as completed Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug future3 Relates to future3 development needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants