ChrisTitusTech
Get system info on command start and export it to the scripts
Pull Request
Title
Get system info on command start and export it to the scripts
Type of Change
- [x] New feature
- [ ] Bug fix
- [ ] Documentation Update
- [x] Refactoring
- [ ] Hotfix
- [ ] Security patch
- [ ] UI/UX improvement
Description
We could lower the barrier of entry for contributing commands by providing a "shell" that takes care of any distro specific tasks that the scripts can then leverage. This is the first step towards that.
Instead of relying on every script having to import common-script.sh, it's very easy for us to do that detection before running and export those variables to the temporary shell.
I'm hoping this will also reduce:
if command -v apt-get >/dev/null 2>&1; then
sudo apt-get update
sudo apt-get install -y zsh
elif command -v yum >/dev/null 2>&1; then
sudo yum install -y zsh
elif command -v dnf >/dev/null 2>&1; then
sudo dnf install -y zsh
elif command -v pacman >/dev/null 2>&1; then
sudo pacman -Syu zsh
elif command -v zypper >/dev/null 2>&1; then
sudo zypper install -y zsh
else
echo "No compatible package manager found. Please install zsh manually." >&2
exit 1
fi
and the like by also providing variables for install, remove and update commands.
Testing
I added the info.sh command which should display all the variables that are exported. I have only tested it with my distro, so if you're running something other than Fedora I would appreciate a quick check on this.
Impact
Easier command development.
Additional Information
I just want to be sure I accounted for all the distros/package managers.
Checklist
- [x] My code adheres to the coding and style guidelines of the project.
- [x] I have performed a self-review of my own code.
- [x] I have commented my code, particularly in hard-to-understand areas.
- [ ] I have made corresponding changes to the documentation.
- [x] My changes generate no errors/warnings/merge conflicts.
Package names are different between distros, so I'm not convinced that this is a particularly good idea in general. This execution is not particularly good either. Failure to find a package manager should not result in a panic. You've also rewritten a std function and allocated a few strings for no reason. Using a variable containing spaces in execution probably also isn't going to work, try running "apt install" or the equivalent from your distro (including quotes) and you'll see the issue. It's parsed as one command.
@lj3954 Thanks for the feedback! This is my first time writing Rust so I'm not surprised I made mistakes along the way. Do you mind being a little more specific in what I could fix?
As far as the variable goes, wouldn't using eval() work? Or am I thinking about it wrong?
Also, since the package names are different, we could still account for that using $SYS_ID in the scripts no?
Do you mind being a little more specific in what I could fix?
Pull down https://github.com/lj3954/linutil/commit/74b75eb29dc400716f2b0bf4e7abb37954203f46.
wouldn't using eval() work?
Eval usually isn't recommended for security reasons, it's probably better to just split the command out.
we could still account for that using $SYS_ID
Or through the package manager's name. Which is what's already being done throughout the scripts.
@lj3954 I really appreciate you taking the time to do this :pray:
I'm going to implement your changes and put up a new commit. Just want to take some time to learn from it first.
As far as splitting the commands, I see you removed the package manager name from the install, update, and remove commands which makes a ton of sense. I understand your points when it comes to this idea.
What I'm trying to work towards is creating a slight layer of abstraction so the actual scripts are easier to write. I'm also new to daily driving linux so I'm not as familiar with the slight differences between distros. How often would the package names be different?
If they are the same more than they are different, then I would find it helpful if we could reduce the amount of scripts in which we have to check for a specific distro just to know what install command to use.
Example:
Kitty is kitty in most distros as far as I know. So instead of having to do:
case ${PACKAGER} in
pacman)
sudo "${PACKAGER}" -S --noconfirm kitty
;;
*)
sudo "${PACKAGER}" install -y kitty
;;
esac
It could just be "$PACKAGE_MANAGER" "$INSTALL_COMMAND" kitty.
Not to mention that if we ever needed to update the install commands such as in this issue we only have to do it in one place.
@lj3954 Implemented your changes :+1:
Thanks again. I learned a lot of the insane string manipulation features rust has.
Sorry for the inconvenience. We had a massive restructure of the codebase to improve future development. Because of this can you update your PR to the new structure. Thank you for your assistance and contribution.
