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

Added install function to static-get #14

Merged
merged 3 commits into from Sep 5, 2019

Conversation

@the-djdj
Copy link
Contributor

commented Sep 4, 2019

Added the ability to tell static-get to install a downloaded and extracted package.

[new] Added install function
Added the ability to tell static-get to install a downloaded and extracted package.
@javier-lopez
Copy link
Member

left a comment

Thank you for your collaboration 🍻 , I've left a short code review, also, would you consider adding some test cases for the new functionality?, https://github.com/minos-org/minos-static/blob/master/tests/tests.db

Have a nice day!

static-get Outdated
if [ "${install}" ]; then
_mkdir_p "${directory}/${_sget__bfile%%.tar*}"
(cd "${directory}/${_sget__bfile%%.tar*}" && \
${compress_bin} < "../${_sget__bfile}" | tar xf -) && \

This comment has been minimized.

Copy link
@javier-lopez

javier-lopez Sep 5, 2019

Member

Not really sure about the && \ at the end of this line, I think it's unclear at best

static-get Outdated
@@ -369,7 +370,26 @@ _static_get() {
if [ "${_sget__hash}" = "${_sget__hash_new}" ]; then
_cat "${tmpcache}.${uniq_id_mirror}/${_sget__bfile}"

if [ "${extract}" ]; then
if [ "${install}" ]; then
_mkdir_p "${directory}/${_sget__bfile%%.tar*}"

This comment has been minimized.

Copy link
@javier-lopez

javier-lopez Sep 5, 2019

Member

This code seems similar to the one in extract, do you think is it worth to create a function in order to reuse code?

the-djdj added 2 commits Sep 5, 2019
[fix] Removed duplicate code
Created a method for extracting tarball archives so that similar code doesn't have to be repeated
[new] Added test cases
Added test cases for installing packages
@the-djdj

This comment has been minimized.

Copy link
Contributor Author

commented Sep 5, 2019

The issues should be fixed now. I couldn't figure out how to write a test case that invoked superuser privileges, so the tests focus on lack of write access to root.

@javier-lopez javier-lopez merged commit 210df9d into minos-org:master Sep 5, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@maximbaz

This comment has been minimized.

Copy link

commented Sep 10, 2019

Nice feature! It would have been nice if we could configure a prefix, i.e. instead of installing to /bin/ to be able to install to /opt/bin/

@the-djdj

This comment has been minimized.

Copy link
Contributor Author

commented Sep 10, 2019

I'll take a look at that, @maximbaz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.