[PATCH] D9600: Add scan-build python implementation

[Devin Coughlin via cfe-commits]

dcoughlin added a comment.

Thanks Laszlo.

I'm still not convinced that all the python package stuff is needed because scan-build is distributed with clang (see my question inline).

Also, what do you think about renaming intercept-build to "log-build" or some of the other alternatives I proposed above? I think it is important for the name of the executable to communicate its purpose.


================
Comment at: tools/scan-build-py/MANIFEST.in:2
@@ +1,3 @@
+include README.md
+include *.txt
+recursive-include libear *
----------------
I see that at https://pypi.python.org/pypi/scan-build it lists clang as a prerequisite. But since scan-build-py will now be distributed as part of clang I don't understand what the point of the standalone tool is. Can you explain why this is needed?

================
Comment at: tools/scan-build-py/libscanbuild/analyze.py:131
@@ +130,3 @@
+def setup_environment(args, destination, wrapper_dir):
+    """ Set up environment for build command to intrepose compiler wrapper. """
+
----------------
Typo: 'intrepose'

================
Comment at: tools/scan-build-py/libscanbuild/driver.py:67
@@ +66,3 @@
+    except Exception:
+        logging.exception("Something unexpected had happened.")
+        return 127
----------------
rizsotto.mailinglist wrote:
> jroelofs wrote:
> > rizsotto.mailinglist wrote:
> > > dcoughlin wrote:
> > > > I think this error message can be improved. Perhaps "Unexpected error running intercept-build"?
> > > this line is printed as:
> > > 
> > >   intercept-build: ERROR: Something unexpected had happened.
> > >   (and the stack-trace)
> > > 
> > > because the logging formating. so, 'intercept-build' and 'error' will be part of the message anyway.
> > Is there a pythonic way of doing llvm crash handlers? I.e. the "here's the steps to reproduce this, a stack trace, and a bug report url" things that clang spits out.
> this crash/exception is not a clang crash. it's more like this program's fault. clang crash reports are recorded already in crash report files (and linked into the html report file).
Maybe this should ask the user to file a bug against scan-build? Can it point to the bugzilla and tell the user what information, files, etc. they should attach to the bug report to help us reproduce the problem? Just saying "Something unexpected happened" is not as user-friendly as it could be because it does not tell the user that the problem is not their fault nor what they should do about it.

================
Comment at: tools/scan-build-py/libscanbuild/runner.py:21
@@ +20,3 @@
+
+def run(opts):
+    """ Entry point to run (or not) static analyzer against a single entry
----------------
dcoughlin wrote:
> This needs documentation on what opts can/should contain.
What I was asking for here is documentation about the 'opts' parameter.  Where does 'opts' come from? Is it json input from a compilation database? What keys should it contain, what is meaning of their values, etc.

It is important to at least document the expected structure of the dictionary. This documentation will help people understand your design and make sure they don't accidentally break some invariant when fixing bugs or adding features in the future.


http://reviews.llvm.org/D9600