Friday, 22 April 2011

Enforcing target descriptions within build files with a Git hook

When automating mundane tasks of a project or development environment with a build tool like Phing or Ant, the driving build file will naturally accumulate several targets and tasks over time. To ease the build file acceptance within a team and at a later stage also the contribution rate by team members, it's crucial that all build targets have a description attribute to provide at least a rough outline of the build features at hand. When these attributes are in place the (potential) build file user will get such an outline by executing the build tool's list command (phing -l or ant -p). To get a better picture of the problem at hand imagine a project poorly covered with tests and your personal attitude towards extending it or just take a peek at the screenshot below showing a very poorly documented build file.

A poorly documented build file in Phing's list view

To overcome this accumulation of some sort of technical debt (i.e. poorly documented targets) there are various options at hand. The first one, not covered in this blog post, would be to add a pursuant test which verifies the existence of a description for every target/task of the build file under test. As it's very uncommon, at least from what I've heard, to have your build files covered by tests; the next thinkable approach would be to use a Git pre-commit hook to guard your repository/ies against the creeping in of such poorly documented build files.

The next listing shows such a Git hook (also available via GitHub) scribbled away in PHP, which detects any build file(s) following a common build file naming schema (i.e. build.xml|build.xml.dist|personal-build.xml|…) , prior to the actual commit. For every target element in the detected build file(s) it's then verified that it has a description attribute and that it's actual content is long enough to carry some meaning. If one of those two requirements aren't met, the commit is rejected while revealing the build file smells to the committer, so she can fix it, as shown in the outro screenshot. Happy build file sniffing.

#!/usr/bin/php
<?php
define('DEPENDENT_EXTENSION', 'SimpleXML');

if (!extension_loaded(DEPENDENT_EXTENSION)) {
    $consoleMessage = sprintf(
        "Skipping build file checks as the '%s' extension isn't available.", 
        DEPENDENT_EXTENSION
    );
    echo $consoleMessage . PHP_EOL;
    exit(0);
}

define('MIN_TARGET_DESCRIPTION_LENGTH', 10);
define('TARGET_DESCRIPTION_ATTRIBUTE', 'description');
define('TARGET_NAME_ATTRIBUTE', 'name');
define('CHECK_DESCRIPTION_LENGTH', true);

$possibleBuildFileNames = array(
    'build.xml.dist',
    'build.xml-dist',
    'build-dist.xml',
    'build.xml',
    'personal-build.xml'
);

$violations = getAllBuildFileViolationsOfCommit($possibleBuildFileNames);
fireBackPossibleViolationsAndExitAccordingly($violations);

function getAllBuildFileViolationsOfCommit(array $possibleBuildFileNames)
{
    $filesOfCommit = array();
    $gitCommand = 'git diff --cached --name-only';
    
    exec($gitCommand, $filesOfCommit, $commandReturnCode);
    
    $allViolations = array();
    foreach ($filesOfCommit as $file) {
      if (in_array(basename($file), $possibleBuildFileNames)) {
          $violations = checkBuildFileForViolations($file);
          if (count($violations) > 0) {
            $allViolations[$file] = $violations;
          }
      }
    }

    return $allViolations;
}

/**
 *  @param  array $allViolations
 *  @return void
 */
function fireBackPossibleViolationsAndExitAccordingly(array $allViolations)
{
    if (count($allViolations) > 0) {
        foreach ($allViolations as $buildFile => $violations) {

            $buildFileConsoleMessageHeader = sprintf("Build file '%s':", $buildFile);
            echo $buildFileConsoleMessageHeader . PHP_EOL;

            foreach ($violations as $violationMessage) {
                $buildFileConsoleMessageLine = sprintf(" + %s", $violationMessage);
                echo $buildFileConsoleMessageLine . PHP_EOL;
            }
        }
        if (count($allViolations) > 1) {
            $rejectCommitConsoleMessage = sprintf(
                "Therefore rejecting the commit of build files [ %s ].", 
                implode(', ', array_keys($allViolations))
            );
        } else {
            $rejectCommitConsoleMessage = sprintf(
                "Therefore rejecting the commit of build file [ %s ].", 
                implode(', ', array_keys($allViolations))
            );
        }

        echo $rejectCommitConsoleMessage . PHP_EOL;
        exit(1);
    }
    exit(0);
}
/**
 *  @param  string $buildfile
 *  @return array
 */
function checkBuildFileForViolations($buildFile) {
    if (!file_exists($buildFile)) {
        return array();
    }

    $buildfileXml = file_get_contents($buildFile);
    $buildXml = new SimpleXMLElement($buildfileXml);
    $allBuildTargets = $buildXml->xpath("//target");
    $violations = array();

    if (count($allBuildTargets) > 0) {

        $targetsWithNoDescription = $targetsWithTooShortDescription = array();

        foreach ($allBuildTargets as $buildTarget) {

            $actualTragetAttributes = $buildTarget->attributes();
            $allUsedTragetAttributes = array();
            $actualTargetName = null;

            foreach ($actualTragetAttributes as $attribute => $value) {
                $allUsedTragetAttributes[] = $attribute;

                if ($attribute === TARGET_NAME_ATTRIBUTE) {
                    $actualTargetName = $value;
                }

                if (CHECK_DESCRIPTION_LENGTH === true && 
                    $attribute === TARGET_DESCRIPTION_ATTRIBUTE && 
                    strlen($value) < MIN_TARGET_DESCRIPTION_LENGTH) 
                {
                    $targetsWithTooShortDescription[] = $actualTargetName;
                }
            }   

            if (!in_array(TARGET_DESCRIPTION_ATTRIBUTE, $allUsedTragetAttributes)) {
                $targetsWithNoDescription[] = $actualTargetName;
            }
        }
        if (count($targetsWithNoDescription) > 0) {
            if (count($targetsWithNoDescription) > 1) {
                $violations[] = sprintf(
                    "Build targets [ %s ] don't have mandatory descriptions.", 
                    implode(', ', $targetsWithNoDescription)
                );
            } else {
                $violations[] = sprintf(
                    "Build target [ %s ] doesn't have a mandatory description.", 
                    implode(', ', $targetsWithNoDescription)
                );
            }
        }

        if (count($targetsWithTooShortDescription) > 0) {
            if (count($targetsWithTooShortDescription) > 1) {
                $violations[] = sprintf(
                    "Build targets [ %s ] don't have an adequate target description length.", 
                    implode(', ', $targetsWithTooShortDescription),
                    MIN_TARGET_DESCRIPTION_LENGTH
                );
            } else {
                $violations[] = sprintf(
                    "Build target [ %s ] doesn't have an adequate target description length.", 
                    implode(', ', $targetsWithTooShortDescription),
                    MIN_TARGET_DESCRIPTION_LENGTH
                );
            }
        }
    }
    return $violations;
}
Non-Descriptive Phing build files rejected by a Git hook

No comments: