Skip to content

Adds missing exit value checks in install script#848

Merged
fwsGonzo merged 1 commit intoincludeos:devfrom
gurka:dev
Oct 6, 2016
Merged

Adds missing exit value checks in install script#848
fwsGonzo merged 1 commit intoincludeos:devfrom
gurka:dev

Conversation

@gurka
Copy link
Copy Markdown
Contributor

@gurka gurka commented Oct 5, 2016

So this is basically just some additional return / exit value checks in the install scripts to make it more robust. I found most of these problems when trying to install IncludeOS under Debian stable. Most of the build steps failed but the script kept going, and finally outputted "Done! Test your installation with ./test.sh", which was kind of .. not so good. This commit partially solves #620 .

I removed the "sudo" part from the README since the script will use sudo when needed. Using sudo when executing install.sh will also build and install everything as root:root and that is probably not want most users want.

I'm not sure if the check for "sudo" is necessary since all supported systems have sudo installer per default (?). Debian does however not, and that's why I added it.

@jenkins-includeos
Copy link
Copy Markdown

Can one of the admins verify this patch with one of the following commands:

  • "test please" for a one time test run
  • "ok to test" to accept latest and future commits on this pull request for testing
  • "add to whitelist" to add the author to the whitelist

@AndreasAakesson
Copy link
Copy Markdown
Contributor

Yeah, sudo should not be used with install.sh - because of the reason you state; everything gets build and installed as root. Thanks for the PR :)

ok to test

@AndreasAakesson
Copy link
Copy Markdown
Contributor

retest please

@mnordsletten
Copy link
Copy Markdown
Contributor

This looks great. Having it exit at the point of failure will make it a lot easier to identify the error.

@fwsGonzo fwsGonzo merged commit 2955709 into includeos:dev Oct 6, 2016
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.

5 participants