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.

Sign up for GitHub

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

Bug fix: #290 run apt only on required packages #291

Closed
wants to merge 2 commits into from
Closed

Bug fix: #290 run apt only on required packages #291

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a3c5d1
Bug fix: #290 run apt only on required packages
RecursiveFunctions Dec 24, 2023
f8e25ce
better fix for #290 that still upgrades packages.
RecursiveFunctions Dec 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions setup.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,10 @@ if [[ $(cat /etc/os-release | grep -i debian) != "" ]]; then
echo "Client is a Debian-based system. Installing binaries";
echo
echo "*** RUNNING APT-GET UPDATE ***"
sudo apt-get update --allow-releaseinfo-change
if [ $? -ne 0 ]; then echo "ERROR: 'apt-get update' failed with error code: $?"; exit 1; fi

echo
sudo apt update --allow-releaseinfo-change || true
Copy link
Owner

@vicwomg vicwomg Dec 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This will disable the error checking on apt update. If apt fails to update, then the installs may result in outdated and or incompatible packages.

I feel like if there's an error running apt update, it should be addressed by the end user rather than ignoring it.

Copy link
Author

Choose a reason for hiding this comment

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

I added it back in a little further down in the next commit
+14

Altogether, it does check for updates, but if there is an error with apt update, it will output the error message to the user so they can address it. In either case it will still attempt to install pikaraoke

Copy link
Owner

Choose a reason for hiding this comment

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

If you force "|| true" though, wont the $? return 0? If that's the case then line 14 will never throw the error.

On the other point, suppose a bad repository is accessed before the valid pikaraoke dependent-repositories are. The script would error AND the install would occur with out of date repositories. I don't see a way to handle this cleanly.

if [ $? -ne 0 ]; then echo "ERROR: 'apt-get update' failed with error code: $?"; fi
echo "*** INSTALLING REQUIRED BINARIES ***"
sudo apt-get install ffmpeg -y
sudo apt-get install chromium-browser -y
sudo apt-get install chromium-chromedriver -y
sudo apt install --only-upgrade ffmpeg chromium-browser chromium-chromedriver -y
if [ $? -ne 0 ]; then echo "ERROR: Binary dependency installation failed with error code: $?"; exit 1; fi
else
echo "Client is not Debian-based. Skipping binary installation. Please install ffmpeg and chrome manually.";
Expand Down