Issue #273: converts diff.groovy to use Checkstyle's CLI

.ci/validation.sh

@@ -40,6 +40,7 @@ checkstyle-tester-diff-groovy-patch)
   cp projects-to-test-on.properties ../.ci-temp/projects-to-test-on.properties
   sed -i'' 's/^guava/#guava/' ../.ci-temp/projects-to-test-on.properties
   sed -i'' 's/#checkstyle/checkstyle/' ../.ci-temp/projects-to-test-on.properties
+  export JDK_JAVA_OPTIONS="-Xmx2048m"
   groovy diff.groovy -l ../.ci-temp/projects-to-test-on.properties \
     -c my_check.xml -b master -p patch-branch -r ../.ci-temp/checkstyle
   ;;
@@ -49,6 +50,7 @@ checkstyle-tester-diff-groovy-base-patch)
   cd .ci-temp/checkstyle
   git checkout -b patch-branch
   cd ../../checkstyle-tester
+  export JDK_JAVA_OPTIONS="-Xmx2048m"
   groovy diff.groovy -l projects-to-test-on.properties \
     -bc my_check.xml -pc my_check.xml -b master -p patch-branch -r ../.ci-temp/checkstyle
   ;;
@@ -58,6 +60,7 @@ checkstyle-tester-diff-groovy-patch-only)
   cd .ci-temp/checkstyle
   git checkout -b patch-branch
   cd ../../checkstyle-tester
+  export JDK_JAVA_OPTIONS="-Xmx2048m"
   groovy diff.groovy -l projects-to-test-on.properties \
     -pc my_check.xml -p patch-branch -r ../.ci-temp/checkstyle -m single
   ;;
@@ -72,10 +75,10 @@ checkstyle-tester-diff-groovy-regression-single)
   cd .ci-temp/contribution/checkstyle-tester
   sed -i'' 's/^guava/#guava/' projects-to-test-on.properties
   sed -i'' 's/#checkstyle|/checkstyle|/' projects-to-test-on.properties
-  export MAVEN_OPTS="-Xmx2048m"
+  export JDK_JAVA_OPTIONS="-Xmx2048m"
   groovy ./diff.groovy --listOfProjects projects-to-test-on.properties \
     -pc ../../../checkstyle-tester/diff-groovy-regression-config.xml \
-    -r ../../checkstyle -xm "-Dcheckstyle.failsOnError=false" \
+    -r ../../checkstyle -ce \
     -m single -p master
 
   # Run report with current branch
@@ -85,7 +88,7 @@ checkstyle-tester-diff-groovy-regression-single)
   rm -rf reports repositories
   groovy ./diff.groovy --listOfProjects projects-to-test-on.properties \
     -pc diff-groovy-regression-config.xml -r ../.ci-temp/checkstyle/ \
-    -m single -p master -xm "-Dcheckstyle.failsOnError=false"
+    -m single -p master -ce
 
   cd ..
   # We need to ignore file paths below, since they will be different between reports

.gitignore

@@ -7,9 +7,7 @@ target
 .idea/
 bin
 *~
-/checkstyle-tester/src/main/java/
 /checkstyle-tester/repositories
-/checkstyle-tester/target
 /checkstyle-tester/reports
 /checkstyle-tester/tmp_reports
 /qulice/src/main/java/

checkstyle-tester/README.md

@@ -63,15 +63,17 @@ will be analyzed by Checkstyle during report generation.
 This option is useful for Windows users where they are restricted to maximum directory depth
 (optional, default is false).
 
-**extraMvnRegressionOptions** (xm) - this option can be used to supply extra command line arguments
-to maven during the Checkstyle regression run.  For example, if you want to skip site generation, you
-would add `--extraMvnRegressionOptions "-Dmaven.site.skip=true"` to `diff.groovy` execution.
+**extraOptions** (xo) - this option can be used to pass CLI options for the Checkstyle execution.
 
 **allowExcludes** (g) - this option tells `diff.groovy` to allow paths and files defined in the file
  specified for the `--listOfProjects (-l)` argument to be excluded from report generation
  (optional, default is false). This option is
  used for files that are not compilable or that Checkstyle cannot parse.
 
+**continueOnError** (ce) - whether to fail or continue the Checkstyle run when it reports a
+ non-zero return code.
+(optional, default is false)
+
 ## Outputs
 
 When the script finishes its work the following directory structure will be created

checkstyle-tester/diff.groovy

@@ -74,8 +74,10 @@ def getCliOptions(args) {
             + ' as a shorter version to prevent long paths. (optional, default is false)')
         m(longOpt: 'mode', args: 1, required: false, argName: 'mode', 'The mode of the tool:' \
             + ' \'diff\' or \'single\'. (optional, default is \'diff\')')
-        xm(longOpt: 'extraMvnRegressionOptions', args: 1, required: false, 'Extra arguments to pass to Maven' \
-            + 'for Checkstyle Regression run (optional, ex: -Dmaven.prop=true)')
+        ce(longOpt: 'continueOnError', required: false, 'Whether to fail or continue the Checkstyle' \
+            + ' run when it reports a non-zero return code. (optional, default is false)')
+        xo(longOpt: 'extraOptions', args: 1, required: false, 'Extra arguments to pass ' \
+            + 'for Checkstyle execution (optional, ex: -Dprop=true)')
     }
     return cli.parse(args)
 }
@@ -218,11 +220,11 @@ def launchCheckstyleReport(cfg) {
 
     // If "no exception" testing, these may not be defined in repos other than checkstyle
     if (isRegressionTesting) {
-        println "Installing Checkstyle artifact ($cfg.branch) into local Maven repository ..."
+        println "Building Checkstyle artifact ($cfg.branch) ..."
         executeCmd("git checkout $cfg.branch", cfg.localGitRepo)
         executeCmd("git log -1 --pretty=MSG:%s%nSHA-1:%H", cfg.localGitRepo)
-        executeCmd("mvn -e --no-transfer-progress --batch-mode -Pno-validations clean install",
-            cfg.localGitRepo)
+        executeCmd("mvn -e --no-transfer-progress --batch-mode clean package -Pno-validations,assembly " +
+            "-Dassembly.skipAssembly=true -Dmaven.test.skip=true", cfg.localGitRepo)
     }
 
     cfg.checkstyleVersion =
@@ -247,11 +249,10 @@ def launchCheckstyleReport(cfg) {
 def generateCheckstyleReport(cfg) {
     println 'Testing Checkstyle started'
 
-    def targetDir = 'target'
-    def srcDir = getOsSpecificPath("src", "main", "java")
+    def allJar = getOsSpecificPath(cfg.localGitRepo.path, "target", "checkstyle-" + cfg.checkstyleVersion + "-all.jar")
     def reposDir = 'repositories'
     def reportsDir = 'reports'
-    makeWorkDirsIfNotExist(srcDir, reposDir, reportsDir)
+    makeWorkDirsIfNotExist(reposDir, reportsDir)
 
     final REPO_NAME_PARAM_NO = 0
     final REPO_TYPE_PARAM_NO = 1
@@ -261,11 +262,11 @@ def generateCheckstyleReport(cfg) {
     final FULL_PARAM_LIST_SIZE = 5
 
     def checkstyleConfig = cfg.checkstyleCfg
-    def checkstyleVersion = cfg.checkstyleVersion
     def allowExcludes = cfg.allowExcludes
     def listOfProjectsFile = new File(cfg.listOfProjects)
     def projects = listOfProjectsFile.readLines()
-    def extraMvnRegressionOptions = cfg.extraMvnRegressionOptions
+    def continueOnError = cfg.continueOnError
+    def extraOptions = cfg.extraOptions
 
     projects.each {
         project ->
@@ -287,27 +288,27 @@ def generateCheckstyleReport(cfg) {
                     excludes = params[REPO_EXCLUDES_PARAM_NO]
                 }
 
-                deleteDir(srcDir)
+                def srcDir
+                def saveDir = getOsSpecificPath("$reportsDir", "$repoName")
                 if (repoType == 'local') {
-                    copyDir(repoUrl, getOsSpecificPath("$srcDir", "$repoName"))
+                    srcDir = new File("$repoUrl").getCanonicalPath()
                 } else {
                     cloneRepository(repoName, repoType, repoUrl, commitId, reposDir)
-                    copyDir(getOsSpecificPath("$reposDir", "$repoName"), getOsSpecificPath("$srcDir", "$repoName"))
+                    srcDir = getOsSpecificPath("$reposDir", "$repoName")
                 }
-                runMavenExecution(srcDir, excludes, checkstyleConfig,
-                    checkstyleVersion, extraMvnRegressionOptions)
-                def repoPath = repoUrl
-                if (repoType != 'local') {
-                    repoPath = new File(getOsSpecificPath("$reposDir", "$repoName")).absolutePath
-                }
-                postProcessCheckstyleReport(targetDir, repoName, repoPath)
-                deleteDir(getOsSpecificPath("$srcDir", "$repoName"))
-                moveDir(targetDir, getOsSpecificPath("$reportsDir", "$repoName"))
+
+                def mainClassOptions = [
+                    srcDir: srcDir,
+                    excludes: excludes,
+                    checkstyleConfig: checkstyleConfig,
+                    saveDir: saveDir,
+                    continueOnError: continueOnError,
+                    extraOptions: extraOptions,
+                ]
+
+                runCliExecution(allJar, mainClassOptions)
             }
     }
-
-    // restore empty_file to make src directory tracked by git
-    new File(getOsSpecificPath("$srcDir", "empty_file")).createNewFile()
 }
 
 def getLastCheckstyleCommitSha(gitRepo, branch) {
@@ -538,11 +539,7 @@ def getFilenameWithoutExtension(filename) {
     return filenameWithoutExtension
 }
 
-def makeWorkDirsIfNotExist(srcDirPath, repoDirPath, reportsDirPath) {
-    def srcDir = new File(srcDirPath)
-    if (!srcDir.exists()) {
-        srcDir.mkdirs()
-    }
+def makeWorkDirsIfNotExist(repoDirPath, reportsDirPath) {
     def repoDir = new File(repoDirPath)
     if (!repoDir.exists()) {
         repoDir.mkdir()
@@ -629,36 +626,39 @@ def getProjectsStatistic(diffDir) {
     return projectsStatistic
 }
 
-def runMavenExecution(srcDir, excludes, checkstyleConfig,
-                      checkstyleVersion, extraMvnRegressionOptions) {
-    println "Running 'mvn clean' on $srcDir ..."
-    def mvnClean = "mvn -e --no-transfer-progress --batch-mode clean"
-    executeCmd(mvnClean)
-    println "Running Checkstyle on $srcDir ... with excludes {$excludes}"
-    def mvnSite = "mvn -e --no-transfer-progress --batch-mode site " +
-        "-Dcheckstyle.config.location=$checkstyleConfig -Dcheckstyle.excludes=$excludes"
-    if (checkstyleVersion) {
-        mvnSite = mvnSite + " -Dcheckstyle.version=$checkstyleVersion"
+def runCliExecution(allJar, mainClassOptions) {
+    def saveLoc = new File(mainClassOptions.saveDir)
+    if (!saveLoc.exists()) {
+        saveLoc.mkdirs()
     }
-    if (extraMvnRegressionOptions) {
-        mvnSite = mvnSite + " "
 
-        if (!extraMvnRegressionOptions.startsWith("-")) {
-            mvnSite = mvnSite + "-"
+    println "Running Checkstyle CLI on ${mainClassOptions.srcDir} ... with excludes {${mainClassOptions.excludes}}"
+    def cliCommand = "java -jar $allJar -c ${mainClassOptions.checkstyleConfig} " +
+        "${mainClassOptions.srcDir} -f xml -o " +
+        getOsSpecificPath("${mainClassOptions.saveDir}", "checkstyle-result.xml") +
+        " -e " + getOsSpecificPath("${mainClassOptions.srcDir}", ".git") +
+        " -e " + getOsSpecificPath("${mainClassOptions.srcDir}", ".hg")
+
+    if (mainClassOptions.excludes.length() > 0) {
+        for (String singleExclude : mainClassOptions.excludes.split(',')) {
+            if (isWindows()) {
+                singleExclude = singleExclude.replace("/", "\\")
+            }
+            cliCommand = cliCommand + " -x " + singleExclude
         }
-        mvnSite = mvnSite + extraMvnRegressionOptions
     }
-    println(mvnSite)
-    executeCmd(mvnSite)
-    println "Running Checkstyle on $srcDir - finished"
-}
 
-def postProcessCheckstyleReport(targetDir, repoName, repoPath) {
-    new AntBuilder().replace(
-        file: getOsSpecificPath("$targetDir", "checkstyle-result.xml"),
-        token: new File(getOsSpecificPath("src", "main", "java", "$repoName")).absolutePath,
-        value: getOsSpecificPath("$repoPath")
-    )
+    if (mainClassOptions.extraOptions) {
+        cliCommand = cliCommand + " "
+
+        if (!mainClassOptions.extraOptions.startsWith("-")) {
+            cliCommand = cliCommand + "-"
+        }
+        cliCommand = cliCommand + mainClassOptions.extraOptions
+    }
+
+    executeCliCmd(cliCommand, mainClassOptions.continueOnError)
+    println "Running Checkstyle CLI on ${mainClassOptions.srcDir} - finished"
 }
 
 def copyDir(source, destination) {
@@ -688,6 +688,17 @@ def executeCmd(cmd, dir = new File("").absoluteFile) {
     }
 }
 
+def executeCliCmd(cmd, continueOnError) {
+    println "Running command: ${cmd}"
+    def osSpecificCmd = getOsSpecificCmd(cmd)
+    def proc = osSpecificCmd.execute(null, new File("").absoluteFile)
+    proc.consumeProcessOutput(System.out, System.err)
+    proc.waitFor()
+    if (!continueOnError && proc.exitValue() != 0) {
+        throw new GroovyRuntimeException("Error: ${proc.err.text}!")
+    }
+}
+
 def getOsSpecificCmd(cmd) {
     def osSpecificCmd
     if (System.properties['os.name'].toLowerCase().contains('windows')) {
@@ -746,6 +757,7 @@ class Config {
     def shortFilePaths
     def listOfProjects
     def mode
+    def continueOnError
 
     def baseBranch
     def patchBranch
@@ -761,7 +773,7 @@ class Config {
     def tmpMasterReportsDir
     def tmpPatchReportsDir
     def diffDir
-    def extraMvnRegressionOptions
+    def extraOptions
 
     def checkstyleVersion
     def sevntuVersion
@@ -774,7 +786,8 @@ class Config {
 
         shortFilePaths = cliOptions.shortFilePaths
         listOfProjects = cliOptions.listOfProjects
-        extraMvnRegressionOptions = cliOptions.extraMvnRegressionOptions
+        extraOptions = cliOptions.extraOptions
+        continueOnError = cliOptions.continueOnError
 
         checkstyleVersion = cliOptions.checkstyleVersion
         allowExcludes = cliOptions.allowExcludes
@@ -824,8 +837,9 @@ class Config {
             checkstyleCfg: baseConfig,
             listOfProjects: listOfProjects,
             destDir: tmpMasterReportsDir,
-            extraMvnRegressionOptions: extraMvnRegressionOptions,
+            extraOptions: extraOptions,
             allowExcludes:allowExcludes,
+            continueOnError:continueOnError,
         ]
     }
 
@@ -836,8 +850,9 @@ class Config {
             checkstyleCfg: patchConfig,
             listOfProjects: listOfProjects,
             destDir: tmpPatchReportsDir,
-            extraMvnRegressionOptions: extraMvnRegressionOptions,
+            extraOptions: extraOptions,
             allowExcludes: allowExcludes,
+            continueOnError:continueOnError,
         ]
     }