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

Install cleanup #1407

Merged
merged 5 commits into from
Feb 27, 2025
Merged

Install cleanup #1407

merged 5 commits into from
Feb 27, 2025

Conversation

AngelofWoe
Copy link
Contributor

@AngelofWoe AngelofWoe commented Feb 26, 2025

Some small changes to follow some conventions

Shebang as #!/usr/bin/env bash queries the local environ PATH for the location of bash. This makes a script much more portable. However, if someone inserts a bash into PATH that isn't actually bash, some weird stuff can happen - but this is a problem of a user / malicious entity messing with something they should not.

Arrays are slightly safer than strings for globbing/expansion, while still allowing package managers to expand the array into separate installable packages.

@AngelofWoe
Copy link
Contributor Author

AngelofWoe commented Feb 26, 2025

Does not fix #1396. Skipping Bazzite and adding rpm-ostree as #1399 should be fine.

I don't think that ignoring SC2086 is ideal (I prefer using the arrays or a similar solution).

@AngelofWoe
Copy link
Contributor Author

AngelofWoe commented Feb 26, 2025

Also, if you wanted script_failure to (kind of) work. Although I don't think this ever got used.

function script_failure () {
    log_err "An error occurred: $( [ ! -z ${1} ] && echo \"on line ${1}\" || echo '(unknown)' )."
    log_err "Installation failed!"
    exit
}

@AngelofWoe
Copy link
Contributor Author

AngelofWoe commented Feb 26, 2025

Should fix #1396.

I'd rather default to install dependencies, even if Bazzite has them installed by base. A user might have uninstalled something. It shouldn't do any harm as rpm-ostree will just do nothing if all packages are installed already. For whatever reason, if all packages are already installed, rpm-ostree will only report the first in the list is. However, it does check the whole list.

I do note that running rpm-ostree upgrade will have the system want to reboot, if there's an actual upgrade (although it won't reboot automatically).

@dragoonDorise dragoonDorise merged commit 5196228 into dragoonDorise:dev Feb 27, 2025
1 check passed
@AngelofWoe AngelofWoe deleted the install-cleanup branch February 28, 2025 06:58
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.

2 participants