Refactoring instance manager

libcodechecker/analyze/log_parser.py

@@ -224,9 +224,9 @@ def remove_file_if_exists(filename):
         os.remove(filename)
 
 
-def parse_compile_commands_json(logfile, parseLogOptions):
+def parse_compile_commands_json(log_data, parseLogOptions):
     """
-    logfile: is a compile command json
+    log_data: content of a compile command json.
     """
 
     output_path = parseLogOptions.output_path
@@ -239,13 +239,11 @@ def parse_compile_commands_json(logfile, parseLogOptions):
     actions = []
     filtered_build_actions = {}
 
-    data = load_json_or_empty(logfile, {})
-
     compiler_includes = {}
     compiler_target = {}
 
     counter = 0
-    for entry in data:
+    for entry in log_data:
         sourcefile = entry['file']
 
         if not os.path.isabs(sourcefile):
@@ -364,17 +362,17 @@ def parse_log(logfilepath, parseLogOptions):
     """
     LOG.debug('Parsing log file: ' + logfilepath)
 
-    with open(logfilepath) as logfile:
-        try:
-            actions = parse_compile_commands_json(logfile, parseLogOptions)
-        except (ValueError, KeyError, TypeError) as ex:
-            if os.stat(logfilepath).st_size == 0:
-                LOG.error('The compile database is empty.')
-            else:
-                LOG.error('The compile database is not valid.')
-            LOG.debug(traceback.format_exc())
-            LOG.debug(ex)
-            sys.exit(1)
+    try:
+        data = load_json_or_empty(logfilepath, {})
+        actions = parse_compile_commands_json(data, parseLogOptions)
+    except (ValueError, KeyError, TypeError) as ex:
+        if os.stat(logfilepath).st_size == 0:
+            LOG.error('The compile database is empty.')
+        else:
+            LOG.error('The compile database is not valid.')
+        LOG.debug(traceback.format_exc())
+        LOG.debug(ex)
+        sys.exit(1)
 
     LOG.debug('Parsing log file done.')
 

libcodechecker/generic_package_context.py

@@ -103,7 +103,10 @@ def __set_version(self):
         """
         Get the package version from the version config file.
         """
-        vfile_data = load_json_or_empty(self.version_file, sys_exit=True)
+        vfile_data = load_json_or_empty(self.version_file)
+
+        if not vfile_data:
+            sys.exit(1)
 
         package_version = vfile_data['version']
         package_build_date = vfile_data['package_build_date']
@@ -352,7 +355,10 @@ def get_context():
 
     pckg_config_file = os.path.join(package_root, "config", "config.json")
     LOG.debug('Reading config: ' + pckg_config_file)
-    cfg_dict = load_json_or_empty(pckg_config_file, sys_exit=True)
+    cfg_dict = load_json_or_empty(pckg_config_file)
+
+    if not cfg_dict:
+        sys.exit(1)
 
     LOG.debug(cfg_dict)
 
@@ -361,7 +367,10 @@ def get_context():
     layout_cfg_file = os.path.join(package_root, "config",
                                    "package_layout.json")
     LOG.debug(layout_cfg_file)
-    lcfg_dict = load_json_or_empty(layout_cfg_file, sys_exit=True)
+    lcfg_dict = load_json_or_empty(layout_cfg_file)
+
+    if not lcfg_dict:
+        sys.exit(1)
 
     # Merge static and runtime layout.
     layout_config = lcfg_dict['static'].copy()

libcodechecker/server/instance_manager.py

@@ -14,30 +14,31 @@
 import getpass
 import json
 import os
-import portalocker
 import psutil
 import socket
 import stat
 
+import portalocker
+
 from libcodechecker.util import load_json_or_empty
 
 
-def __getInstanceDescriptorPath(folder=None):
+def __get_instance_descriptor_path(folder=None):
     if not folder:
         folder = os.path.expanduser("~")
 
     return os.path.join(folder, ".codechecker.instances.json")
 
 
-def __makeInstanceDescriptorFile(folder=None):
-    descriptor = __getInstanceDescriptorPath(folder)
+def __make_instance_descriptor_file(folder=None):
+    descriptor = __get_instance_descriptor_path(folder)
     if not os.path.exists(descriptor):
         with open(descriptor, 'w') as f:
             json.dump([], f)
         os.chmod(descriptor, stat.S_IRUSR | stat.S_IWUSR)
 
 
-def __checkInstance(hostname, pid):
+def __check_instance(hostname, pid):
     """Check if the given process on the system is a valid, running CodeChecker
     for the current user."""
 
@@ -57,32 +58,34 @@ def __checkInstance(hostname, pid):
         return False
 
 
-def __rewriteInstanceFile(append, remove, folder=None):
-    """This helper method reads the user's instance descriptor and manages it
-    eliminating dead records, appending new ones and reserialising the file."""
+def __rewrite_instance_file(append, remove, folder=None):
+    """
+    This helper method reads the user's instance descriptor and manages it
+    eliminating dead records, appending new ones and re-serialising the file.
+    """
+    __make_instance_descriptor_file(folder)
+
+    append_pids = [i['pid'] for i in append]
 
-    __makeInstanceDescriptorFile(folder)
-    with open(__getInstanceDescriptorPath(folder), 'r+') as f:
-        portalocker.lock(f, portalocker.LOCK_EX)
+    # After reading, check every instance if they are still valid and
+    # make sure PID does not collide accidentally with the
+    # to-be-registered instances, if any exists in the append list as it
+    # would cause duplication.
+    #
+    # Also, we remove the records to the given PIDs, if any exists.
+    instances = [i for i in get_instances(folder)
+                 if i['pid'] not in append_pids and
+                 (i['hostname'] + ":" + str(i['pid'])) not in remove]
 
-        # After reading, check every instance if they are still valid and
-        # make sure PID does not collide accidentally with the
-        # to-be-registered instances, if any exists in the append list as it
-        # would cause duplication.
-        #
-        # Also, we remove the records to the given PIDs, if any exists.
-        append_pids = [i['pid'] for i in append]
-        instances = [i for i in json.load(f)
-                     if i['pid'] not in append_pids and
-                     (i['hostname'] + ":" + str(i['pid'])) not in remove and
-                     __checkInstance(i['hostname'], i['pid'])]
+    with open(__get_instance_descriptor_path(folder), 'w') as instance_file:
+        portalocker.lock(instance_file, portalocker.LOCK_EX)
 
         instances = instances + append
 
-        f.seek(0)
-        f.truncate()
-        json.dump(instances, f, indent=2)
-        portalocker.unlock(f)
+        instance_file.seek(0)
+        instance_file.truncate()
+        json.dump(instances, instance_file, indent=2)
+        portalocker.unlock(instance_file)
 
 
 def register(pid, workspace, port, folder=None):
@@ -91,12 +94,12 @@ def register(pid, workspace, port, folder=None):
     descriptor.
     """
 
-    __rewriteInstanceFile([{"pid": pid,
-                            "hostname": socket.gethostname(),
-                            "workspace": workspace,
-                            "port": port}],
-                          [],
-                          folder)
+    __rewrite_instance_file([{"pid": pid,
+                              "hostname": socket.gethostname(),
+                              "workspace": workspace,
+                              "port": port}],
+                            [],
+                            folder)
 
 
 def unregister(pid, folder=None):
@@ -105,15 +108,17 @@ def unregister(pid, folder=None):
     descriptor.
     """
 
-    __rewriteInstanceFile([], [socket.gethostname() + ":" + str(pid)], folder)
+    __rewrite_instance_file([],
+                            [socket.gethostname() + ":" + str(pid)],
+                            folder)
 
 
 def get_instances(folder=None):
     """Returns the list of running servers for the current user."""
 
     # This method does NOT write the descriptor file.
 
-    descriptor = __getInstanceDescriptorPath(folder)
+    descriptor = __get_instance_descriptor_path(folder)
     instances = load_json_or_empty(descriptor, {}, lock=True)
 
-    return [i for i in instances if __checkInstance(i['hostname'], i['pid'])]
+    return [i for i in instances if __check_instance(i['hostname'], i['pid'])]

libcodechecker/util.py

@@ -15,7 +15,6 @@
 import hashlib
 import json
 import os
-import portalocker
 import re
 import shutil
 import signal
@@ -28,6 +27,7 @@
 
 from threading import Timer
 
+import portalocker
 import psutil
 
 from libcodechecker.logger import get_logger
@@ -562,8 +562,7 @@ def check_file_owner_rw(file_to_check):
     return True
 
 
-def load_json_or_empty(path, default=None, kind=None, sys_exit=False,
-                       lock=False):
+def load_json_or_empty(path, default=None, kind=None, lock=False):
     """
     Load the contents of the given file as a JSON and return it's value,
     or default if the file can't be loaded.
@@ -583,14 +582,10 @@ def load_json_or_empty(path, default=None, kind=None, sys_exit=False,
         LOG.warning("Failed to open {0} file: {1}"
                     .format(kind if kind else 'json', path))
         LOG.warning(ex)
-        if sys_exit:
-            sys.exit(1)
     except ValueError as ex:
         LOG.warning("'{1}' is not a valid {0} file."
                     .format(kind if kind else 'json', path))
         LOG.warning(ex)
-        if sys_exit:
-            sys.exit(1)
 
     return ret
 

tests/unit/test_buildcmd_escaping.py

@@ -74,18 +74,16 @@ def __get_cmp_json(self, buildcmd):
                        "command": buildcmd + " -c " + self.src_file_path,
                        "file": self.src_file_path}
 
-        compile_cmds = [compile_cmd]
-        return json.dumps(compile_cmds)
+        return [compile_cmd]
 
     def __get_comp_actions(self, compile_cmd):
         """
         Generate a compilation command json file and parse it
         to return the compilation actions.
         """
         comp_cmd_json = self.__get_cmp_json(compile_cmd)
-        with closing(StringIO(comp_cmd_json)) as text:
-            return log_parser.parse_compile_commands_json(text,
-                                                          ParseLogOptions())
+        return log_parser.parse_compile_commands_json(comp_cmd_json,
+                                                      ParseLogOptions())
 
     def test_buildmgr(self):
         """

tests/unit/test_log_parser.py

@@ -150,26 +150,26 @@ def test_omit_preproc(self):
         """
         Compiler preprocessor actions should be omitted.
         """
-        preprocessor_actions = StringIO('''[
+        preprocessor_actions = [
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -c /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -c /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -E /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -E /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MT /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MT /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MM /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MM /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MF /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MF /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -M /tmp/a.cpp",
-            "file": "/tmp/a.cpp" }]
-        ''')
+             "command": "g++ /tmp/a.cpp -M /tmp/a.cpp",
+             "file": "/tmp/a.cpp"}]
+
         build_actions = \
             log_parser.parse_compile_commands_json(preprocessor_actions,
                                                    ParseLogOptions())
@@ -182,11 +182,10 @@ def test_keep_compile_and_dep(self):
         """ Keep the compile command if -MD is set.
         Dependency generation is done as a side effect of the compilation.
         """
-        preprocessor_actions = StringIO('''[
+        preprocessor_actions = [
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MD /tmp/a.cpp",
-            "file": "/tmp/a.cpp" }]
-        ''')
+             "command": "g++ /tmp/a.cpp -MD /tmp/a.cpp",
+             "file": "/tmp/a.cpp"}]
 
         build_actions = \
             log_parser.parse_compile_commands_json(preprocessor_actions,
@@ -197,14 +196,13 @@ def test_keep_compile_and_dep(self):
     def test_omit_dep_with_e(self):
         """ Skip the compile command if -MD is set together with -E. """
 
-        preprocessor_actions = StringIO('''[
+        preprocessor_actions = [
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -MD -E /tmp/a.cpp",
-            "file": "/tmp/a.cpp" },
+             "command": "g++ /tmp/a.cpp -MD -E /tmp/a.cpp",
+             "file": "/tmp/a.cpp"},
             {"directory": "/tmp",
-            "command": "g++ /tmp/a.cpp -E -MD /tmp/a.cpp",
-            "file": "/tmp/a.cpp" } ]
-        ''')
+             "command": "g++ /tmp/a.cpp -E -MD /tmp/a.cpp",
+             "file": "/tmp/a.cpp"}]
 
         build_actions = \
             log_parser.parse_compile_commands_json(preprocessor_actions,