[JENKINS-29329] CMAKE_BUILD_TOOL: Fixed by implementing solution

#1
jenkinsci · Aug 15, 2015 · 41b7f6f · 41b7f6f
1 parent 772ca20
commit 41b7f6f
Show file tree

Hide file tree

Showing 4 changed files with 112 additions and 18 deletions.
diff --git a/src/main/java/hudson/plugins/cmake/BuildToolStep.java b/src/main/java/hudson/plugins/cmake/BuildToolStep.java
@@ -32,6 +32,7 @@ public class BuildToolStep extends AbstractDescribableImpl<BuildToolStep> {
 
     private String args;
     private String vars;
+    private boolean withCmake;
 
     /**
      *
@@ -98,6 +99,24 @@ public DescriptorImpl getDescriptor() {
         return (DescriptorImpl) super.getDescriptor();
     }
 
+    /**
+     * Gets whether to run the actual build tool directly (by expanding
+     * <code>$CMAKE_BUILD_TOOL</code>) or to have <code>cmake</code> run the
+     * build tool (by invoking <code>cmake --build</code>).
+     */
+    public boolean getWithCmake() {
+        return withCmake;
+    }
+
+    /**
+     * Sets whether to run the actual build tool directly (by expanding
+     * <code>$CMAKE_BUILD_TOOL</code>) or to have <code>cmake</code> run the
+     * build tool (by invoking <code>cmake --build</code>).
+     */
+    public void setWithCmake(boolean withCmake) {
+        this.withCmake = withCmake;
+    }
+
     /**
      * Gets the content of the form field 'args'.
      */

diff --git a/src/main/java/hudson/plugins/cmake/CmakeBuilder.java b/src/main/java/hudson/plugins/cmake/CmakeBuilder.java
@@ -191,6 +191,7 @@ private CmakeTool getSelectedInstallation() {
 
     public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
             BuildListener listener) throws InterruptedException, IOException {
+        EnvVars exportedEnvVars = new EnvVars();
 
         CmakeTool installToUse = getSelectedInstallation();
         // Raise an error if the cmake installation isn't found
@@ -236,33 +237,45 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher,
                     Util.replaceMacro(this.buildType, envs),
                     Util.replaceMacro(cmakeArgs, envs));
             // invoke cmake
-            if (0 != launcher.launch().pwd(theBuildDir).envs(envs).stdout(listener)
-                    .cmds(cmakeCall).join()) {
+            if (0 != launcher.launch().pwd(theBuildDir).envs(envs)
+                    .stdout(listener).cmds(cmakeCall).join()) {
                 return false; // invokation failed
             }
 
             /* parse CMakeCache.txt to get the actual build tool */
             FilePath cacheFile = theBuildDir.child("CMakeCache.txt");
             String buildTool = cacheFile.act(new BuildToolEntryParser());
             if (buildTool == null) {
-                listener.error("Failed to get CMAKE_MAKE_PROGRAM value from "
-                        + cacheFile.getRemote());
-                return false; // abort build
+                listener.error("Failed to get value for %1s from %2$s",
+                        CmakeBuilder.ENV_VAR_NAME_CMAKE_BUILD_TOOL,
+                        cacheFile.getRemote());
             }
-            // export the variable..
-            EnvVars envVars = new EnvVars(
+            // add variable
+            exportedEnvVars.putIfNotNull(
                     CmakeBuilder.ENV_VAR_NAME_CMAKE_BUILD_TOOL, buildTool);
-            build.getEnvironments().add(Environment.create(envVars));
-//            listener.getLogger().println(
-//                    "Exported CMAKE_BUILD_TOOL=" + buildTool);
+            // export our environment
+            build.getEnvironments().add(Environment.create(exportedEnvVars));
 
             /* invoke each build tool step in build dir */
             for (BuildToolStep step : toolSteps) {
-                ArgumentListBuilder toolCall = buildBuildToolCall(buildTool,
-                        step.getCommandArguments(envVars));
+                ArgumentListBuilder toolCall;
+                if (!step.getWithCmake()) {
+                    // invoke directly
+                    // if buildTool == null, let the unexpanded macro show up in
+                    // the log
+                    final String buildToolMacro = Util.replaceMacro("${"
+                            + CmakeBuilder.ENV_VAR_NAME_CMAKE_BUILD_TOOL + "}",
+                            exportedEnvVars);
+                    toolCall = buildBuildToolCall(buildToolMacro,
+                            step.getCommandArguments(envs));
+                } else {
+                    // invoke through 'cmake --build <dir>'
+                    toolCall = buildBuildToolCallWithCmake(cmakeBin,
+                            theBuildDir, step.getCommandArguments(envs));
+                }
                 if (0 != launcher.launch().pwd(theBuildDir)
-                        .envs(step.getEnvironmentVars(envs, listener)).stdout(listener)
-                        .cmds(toolCall).join()) {
+                        .envs(step.getEnvironmentVars(envs, listener))
+                        .stdout(listener).cmds(toolCall).join()) {
                     return false; // invokation failed
                 }
             }
@@ -337,6 +350,33 @@ private static ArgumentListBuilder buildBuildToolCall(final String toolBin,
         return args;
     }
 
+    /**
+     * Constructs the command line to have the actual build tool invoked with
+     * cmake.
+     *
+     * @param cmakeBin
+     *            the name of the cmake tool binary, either as an absolute or
+     *            relative file system path.
+     * @param theBuildDir
+     *            the build directory path
+     * @param toolArgs
+     *            addional build tool arguments, separated by spaces to pass to
+     *            cmake or {@code null}
+     * @return the argument list, never {@code null}
+     */
+    private static ArgumentListBuilder buildBuildToolCallWithCmake(
+            final String cmakeBin, FilePath theBuildDir, String... toolArgs) {
+        ArgumentListBuilder args = new ArgumentListBuilder();
+
+        args.add(cmakeBin);
+        args.add("--build");
+        args.add(theBuildDir.getRemote());
+        if (toolArgs != null) {
+            args.add(toolArgs);
+        }
+        return args;
+    }
+
     /**
      * Overridden for better type safety.
      */

diff --git a/src/main/resources/hudson/plugins/cmake/BuildToolStep/help-withCmake.html b/src/main/resources/hudson/plugins/cmake/BuildToolStep/help-withCmake.html
@@ -1,9 +1,9 @@
 <div>
 Whether to run the actual build tool directly (by expanding <code>$CMAKE_BUILD_TOOL</code>)
-or to have <code>cmake</code> run the build tool (by invoking <code>cmake --build</code>).
+or to have <code>cmake</code> run the build tool (by invoking <code>cmake --build &lt;dir></code>).
 <ul>
-<li>Leave unchecked for minimal overhead.</li>
+<li>Leave unchecked for minimal performance overhead.</li>
 <li>Check this, if you selected one of the <em>Visual Studio</em> or <em>Xcode</em> generators
-above <strong>AND</strong> if you run CMake v. 3.0 or newer.</li>
+above <strong>AND</strong> if you run CMake v. 2.8.11 or newer.</li>
 <ul>
 </div>
diff --git a/src/test/java/hudson/plugins/cmake/JobBuildTest.java b/src/test/java/hudson/plugins/cmake/JobBuildTest.java
@@ -158,7 +158,7 @@ public void testBuildToolVariableInjected() throws Exception {
     }
 
     /**
-     * Verify that build tool invocations work.
+     * Verify that direct build tool invocations work.
      */
     @Test
     public void testBuildToolStep() throws Exception {
@@ -189,6 +189,41 @@ public void testBuildToolStep() throws Exception {
         j.assertBuildStatusSuccess(build);
     }
 
+    /**
+     * Verify that build tool invocations with 'cmake --build' work.
+     * Test will fail with cmake >= 2.8
+     */
+    @Test
+    public void testBuildToolStepWithCmake() throws Exception {
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.setScm(scm);
+
+        CmakeBuilder cmb = new CmakeBuilder(CmakeTool.DEFAULT, "Unix Makefiles");
+        cmb.setCleanBuild(true);
+        cmb.setSourceDir("src");
+        cmb.setBuildDir("build/debug");
+        p.getBuildersList().add(cmb);
+        // let the build invoke 'cmake --build <dir> clean all'..
+        BuildToolStep step = new BuildToolStep();
+        step.setWithCmake(true);
+        final String makeTargets = "--target all";
+        step.setArgs(makeTargets);
+        cmb.getSteps().add(step);
+        // let the build invoke 'cmake --build <dir> rebuild_cache'..
+        step = new BuildToolStep();
+        step.setWithCmake(true);
+        String makeTargets2 = "--target rebuild_cache";
+        step.setArgs(makeTargets2);
+        cmb.getSteps().add(step);
+
+        FreeStyleBuild build = p.scheduleBuild2(0).get();
+        System.out.println(JenkinsRule.getLog(build));
+        j.assertLogContains(makeTargets, build);
+        j.assertLogContains(makeTargets2, build);
+
+        j.assertBuildStatusSuccess(build);
+    }
+
     // //////////////////////////////////////////////////////////////////
     // inner classes
     // //////////////////////////////////////////////////////////////////