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

Determine compatibility by package manager #16

Closed
wants to merge 7 commits into from

Conversation

lucasburlingham
Copy link

@lucasburlingham lucasburlingham commented Apr 6, 2022

Hey! Found this tool in this Hacker News post and thought I might check it out.

To summarize, this pull request seeks to accomplish the following things:

  • Make the unsnap script distro-agnostic by determining what package manager the system uses by default
  • Colorize and standardize output to the terminal

Instead of determining if the script is compatible based upon what distro is being used, why not just find out if the system can successfully run the script? That way any distro can be used as long as support for the package manager (and their specific nuances) is there.

Included is the required code to generate scripts for the following package managers:

  • apt
  • pacman
  • yum
  • dnf
  • zypper
  • eopkg

This will fix the following issues (and more to come):

I have tested my changes on Arch Linux and everything seems to be in working order.

@lucasburlingham lucasburlingham marked this pull request as ready for review April 6, 2022 14:51
unsnap Show resolved Hide resolved
@popey
Copy link
Owner

popey commented Apr 6, 2022

Thank you for this. I think you're right, using the package manager to determine is probably a good path rather than using the current method, thanks. Just one minor edit needed.

@lucasburlingham
Copy link
Author

Syntax error corrected. Thank you!

Copy link

@silkeh silkeh left a comment

Choose a reason for hiding this comment

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

I found one minor issue when running this PR on Solus (eopkg). With the suggestion applied it works beautifully, thanks!

case $CANDIDATE in
Ubuntu|LinuxMint|Pop|elementary|Zorin)
DISTRO="$CANDIDATE"
PACKAGEMGR=$(command -v {apt,pacman} | sed "s/.*\///")
Copy link

Choose a reason for hiding this comment

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

I think this should be:

Suggested change
PACKAGEMGR=$(command -v {apt,pacman} | sed "s/.*\///")
PACKAGEMGR=$(command -v {apt,pacman,zypper,dnf,yum,eopkg} | sed "s/.*\///")

@silkeh
Copy link

silkeh commented Jul 3, 2024

@popey: would you be interested in me picking up this changeset in a new PR?

@popey
Copy link
Owner

popey commented Jul 3, 2024

Heya! Thanks for the ping. I haven't been looking at this for a while, but I'm hoping to get some time to clean up this repo shortly. Sorry for the lack of responses.

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.

3 participants