[scan-build-py] reuse command line output parameter for report directory

[rizsotto.mailinglist]

Summary

It's a chunk from D26390

This is a debatable change, but would like to see applied. :) It has two drivers...

• User specify the desired output directory as command line parameter. Then we take this value and compute another one from it (append a random like directory to it). After this we consider the new value as output directory... Before this change, it was named 'output directory' what was given by the user (or the default values), and 'target directory' or 'report directory' what we generated from it. But I think it's the same thing. Therefore would call it the same name. And store in the same place.
• The other points comes from the code perspective. We passing around all relevant parameters via the parsed command object, except the output directory location. This change eliminate the need to pass multiple parameters for many methods.

[rizsotto.mailinglist]

Gentle ping. (There are a couple of other change I would be based on this.)

[jroelofs]

LGTM

[rizsotto.mailinglist]

Thanks Jonathan!

tools/scan-build-py/libscanbuild/analyze.py

 COMPILER_WRAPPER_CXX = 'analyze-c++'
 
 
 @command_entry_point
 def scan_build():
     """ Entry point for scan-build command. """
 
     args = parse_args_for_scan_build()
-    with report_directory(args.output, args.keep_empty) as target_dir:
+    # will re-assign the report directory as new output
+    with report_directory(args.output, args.keep_empty) as args.output:
         # Run against a build command. there are cases, when analyzer run
         # is not required. But we need to set up everything for the
         # wrappers, because 'configure' needs to capture the CC/CXX values
         # for the Makefile.
         if args.intercept_first:
             # Run build command with intercept module.
             exit_code = capture(args)
             # Run the analyzer against the captured commands.
             if need_analyzer(args.build):
-                run_analyzer(args, target_dir)
+                run_analyzer(args)
         else:
             # Run build command and analyzer with compiler wrappers.
-            environment = setup_environment(args, target_dir)
+            environment = setup_environment(args)
             exit_code = run_build(args.build, env=environment)
         # Cover report generation and bug counting.
-        number_of_bugs = document(args, target_dir, False)
+        number_of_bugs = document(args)
         # Set exit status as it was requested.
         return number_of_bugs if args.status_bugs else exit_code
 
 
 @command_entry_point
 def analyze_build():
     """ Entry point for analyze-build command. """
 
     args = parse_args_for_analyze_build()
-    with report_directory(args.output, args.keep_empty) as target_dir:
+    # will re-assign the report directory as new output
+    with report_directory(args.output, args.keep_empty) as args.output:
         # Run the analyzer against a compilation db.
-        run_analyzer(args, target_dir)
+        run_analyzer(args)
         # Cover report generation and bug counting.
-        number_of_bugs = document(args, target_dir, True)
+        number_of_bugs = document(args)
         # Set exit status as it was requested.
         return number_of_bugs if args.status_bugs else 0
 
 
 def need_analyzer(args):
     """ Check the intent of the build command.
 
     When static analyzer run against project configure step, it should be
     silent and no need to run the analyzer or generate report.
 
     To run `scan-build` against the configure step might be neccessary,
     when compiler wrappers are used. That's the moment when build setup
     check the compiler and capture the location for the build process. """
 
     return len(args) and not re.search('configure|autogen', args[0])
 
 
-def run_analyzer(args, output_dir):
+def run_analyzer(args):
     """ Runs the analyzer against the given compilation database. """
 
     def exclude(filename):
         """ Return true when any excluded directory prefix the filename. """
         return any(re.match(r'^' + directory, filename)
                    for directory in args.excludes)
 
     consts = {
         'clang': args.clang,
-        'output_dir': output_dir,
+        'output_dir': args.output,
         'output_format': args.output_format,
         'output_failures': args.output_failures,
         'direct_args': analyzer_params(args),
         'force_debug': args.force_debug
     }
 
     logging.debug('run analyzer against compilation database')
     with open(args.cdb, 'r') as handle:
         generator = (dict(cmd, **consts)
                      for cmd in json.load(handle) if not exclude(cmd['file']))
         # when verbose output requested execute sequentially
         pool = multiprocessing.Pool(1 if args.verbose > 2 else None)
         for current in pool.imap_unordered(run, generator):
             if current is not None:
                 # display error message from the static analyzer
                 for line in current['error_output']:
                     logging.info(line.rstrip())
         pool.close()
         pool.join()
 
 
-def setup_environment(args, destination):
+def setup_environment(args):
     """ Set up environment for build command to interpose compiler wrapper. """
 
     environment = dict(os.environ)
     environment.update(wrapper_environment(args))
     environment.update({
         'CC': COMPILER_WRAPPER_CC,
         'CXX': COMPILER_WRAPPER_CXX,
         'ANALYZE_BUILD_CLANG': args.clang if need_analyzer(args.build) else '',
-        'ANALYZE_BUILD_REPORT_DIR': destination,
+        'ANALYZE_BUILD_REPORT_DIR': args.output,
         'ANALYZE_BUILD_REPORT_FORMAT': args.output_format,
         'ANALYZE_BUILD_REPORT_FAILURES': 'yes' if args.output_failures else '',
         'ANALYZE_BUILD_PARAMETERS': ' '.join(analyzer_params(args)),
         'ANALYZE_BUILD_FORCE_DEBUG': 'yes' if args.force_debug else ''
     })
     return environment
 
 

tools/scan-build-py/libscanbuild/report.py

 import os.path
 import sys
 import shutil
 import itertools
 import plistlib
 import glob
 import json
 import logging
+import datetime
 from libscanbuild import duplicate_check
 from libscanbuild.clang import get_version
 
 __all__ = ['document']
 
 
-def document(args, output_dir, use_cdb):
+def document(args):
     """ Generates cover report and returns the number of bugs/crashes. """
 
     html_reports_available = args.output_format in {'html', 'plist-html'}
 
     logging.debug('count crashes and bugs')
-    crash_count = sum(1 for _ in read_crashes(output_dir))
+    crash_count = sum(1 for _ in read_crashes(args.output))
     bug_counter = create_counters()
-    for bug in read_bugs(output_dir, html_reports_available):
+    for bug in read_bugs(args.output, html_reports_available):
         bug_counter(bug)
     result = crash_count + bug_counter.total
 
     if html_reports_available and result:
+        use_cdb = os.path.exists(args.cdb)
+
         logging.debug('generate index.html file')
-        # common prefix for source files to have sort filenames
+        # common prefix for source files to have sorter path
         prefix = commonprefix_from(args.cdb) if use_cdb else os.getcwd()
         # assemble the cover from multiple fragments
-        try:
-            fragments = []
+        try:
             if bug_counter.total:
-                fragments.append(bug_summary(output_dir, bug_counter))
-                fragments.append(bug_report(output_dir, prefix))
+                fragments.append(bug_summary(args.output, bug_counter))
+                fragments.append(bug_report(args.output, prefix))
             if crash_count:
-                fragments.append(crash_report(output_dir, prefix))
-            assemble_cover(output_dir, prefix, args, fragments)
-            # copy additinal files to the report
-            copy_resource_files(output_dir)
+                fragments.append(crash_report(args.output, prefix))
+            assemble_cover(args, prefix, fragments)
+            # copy additional files to the report
+            copy_resource_files(args.output)
             if use_cdb:
-                shutil.copy(args.cdb, output_dir)
+                shutil.copy(args.cdb, args.output)
         finally:
             for fragment in fragments:
                 os.remove(fragment)
     return result
 
 
-def assemble_cover(output_dir, prefix, args, fragments):
+def assemble_cover(args, prefix, fragments):
     """ Put together the fragments into a final report. """
 
     import getpass
     import socket
-    import datetime
 
     if args.html_title is None:
         args.html_title = os.path.basename(prefix) + ' - analyzer results'
 
-    with open(os.path.join(output_dir, 'index.html'), 'w') as handle:
+    with open(os.path.join(args.output, 'index.html'), 'w') as handle:
         indent = 0
         handle.write(reindent("""
         |<!DOCTYPE html>
         |<html>
         |  <head>
         |    <title>{html_title}</title>
         |    <link type="text/css" rel="stylesheet" href="scanview.css"/>
         |    <script type='text/javascript' src="sorttable.js"></script>

 def commonprefix_from(filename):
     """ Create file prefix from a compilation database entries. """
 
     with open(filename, 'r') as handle:
         return commonprefix(item['file'] for item in json.load(handle))
 
 
 def commonprefix(files):
-    """ Fixed version of os.path.commonprefix. Return the longest path prefix
-    that is a prefix of all paths in filenames. """
+    """ Fixed version of os.path.commonprefix.
 
+    :param files: list of file names.
+    :return: the longest path prefix that is a prefix of all files. """
     result = None
     for current in files:
         if result is not None:
             result = os.path.commonprefix([result, current])
         else:
             result = current
 
     if result is None:
         return ''
     elif not os.path.isdir(result):
         return os.path.dirname(result)
     else:
         return os.path.abspath(result)