From 56ba10365c3bbf7ad33dffbf33c96535d2347fb1 Mon Sep 17 00:00:00 2001
From: Robin Roth <robin-roth@online.de>
Date: Wed, 4 May 2016 21:32:08 +0200
Subject: [PATCH] better fix for arch version detection (#15705)

* better fix for arch version detection

fixes  #15696

* be extra safe about tracebacks in facts.py

* add comments to explain the setup
* make allowempty more conservative, ignore file content
* wrap function call in try/except
  * should never happen, but if it happens the bug should be distribtion=N/A and not a traceback
---
 lib/ansible/module_utils/facts.py | 38 ++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py
index d847876f48b..690403f8106 100644
--- a/lib/ansible/module_utils/facts.py
+++ b/lib/ansible/module_utils/facts.py
@@ -604,6 +604,10 @@ class Distribution(object):
     This is unit tested. Please extend the tests to cover all distributions if you have them available.
     """
 
+    # every distribution name mentioned here, must have one of
+    #  - allowempty == True
+    #  - be listed in SEARCH_STRING
+    #  - have a function get_distribution_DISTNAME implemented
     OSDIST_LIST = (
         {'path': '/etc/oracle-release', 'name': 'OracleLinux'},
         {'path': '/etc/slackware-version', 'name': 'Slackware'},
@@ -687,12 +691,12 @@ class Distribution(object):
 
                 if not os.path.exists(path):
                     continue
+                # if allowempty is set, we only check for file existance but not content
+                if 'allowempty' in ddict and ddict['allowempty']:
+                    self.facts['distribution'] = name
+                    break
                 if os.path.getsize(path) == 0:
-                    if 'allowempty' in ddict and ddict['allowempty']:
-                        self.facts['distribution'] = name
-                        break
-                    else:
-                        continue
+                    continue
 
                 data = get_file_content(path)
                 if name in self.SEARCH_STRING:
@@ -707,13 +711,19 @@ class Distribution(object):
                     break
                 else:
                     # call a dedicated function for parsing the file content
-                    distfunc = getattr(self, 'get_distribution_' + name)
-                    parsed = distfunc(name, data, path)
-                    if parsed is None or parsed:
-                        # distfunc return False if parsing failed
-                        # break only if parsing was succesful
-                        # otherwise continue with other distributions
-                        break
+                    try:
+                        distfunc = getattr(self, 'get_distribution_' + name)
+                        parsed = distfunc(name, data, path)
+                        if parsed is None or parsed:
+                            # distfunc return False if parsing failed
+                            # break only if parsing was succesful
+                            # otherwise continue with other distributions
+                            break
+                    except AttributeError:
+                        # this should never happen, but if it does fail quitely and not with a traceback
+                        pass
+
+
 
                     # to debug multiple matching release files, one can use:
                     # self.facts['distribution_debug'].append({path + ' ' + name:
@@ -780,10 +790,6 @@ class Distribution(object):
         if release:
             self.facts['distribution_release'] = release.groups()[0]
 
-    def get_distribution_Archlinux(self, name, data, path):
-        self.facts['distribution'] = 'Archlinux'
-        self.facts['distribution_version'] = data
-
     def get_distribution_Alpine(self, name, data, path):
         self.facts['distribution'] = 'Alpine'
         self.facts['distribution_version'] = data