Add support for OpenBSD, NetBSD, and FreeBSD #207
Conversation
|
Seems to be failing the |
| @@ -0,0 +1,4 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Can this be #!/bin/sh instead?
Note that the BSDs do not have bash installed by default. This comment should apply to all shell scripts that are not using any bash specific features.
| @@ -0,0 +1,4 @@ | |||
| #!/bin/bash | |||
| @@ -0,0 +1,4 @@ | |||
| #!/bin/bash | |||
distro.py
Outdated
| @@ -932,9 +932,16 @@ def _lsb_release_info(self): | |||
| cmd = ('lsb_release', '-a') | |||
| stdout = subprocess.check_output(cmd, stderr=devnull) | |||
| except OSError: # Command not found | |||
| return {} | |||
| try: | |||
There was a problem hiding this comment.
Maybe the name of this function should be changed from _lsb_release_info, given that it is not using lsb information? Or create a separate _uname_info function that is used if the OS is not Linux?
There was a problem hiding this comment.
Yeah I think I'm going to do that. Definitely felt weird tagging along on lsb_release's prop names.
distro.py
Outdated
| @@ -960,6 +967,18 @@ def _parse_lsb_release_content(lines): | |||
| props.update({k.replace(' ', '_').lower(): v.strip()}) | |||
| return props | |||
|
|
|||
| @staticmethod | |||
| def _parse_uname_content(lines): | |||
There was a problem hiding this comment.
2 extra pieces of information could be added for FreeBSD:
$ uname -rs
FreeBSD 11.1-RELEASE-p4
-
On FreeBSD the distro.codename() function could output either "RELEASE", "STABLE" or "CURRENT" which are described here: https://www.freebsd.org/doc/handbook/current-stable.html
-
On FreeBSD the distro.build_number() output could be the kernel patch level. FreeBSD releases get regular security/errata patches which get added to the uname output if they affect the kernel. In the above output, the kernel is at patch level "p4". Note that the userland can at times be patched separate from the kernel, leading to userland patchlevel and kernel patchlevel being different. Note that uname only provides kernel details. The way to show userland patch level on FreeBSD is "freebsd-version -u", whilst kernel patch level is shown on uname or exactly the same in "freebsd-version -k".
There was a problem hiding this comment.
-
In my VM I didn't see the
-p4, only the-RELEASE. The thing is that's not really the "codename"? Codename typically is the name of the release like "Trusty" for Ubuntu 14.04. I'm not sure ifRELEASE,STABLEandCURRENTfit that bill enough to use them fordistro.codename()? -
Could the
pbe dropped from thep4, does it have meaning? I'm pretty sure no otherdistro.version()call will result in anything but integers and dots so it would breakdistro.version_parts()'s contract? Not 100% certain here.
|
Thanks @woodsb02 for the review, I'll take a look at the comments and expand FreeBSD support. :) |
|
So I wasn't able to get any additional information from |
|
Pinging @nir0s for a review, this patch should be ready to ship. :) |
|
Thanks! I'll review.. But regardless of that, it would be great to add the coverage. |
|
I'll see what I can do! |
|
So, I think that this PR means it's not longer a "Linux OS Distribution Information API" :) The PR should include changes to the README, docs and code removing the "Linux" specificity. Given that we want to propose your addition for Windows and OSX support, I guess we can call distro an "OS Platform Information API". |
|
@woodsb02, would you also take a look please? |
|
I'll comb through everything and try to remove as much as I can. :) |
|
Thanks @nir0s! :) |
|
Indeed, thanks for this! |
I had to use
unamefor *BSD as it doesn't supplylsb-release,os-release, ordist-releaseinfo the way we expect so instead I collapse uname output into lsb-release properties. Fixes #191