Add "--version" option to the usbguard CLI

[Cropi]

Running usbguard --version should help us handle fixes and issues more easily in the future. By doing so, it's easier to track down which options have been enabled during build time. Fixes #570

Any suggestions on what else to display are welcomed.

[hartwork]

Hi @Cropi,

I had a quick look through the changes and also inspected the output locally:

# ./usbguard --version
usbguard 1.1.2 compiled with:
        Platform:                       x86_64-pc-linux-gnu
        Platform (lsb_release -d)       Description:    Gentoo Linux
        Linux audit support:            1
        Libcapng support:               1
        Seccomp support:                1
        Crypto backend library:         libsodium

Based on what I saw I would like to feedback the following:

• I like the idea of including some additional information to ease reporting.
  At least we can ask for that output from users running a recent enough version.

• Currently, all of that information included is gathered at compile time.
  For two of those, that could be trouble:

  • "Platform"
  • "Platform (lsb_release -d)"

  I suggest to drop these two because of risk of being out of sync with the running
  machine and because gathering them live from C++ code is probably more code
  and effort than healthy here. Also, theyes can be asked for rather easily.

• The Description: part from output of lsb_release -d looks out of place and unintended.

• The new code roughly follows pattern if foo_enabled: print("foo: 1").
  The problem here is that:

  • Absence is not communicated, there will be no line foo: 0 for absent features. That's rather unpractical.
  • A constant value of 1 does not seem to offer much value.
    I would like to suggest adding #else branches and to go to human enabled
    and disabled or similar.

• The use of tabs in the output seems well intended but does not feel "organic".
  I suggest use of only two spaces rather than tabs for a more "human" feel.

• Adding two new files for the new functions seems overkill, given that the new code could live right next to sibling function showTopLevelHelp in file src/CLI/usbguard.cpp.

• Bash completion at scripts/bash_completion/usbguard should probably learn about --version as well.

What do you think?

Best, Sebastian

[Cropi]

Hi Sebastian,

Hi @Cropi,

I had a quick look through the changes and also inspected the output locally:

# ./usbguard --version
usbguard 1.1.2 compiled with:
        Platform:                       x86_64-pc-linux-gnu
        Platform (lsb_release -d)       Description:    Gentoo Linux
        Linux audit support:            1
        Libcapng support:               1
        Seccomp support:                1
        Crypto backend library:         libsodium

Based on what I saw I would like to feedback the following:

* I like the idea of including some additional information to ease reporting.
  At least we can ask for that output from users running a recent enough version.

* Currently, all of that information included is gathered at _compile_ time.
  For two of those, that could be trouble:
  
  * "Platform"
  * "Platform (lsb_release -d)"
  
  I suggest to drop these two because of risk of being out of sync with the running
  machine and because gathering them live from C++ code is probably more code
  and effort than healthy here. Also, theyes can be asked for rather easily.

I am fine either way. I saw these in another open source project, rsyslog , that's why I thought we could give it a try.

* The `Description:    ` part from output of `lsb_release -d` looks out of place and unintended.

* The new code roughly follows pattern `if foo_enabled: print("foo: 1")`.
  The problem here is that:
  
  * Absence is not communicated, there will be no line `foo: 0` for absent features. That's rather unpractical.
  * A constant value of 1 does not seem to offer much value.
    I would like to suggest adding `#else` branches and to go to human `enabled`
    and `disabled` or similar.

I agree.

* The use of tabs in the output seems well intended but does not feel "organic".
  I suggest use of only two spaces rather than tabs for a more "human" feel.

I agree.

* Adding two new files for the new functions seems overkill, given that the new code could live right next to sibling function `showTopLevelHelp` in file `src/CLI/usbguard.cpp`.

* Bash completion at `scripts/bash_completion/usbguard` should probably learn about `--version` as well.

Seems good to me.

What do you think?

Best, Sebastian

I wonder if there is anything else we could include in the output. Maybe if you build usbguard from sources, then the last commit ID could be mentioned as well.

[hartwork]

I wonder if there is anything else we could include in the output. Maybe if you build usbguard from sources, then the last commit ID could be mentioned as well.

@Cropi sounds good! It's a classic way to break the build for release tarballs without a .git folder around but if done properly... 😃