emsec / hal

HAL – The Hardware Analyzer
MIT License
619 stars 74 forks source link

install_dependencies.sh continues on error #512

Open nandlab opened 1 year ago

nandlab commented 1 year ago

Without the rapidjson-dev package, CMake exits with an error. Can you please add it to install_dependencies.sh?

joern274 commented 1 year ago

Thanks for the comment. Which operating system / distribution / version are you using?

nandlab commented 1 year ago

My OS is Devuan GNU/Linux 4 (chimaera) x86_64. It is a Debian-based distribution.

In install_dependencies.sh I changed if [ "$distribution" == 'Ubuntu' ] || [ "$distribution" == 'LinuxMint' ] to: if [ "$distribution" == 'Ubuntu' ] || [ "$distribution" == 'LinuxMint' ] || [ "$distribution" == 'Devuan' ]

RenWal commented 1 year ago

rapidjson-dev is in https://github.com/emsec/hal/blob/90b72d4814b5c2ffdcb89f5db2413f82344e09b6/install_dependencies.sh#L78

In fact, it has been a dependency since d6483551cb416fe8aaae12902f769951e32882aa, so unless you're building a version that is almost 4 years old, or you're building something other than master (in this case, please indicate the commit you're building from), rapidjson-dev should be installed as a dependency.

Could you please verify that:

If all of the above apply, are you able to manually install rapidjson-dev, and does that fix the build issue?

nandlab commented 1 year ago

Thank you for the reply!

Sorry, rapidjson-dev was indeed already in install_dependencies.sh, I somehow overlooked it :/ When I execute the install_dependecies.sh script, the apt command shows the following message and no apt packages are installed:

Package apport is not available, but is referred to by another package.
This may mean that the package is missing, has been obsoleted, or
is only available from another source

E: Package 'apport' has no installation candidate`

Apport seems to be an Ubuntu-specific crash-report software and is not available on Debian-based distributions. But the script continued anyway (installed pip requirements) and exited with a result code of 0.

When I remove apport from install_dependencies.sh, install_dependencies.sh runs fine and installs all the required apt packages (except apport of course).

I have two questions:

  1. Is it possible to make install_dependencies.sh exit with non-zero if any subcommand fails?
  2. Is apport really required or is it possible to use HAL without it?
RenWal commented 1 year ago

As a general word of caution; this is the reason why we currently only support builds on Ubuntu (and macOS to some degree). I agree though that the error handling in the install_dependency.sh script could be improved. Would you be willing to send us a pull request?

Regarding apport: Looks like this was introduced in fd5b00ef028b7f22e3f007e2b6e311ac9704069c and its primary use is crash dump collection in our CI.

@joern274 do we need to depend on apport for a local build at all, or is this really just for GitHub CI?

joern274 commented 1 year ago

Must apologize that I haven't fixed this issue already but the plan is to have the modification in install_dependency.sh tested on Debian based virtual machine before updating the master. In case it works flawlessly it might also be a good idea to add an appropriate workflow to GitHub CI.

And yes, apport is the tool to uppload core dump from GitHub CI in case a build problem cannot be reproduced on the local machine.

nandlab commented 1 year ago

You can set -e at the top of the script to "exit immediately if a command exits with a non-zero status" (see help set). I tested it and it did abort with a non-zero exit status when the apt command failed.