Guess common *-release files if /etc not readable.

[sethmlarson]

Fixes issue #174. Just added the most common *-release files that are found in /etc. If the files aren't actually there then the resulting OSError is caught in _parse_distro_release_file().

[alex]

This order is different from the one you'd get with basenames.sort(), you may want to move the basenames.sort() outside of the try block to preserve equivilant results for with/without permissions on /etc/

[sethmlarson]

Or I could just sort it myself. :) I'll do that.

[alex]

Calling `sort` has the advantage of it being guarnteed to stay correct when some new distro is released, but it doesn't really matter one way or the other.

…

On Thu, Mar 23, 2017 at 1:10 PM, Seth M. Larson ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In distro.py <#175 (comment)>: > - # containing `redhat-release` on top of their own. - basenames.sort() + try: + basenames = os.listdir(_UNIXCONFDIR) + # We sort for repeatability in cases where there are multiple + # distro specific files; e.g. CentOS, Oracle, Enterprise all + # containing `redhat-release` on top of their own. + basenames.sort() + except OSError: + # This may occur when /etc is not readable but we can't be + # sure about the *-release files. Check common entries of + # /etc for information. If they turn out to not be there the + # error is handled in `_parse_distro_release_file()`. + basenames = ['os-release', 'redhat-release', 'system-release', + 'base-release', 'fedora-release', + 'centos-release', 'SuSE-release'] Or I could just sort it myself. :) I'll do that. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#175 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAADBPD0yjbqrI1EepEOF7rc_oQ8gUuQks5roqeLgaJpZM4MnBAQ> .

-- "I disapprove of what you say, but I will defend to the death your right to say it." -- Evelyn Beatrice Hall (summarizing Voltaire) "The people's good is the highest law." -- Cicero GPG Key fingerprint: D1B3 ADC0 E023 8CA6

[sethmlarson]

The list is going to have to be updated anyways. And at least how I understand it the sort() call is only there for reproducibility. If the list is a constant that's definitely reproducible.

[sethmlarson]

Looks like the Python 2.7 builder didn't even start? Cycling.

[nir0s]

centos-release appears twice.

Also, should add arch-release, oem-release, debian_version, gentoo-release, base-release, lsb-release, manjaro-release, oracle-release, sl-release and slackware-version. Unless I misunderstood your intention :)

[sethmlarson]

Oh no you didn't miss the intention :) Can lsb-release be left off because it's handled elsewhere?

[nir0s]

Actually, you can ignore anything in _DISTRO_RELEASE_IGNORE_BASENAMES, so you can remove oem-release, system-release, lsb-release, debian_version and os-release :) So I guess we're left with

basenames = [
    'SuSE-release', 
    'base-release', 
    'centos-release', 
    'fedora-release', 
    'redhat-release', 
    'arch-release', 
    'gentoo-release', 
    'manjaro-release', 
    'oracle-release', 
    'sl-release', 
    'slackware-version'
]

[nir0s]

Um.. also, what about sorting the list? It should be consistent.

[sethmlarson]

@nir0s Just mentioned that we can pre-sort the list to skip the .sort() call inside the try block.

[nir0s]

Please make sure the tests cover this.

[sethmlarson]

I'll add some tests for this.

[nir0s]

Also, please don't forget to squash the commits

[sethmlarson]

Assuming I know how py.test class test cases work this should run all tests in TestOverall except with /etc acting as if it were not listable. Will check this when Travis CI runs.

[sethmlarson]

🎉 @nir0s Ready for re-review. :)