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

Create install_arch.sh #335

Merged
merged 9 commits into from
May 4, 2024
Merged

Create install_arch.sh #335

merged 9 commits into from
May 4, 2024

Conversation

CooperGerman
Copy link
Contributor

I had this script working on a endeavourOS machine.

@Donkie
Copy link
Owner

Donkie commented Apr 7, 2024

Thank you! I see a lot of similarities between this and the debian one, perhaps the script can be split up into several scripts and then having common parts be located in some scripts/functions/ folder or something?

@CooperGerman
Copy link
Contributor Author

Hi, yes you are right i think some common stuff can be factorized into something as mentioned. I didn't want to take the initiative of this but i can give it a try if you want.

@Donkie
Copy link
Owner

Donkie commented Apr 8, 2024

Yes please do :)

@CooperGerman
Copy link
Contributor Author

CooperGerman commented Apr 8, 2024

It seems like the requirements.txt file needed for install_debian.sh is generated outside of the script itself, is this intended or can i add the

pip install pdm
pdm export -o requirements.txt --without-hashes > requirements.txt

lines to the generic installer ?

@Donkie
Copy link
Owner

Donkie commented Apr 8, 2024

requirements.txt is generated in the CI job (to the root dir) to prevent people from needing to install pdm, which has proved to have quite annoying dependencies on some systems. So that cannot really be changed.

@CooperGerman
Copy link
Contributor Author

Hi, i updated the script and updated the README accordingly.
I only tested it on arch for now. Pre-commit was OK.

Is there a way to ensure it works without me having to setup a VM ?

echo -e "${ORANGE}Your operating system is not supported. Either try to install manually or reach out to spoolman github for support.${NC}"
exit 1
fi
fi
Copy link
Owner

Choose a reason for hiding this comment

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

If /etc/os-release is not available it should also fail

@Donkie
Copy link
Owner

Donkie commented Apr 13, 2024

Hi, i updated the script and updated the README accordingly. I only tested it on arch for now. Pre-commit was OK.

Is there a way to ensure it works without me having to setup a VM ?

I can test it for you on debian

sudo apt-get update || exit 1
if [[ -f /etc/os-release ]]; then
source /etc/os-release
if [[ "$ID_LIKE" == *"debian"* ]]; then
Copy link
Owner

Choose a reason for hiding this comment

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

Turns out on Debian itself, ID_LIKE doesn't exist, only ID does, which is "debian". So you should check for either ID_LIKE like you do here, or "$ID" == *"debian"*

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I check for both ID and ID_LIKE variables, this should do the trick for you.

scripts/install.sh Outdated Show resolved Hide resolved
@Donkie
Copy link
Owner

Donkie commented May 4, 2024

Looks good now and seems to work fine, thank you!

@Donkie Donkie merged commit 1ddd9fb into Donkie:master May 4, 2024
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.

None yet

2 participants