[ZORG] Add new builder for CTU analysis

[gamesh411]

Summary

In order to provide CI coverage for CTU related functionality, a new builder
is introduced. The builder uses CodeChecker tool to drive the analysis, and
run the default Clang SA checkers in CTU mode on open source project TMUX.
CodeChecker: https://github.com/Ericsson/codechecker
TMUX: https://github.com/tmux/tmux

[gamesh411]

Friendly ping @gkistanova. What do you think about this modification? Also, should we add our buildslave to the silent (if I remember correctly it is called staging), or the reporting buildmaster? Thanks!

[gkistanova]

Hello Endre,

Please start from adding a builder to the staging buildbot first. Once it is stable and reliably green, you could move it to the production master.

I have noticed that you do not pass env to any of the custom steps in your factory. Is this intentional?

Please also see my notes inline.

Thanks

Galina

zorg/buildbot/builders/CTUAnalysisBuilder.py

26

Could you elaborate why it is not possible to use the buildbot way of setting build step envars and run the command remotely by ShellCommand? Why do you need to swap a new shell session here?

101

UnifiedTreeBuilder.getCmakeWithNinjaBuildFactory sets the TERM env for you. No need to define TERM and merge, you can just pass env down to UnifiedTreeBuilder.getCmakeWithNinjaBuildFactory as is.

107

The same with extra_configure_args. Just pass it down to UnifiedTreeBuilder.getCmakeWithNinjaBuildFactory as is.

121

Is there a strong reason to have defaults hardcoded as a part of the buildfactory rather than require these being specified as builder params or a properties?

131

Please use RemoveDirectory from buildbot.steps.slave instead. Here and in all similar places in this patch.

[gamesh411]

@gkistanova Your review is very much appreciated!
I have answered the inline comments, and I am currently applying the suggested fixes. After short testing, I will update this revision.

zorg/buildbot/builders/CTUAnalysisBuilder.py

26

source codechecker/venv/bin/activate is necessary, because CodeChecker uses virtualenv to manage Python dependencies, and while it just sets its own specific environmental variables in turn, in my opinion, it is best to rely on the activate script to do this. Also, it is necessary to use it in this prefix form, because the effects of source-ing in a buildstep are not visible in the following buildsteps.

Setting the PATH using realpath is necessary to have absolute paths instead of relative ones. This introduces a dependency on realpath being available, however prior personal experience shows that using absolute paths increase robustness.

An alternative implementation would be to write these steps in a separate bash script, but that would create a dependency from that utility script. In its current form the whole build process does not have any external dependencies, every utility is downloaded/checkek-out from the relevant sources.

In conclusion, I have found this implementation the most optimal for minimizing dependencies and maximizing robustness.

101

Good point, thanks.

107

Same as above.

121

The more I think about it, the more I agree with you. Extracting these into customizable build parameters.

131

Fixing these right now.

[gamesh411]

• Remove redundant parameter handling
• Pass env keyword argument to every ShellCommand step
• Use RemoveDirectory where applicable

[gamesh411]

I have applied the fixes, which seemed fitting. Also see my reasoning behind the command-prefix implementation inline.

CodeChecker repo url and tag as build properties are already handled, and they override default values (see in lines 110-113 in the updated diff), is there a better way? (like providing default values for these properties on the GUI, which currently stays blank and property names and values must both be filled in).

I am creating a new revision for integrating the buildbot into the infrastructure, and I will contact you with the access credentials when it's done.

Cheers,
Endre

[gkistanova]

Hello Endre,

Almost there. :)
There is a couple of minor things to address. Please see my notes inline.

zorg/buildbot/builders/CTUAnalysisBuilder.py

26

Could you add a comment to the source code explaining these, please?

119

Did you miss one rm -rf command, or there is a reason not to use RemoveDirectory here?

[gamesh411]

• Add implementation rationale, fix forgotten build-step

[gamesh411]

zorg/buildbot/builders/CTUAnalysisBuilder.py

119

Thanks for catching this!

[gkistanova]

LGTM.

zorg/buildbot/builders/CTUAnalysisBuilder.py

+from buildbot.process.properties import WithProperties
+from buildbot.steps.shell import ShellCommand
+from buildbot.steps.slave import RemoveDirectory
+
+from zorg.buildbot.builders import UnifiedTreeBuilder
+
+
+def wrapCommandForAnalysis(command, cwd='.'):
+    """ Extends a command line invocation by setting the environment for using
+        the built unified tree binaries, and activating CodeChecker virtualenv.
+        CodeChecker uses virtualenv to manage Python dependencies, it is best
+        to rely on its activate script. Also, it is necessary to use this
+        prefix form, because the effects of source-ing in a buildstep are not
+        visible in the following buildsteps.
+
+        An alternative implementation would be to write these steps in a
+        separate bash script, but that would create a dependency from that
+        utility script. In its current form the whole build process does not
+        have any external dependencies, every utility is downloaded/checkek-out
+        from the relevant sources.
+
+        This implementation is optimal for minimizing dependencies and
+        maximizing robustness.
+
+    Parameters:
+        command (str): The command to be run in the analysis environment.
+        cwd (str): The intended working directory of the command to be run.
+
+    Returns:
+        str: An extended command that uses the analysis environment.
+
+    """
+
+    usingClangAndVenv = (
+        'export PATH="$(realpath build/bin):'
+        '$(realpath codechecker/build/CodeChecker/bin):$PATH" '
+        '&& source codechecker/venv/bin/activate')
+
+    return 'bash -c \'{prefix} && cd "{cwd}" && {command}\''.format(
+        prefix=usingClangAndVenv, command=command, cwd=cwd)
+
+
+def makeAnalysisShellCommand(*args, **kwargs):
+    """ Adjusts the arguments intended for a ShellCommand in order to provide
+    transparent usage of Clang and CodeChecker.
+
+    Parameters:
+        *args (list): Arbitrary positional arguments passed forward to
+            ShellCommand.
+        **kwargs (dict): Arbitrary positional arguments passed forward to
+            ShellCommand, but command, and workdir keys are modified.
+
+    Returns:
+        ShellCommand: An instance, which uses an environment that satisfies
+        the requirements of CodeChecker.
+
+    """
+
+    if 'command' in kwargs:
+        command = kwargs['command']
+        if isinstance(command, list):
+            command = ' '.join(command)
+
+        cwd = kwargs.get('workdir', '.')
+        kwargs['command'] = wrapCommandForAnalysis(command, cwd)
+        kwargs['workdir'] = '.'
+
+    return ShellCommand(*args, **kwargs)
+
+
+def shallowCloneTagCommand(url, tag):
+    """ Assembles a command that makes a shallow clone of a git repository.
+
+    Parameters:
+        url (str): The url of the git repository.
+        tag (str): The needed tag of the the repository. Depending on the git
+                   server hashes may be supported, but its not guaranteed.
+    Returns:
+        str: The command string. Intended to be run in an empty directory.
+
+    """
+
+    return (
+        'git init && '
+        'git remote add origin "{url}" && '
+        'git fetch --depth 1 origin {tag} && '
+        'git checkout FETCH_HEAD').format(url=url, tag=tag)
+
+
+def getFactory(
+        clean=False,
+        extra_configure_args=None,
+        env=None,
+        **kwargs):
+    """ Creates a Builder, which can handle the changes related to the LLVM
+        and Clang codebase with respect to their effects on CTU analysis. Uses
+        CodeChecker as the analysis driver.
+
+    Parameters:
+        clean (bool): Indicates whether Clang should be rebuilt during the
+            current run.
+        extra_configure_args (dict of str: str): Extra arguments provided to
+            the CMake invocation.
+        env (dict of str: str): Extra environmental variables for the shell.
+        **kwargs: Extra keyword argument passed forward to the Builder.
+
+    Returns:
+        A Builder instance that contains steps for running CTU Analysis on TMUX
+        via CodeChecker.
+
+    """
+
+    f = UnifiedTreeBuilder.getCmakeWithNinjaBuildFactory(
+        depends_on_projects=['llvm', 'clang'],
+        checks=[],
+        clean=clean,
+        extra_configure_args=extra_configure_args,
+        env=env,
+        **kwargs)
+
+    # Build properties override default values for CodeChecker repository url
+    # and version tag.
+    cc_url = '%(codechecker_url:-{default})s'.format(
+        default='https://github.com/Ericsson/codechecker.git')
+    cc_tag = '%(codechecker_tag:-{default})s'.format(
+        default='v6.9.1')
+
+    # Start with a clean CodeChecker
+    f.addStep(
+        RemoveDirectory(
+            name='Delete CodeChecker directory',
+            dir='codechecker',
+            haltOnFailure=True))
+
+    # Download CodeChecker source.
+    f.addStep(
+        ShellCommand(
+            name='Download CodeChecker',
+            command=WithProperties(shallowCloneTagCommand(cc_url, cc_tag)),
+            haltOnFailure=True,
+            description=['Download', 'CodeChecker'],
+            workdir='codechecker',
+            env=env))
+
+    # Build CodeChecker virtualenv.
+    f.addStep(
+        ShellCommand(
+            name='Build CodeChecker venv',
+            command=['make', 'venv'],
+            haltOnFailure=True,
+            description=[
+                'Make', 'virtual', 'environment', 'for', 'CodeChecker'],
+            workdir='codechecker',
+            env=env))
+
+    # Build CodeChecker package.
+    f.addStep(
+        makeAnalysisShellCommand(
+            name='Build CodeChecker package',
+            command=['make', 'package'],
+            haltOnFailure=True,
+            description=[
+                'Build', 'CodeChecker', 'package', 'used', 'for', 'analysis'],
+            workdir='codechecker',
+            env=env))
+
+    # Provide default values for analysis target 'tmux'.
+    DEFAULT_TMUX_URL = 'https://github.com/tmux/tmux.git'
+    DEFAULT_TMUX_TAG = '2.6'
+
+    # Build properties override defaults.
+    tmux_url = '%(tmux_url:-{default})s'.format(default=DEFAULT_TMUX_URL)
+    tmux_tag = '%(tmux_tag:-{default})s'.format(default=DEFAULT_TMUX_TAG)
+
+    # Clean tmux.
+    f.addStep(
+        RemoveDirectory(
+            name='Delete tmux',
+            dir='tmux',
+            haltOnFailure=True))
+
+    # Download tmux source.
+    f.addStep(
+        ShellCommand(
+            name='Download tmux',
+            command=WithProperties(
+                shallowCloneTagCommand(tmux_url, tmux_tag)),
+            haltOnFailure=True,
+            description=['Download', 'tmux'],
+            workdir='tmux',
+            env=env))
+
+    # Configure tmux.
+    f.addStep(
+        ShellCommand(
+            name='Configure tmux',
+            command='./autogen.sh && ./configure',
+            haltOnFailure=True,
+            description=['Configure', 'tmux', 'project'],
+            workdir='tmux',
+            env=env))
+
+    # Log tmux with CodeChecker.
+    f.addStep(
+        makeAnalysisShellCommand(
+            name='Log tmux with CodeChecker',
+            command=[
+                'CodeChecker', 'log',
+                '--output', 'compile_commands.json',
+                '-b', 'make'],
+            haltOnFailure=True,
+            description=['Log', 'tmux', 'build', 'with', 'CodeChecker'],
+            workdir='tmux',
+            env=env))
+
+    # Make .sa_args file to disable macro expansion.
+    f.addStep(
+        ShellCommand(
+            name='Create file to disable macro expansion',
+            command=(
+                "echo '-Xclang -analyzer-config -Xclang expand-macros=false '"
+                " > .sa_args"),
+            haltOnFailure=True,
+            description=[
+                'Create', '.sa_args', 'file', 'to', 'disable', 'macro',
+                'expansion'],
+            workdir='tmux',
+            env=env))
+
+    # Clean tmux results.
+    f.addStep(
+        RemoveDirectory(
+            name='Delete tmux results',
+            dir='tmux_results',
+            haltOnFailure=True
+        ))
+
+    # Analyze tmux using CodeChecker.
+    f.addStep(
+        makeAnalysisShellCommand(
+            name='Analyze tmux',
+            command=[
+                'CodeChecker', 'analyze',
+                '--output', 'tmux_results',
+                '--saargs', 'tmux/.sa_args',
+                '--jobs', '1',
+                '--verbose', 'debug',
+                'tmux/compile_commands.json'],
+            haltOnFailure=True,
+            description=['Analyze', 'tmux', 'in', 'CTU', 'mode'],
+            workdir='.',
+            env=env))
+
+    return f