[JENKINS-69647] Eliminate reflection from WinProcess (#71)

jenkinsci · Sep 22, 2022 · 7c790fd · 7c790fd
1 parent f490553
commit 7c790fd
Show file tree

Hide file tree

Showing 11 changed files with 98 additions and 99 deletions.
diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml
@@ -0,0 +1,7 @@
+<extensions xmlns="http://maven.apache.org/EXTENSIONS/1.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/EXTENSIONS/1.0.0 http://maven.apache.org/xsd/core-extensions-1.0.0.xsd">
+  <extension>
+    <groupId>io.jenkins.tools.incrementals</groupId>
+    <artifactId>git-changelist-maven-extension</artifactId>
+    <version>1.4</version>
+  </extension>
+</extensions>
diff --git a/.mvn/maven.config b/.mvn/maven.config
@@ -0,0 +1,2 @@
+-Pconsume-incrementals
+-Pmight-produce-incrementals
diff --git a/Jenkinsfile b/Jenkinsfile
@@ -0,0 +1,9 @@
+/*
+ * While this is not a plugin, it is much simpler to reuse the pipeline code for CI. This allows for
+ * easy Linux/Windows testing and produces incrementals. The only feature that relates to plugins is
+ * allowing one to test against multiple Jenkins versions.
+ */
+buildPlugin(useContainerAgent: true, configurations: [
+  [ platform: 'linux', jdk: '11' ],
+  [ platform: 'linux', jdk: '17' ],
+])
diff --git a/pom.xml b/pom.xml
@@ -1,34 +1,39 @@
+<?xml version="1.0" encoding="UTF-8"?>
 <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
   <parent>
-    <groupId>org.kohsuke</groupId>
-    <artifactId>pom</artifactId>
-    <version>17</version>
+    <groupId>org.jenkins-ci</groupId>
+    <artifactId>jenkins</artifactId>
+    <version>1.88</version>
+    <relativePath />
   </parent>
 
   <modelVersion>4.0.0</modelVersion>
   <groupId>org.jvnet.winp</groupId>
   <artifactId>winp</artifactId>
-  <version>1.29-SNAPSHOT</version>
+  <version>${revision}${changelist}</version>
   <name>winp</name>
   <description>Kill process tree in Windows</description>
+  <url>https://github.com/jenkinsci/${project.artifactId}</url>
 
   <properties>
-    <findbugs-maven-plugin.version>3.0.4</findbugs-maven-plugin.version>
-    <findbugs.failOnError>true</findbugs.failOnError>
+    <revision>1.29</revision>
+    <changelist>-SNAPSHOT</changelist>
+    <gitHubRepo>jenkinsci/${project.artifactId}</gitHubRepo>
+    <!-- TODO fix violations -->
+    <spotbugs.threshold>High</spotbugs.threshold>
   </properties>
 
   <scm>
-    <connection>scm:git:git@github.com/kohsuke/${project.artifactId}.git</connection>
-    <developerConnection>scm:git:ssh://git@github.com/kohsuke/${project.artifactId}.git</developerConnection>
-    <url>http://${project.artifactId}.kohsuke.org/</url>
-    <tag>HEAD</tag>
+    <connection>scm:git:https://github.com/${gitHubRepo}.git</connection>
+    <developerConnection>scm:git:git@github.com:${gitHubRepo}.git</developerConnection>
+    <url>https://github.com/${gitHubRepo}</url>
+    <tag>${scmTag}</tag>
   </scm>
 
   <build>
     <plugins>
       <plugin>
         <artifactId>maven-jar-plugin</artifactId>
-        <version>3.0.2</version>
         <configuration>
           <archive>
             <manifest>
@@ -39,78 +44,63 @@
       </plugin>
       <plugin>
         <artifactId>maven-surefire-plugin</artifactId>
-        <version>2.19.1</version>
         <configuration>
-            <argLine>-XX:+CreateMinidumpOnCrash</argLine>
-            <trimStackTrace>false</trimStackTrace>
-        </configuration>
-      </plugin>
-      <plugin>
-        <groupId>org.codehaus.mojo</groupId>
-        <artifactId>findbugs-maven-plugin</artifactId>
-        <version>${findbugs-maven-plugin.version}</version>
-        <configuration>
-          <effort>Max</effort>
-          <!--TODO: a couple of safe issues, so default to medium for a while <threshold>Low</threshold>-->
-          <xmlOutput>true</xmlOutput>
-          <findbugsXmlOutput>false</findbugsXmlOutput>
-        </configuration>
-        <executions>
-          <execution>
-            <id>findbugs</id>
-            <goals>
-              <goal>check</goal>
-            </goals>
-            <phase>verify</phase>
-          </execution>
-        </executions>
-      </plugin>
-      <plugin>
-        <artifactId>maven-release-plugin</artifactId>
-        <version>2.5.2</version>
-        <configuration>
-          <releaseProfiles>release</releaseProfiles>
+            <argLine>-XX:+CreateCoredumpOnCrash</argLine>
         </configuration>
       </plugin>
     </plugins>
   </build>
 
-  <reporting>
-    <plugins>
-      <plugin>
-        <artifactId>maven-javadoc-plugin</artifactId>
-        <version>2.10.4</version>
-      </plugin>
-    </plugins>
-  </reporting>
-
   <dependencies>
     <dependency>
-      <groupId>junit</groupId>
-      <artifactId>junit</artifactId>
-      <version>4.13.1</version>
+      <groupId>com.github.spotbugs</groupId>
+      <artifactId>spotbugs-annotations</artifactId>
+      <optional>true</optional>
+      <exclusions>
+        <exclusion>
+          <groupId>com.google.code.findbugs</groupId>
+          <artifactId>jsr305</artifactId>
+        </exclusion>
+      </exclusions>
+    </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter</artifactId>
       <scope>test</scope>
     </dependency>
     <dependency>
-      <groupId>com.google.code.findbugs</groupId>
-      <artifactId>annotations</artifactId>
-      <version>3.0.0</version>
-      <scope>provided</scope>
+      <groupId>org.junit.vintage</groupId>
+      <artifactId>junit-vintage-engine</artifactId>
+      <scope>test</scope>
     </dependency>
   </dependencies>
 
   <distributionManagement>
     <site>
-      <id>github-pages</id>
-      <url>gitsite:git@github.com/kohsuke/${project.artifactId}.git</url>
+      <id>github.com</id>
+      <url>gitsite:git@github.com/${gitHubRepo}.git</url>
     </site>
   </distributionManagement>
 
   <licenses>
     <license>
-      <name>The MIT license</name>
-      <url>http://www.opensource.org/licenses/mit-license.php</url>
+      <name>MIT License</name>
+      <url>https://opensource.org/licenses/MIT</url>
       <distribution>repo</distribution>
     </license>
   </licenses>
+
+  <repositories>
+    <repository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </repository>
+  </repositories>
+
+  <pluginRepositories>
+    <pluginRepository>
+      <id>repo.jenkins-ci.org</id>
+      <url>https://repo.jenkins-ci.org/public/</url>
+    </pluginRepository>
+  </pluginRepositories>
 </project>
diff --git a/src/main/java/org/jvnet/winp/Native.java b/src/main/java/org/jvnet/winp/Native.java
@@ -2,7 +2,7 @@
 
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 
-import javax.annotation.CheckReturnValue;
+import edu.umd.cs.findbugs.annotations.CheckReturnValue;
 import java.math.BigInteger;
 import java.net.URL;
 import java.net.URLDecoder;
@@ -104,18 +104,16 @@ public static boolean sendCtrlC(int pid) throws WinpException {
         }
     }
 
+    @SuppressFBWarnings(value = "WEAK_MESSAGE_DIGEST_MD5", justification = "TODO needs triage")
     private static String md5(URL res) {
         try {
             MessageDigest md5 = MessageDigest.getInstance("MD5");
-            InputStream in = res.openStream();
-            try {
+            try (InputStream in = res.openStream()) {
                 byte[] buf = new byte[8192];
                 int len;
                 while((len=in.read(buf))>=0)
                     md5.update(buf, 0, len);
                 return toHex32(md5.digest());
-            } finally {
-                in.close();
             }
         } catch (NoSuchAlgorithmException e) {
             throw new AssertionError(e);

diff --git a/src/main/java/org/jvnet/winp/UserErrorType.java b/src/main/java/org/jvnet/winp/UserErrorType.java
@@ -23,8 +23,6 @@
  */
 package org.jvnet.winp;
 
-import javax.annotation.Nonnegative;
-
 /**
  * User-scope error codes in WinP. 
  * @author Oleg Nenashev
@@ -35,7 +33,7 @@ enum UserErrorType {
 
     private final int shortCode;
 
-    UserErrorType(@Nonnegative int shortCode) {
+    UserErrorType(/* @java.annotation.Nonnegative */ int shortCode) {
         this.shortCode = shortCode;
     }
 

diff --git a/src/main/java/org/jvnet/winp/WinProcess.java b/src/main/java/org/jvnet/winp/WinProcess.java
@@ -1,7 +1,6 @@
 package org.jvnet.winp;
 
-import javax.annotation.CheckReturnValue;
-import java.lang.reflect.Field;
+import edu.umd.cs.findbugs.annotations.CheckReturnValue;
 import java.util.Comparator;
 import java.util.TreeMap;
 import java.util.Iterator;
@@ -39,15 +38,10 @@ public WinProcess(int pid) {
      * Wraps {@link Process} into {@link WinProcess}.
      */
     public WinProcess(Process proc) {
-        try {
-            Field f = proc.getClass().getDeclaredField("handle");
-            f.setAccessible(true);
-            long handle = f.getLong(proc);
-            pid = Native.getProcessId(handle);
-        } catch (NoSuchFieldException e) {
-            throw new NotWindowsException(e);
-        } catch (IllegalAccessException e) {
-            throw new NotWindowsException(e);
+        long pidLong = proc.pid();
+        this.pid = (int) pidLong;
+        if (this.pid != pidLong) {
+            throw new IllegalArgumentException("Out of range: " + pidLong);
         }
     }
 

diff --git a/src/site/site.xml b/src/site/site.xml
@@ -16,7 +16,7 @@
     <menu name="Windows Process Library">
       <item name="Introduction"                   href="/index.html"/>
       <item name="Download"               href="http://mvnrepository.com/artifact/${project.groupId}/${project.artifactId}"/>
-      <item name="Source code" href="https://github.com/kohsuke/${project.artifactId}"/>
+      <item name="Source code" href="https://github.com/jenkinsci/${project.artifactId}"/>
     </menu>
 
     <menu name="References">

diff --git a/src/test/java/org/jvnet/winp/Junit4TestsRanTest.java b/src/test/java/org/jvnet/winp/Junit4TestsRanTest.java
@@ -0,0 +1,14 @@
+package org.jvnet.winp;
+
+import org.junit.Test;
+
+public class Junit4TestsRanTest {
+
+    @Test
+    public void anything() {
+        /*
+         * Intentionally blank. We just want a test that runs with JUnit so that buildPlugin() works
+         * in the Jenkinsfile.
+         */
+    }
+}
diff --git a/src/test/java/org/jvnet/winp/PlatformSpecificProcessTest.java b/src/test/java/org/jvnet/winp/PlatformSpecificProcessTest.java
@@ -39,7 +39,6 @@
 import org.junit.runners.Parameterized.Parameters;
 import org.jvnet.winp.util.ExecutablePlatform;
 import org.jvnet.winp.util.ProcessSpawningTest;
-import static org.jvnet.winp.util.ProcessSpawningTest.isAlive;
 import org.jvnet.winp.util.TestHelper;
 
 /**
@@ -94,7 +93,7 @@ public void getCommandLine_shouldNotFailIfTheProcessIsDead() throws Exception {
         int pid = wp.getPid();
         wp.killRecursively();
         Thread.sleep(1000);
-        assertFalse("The process has not been stopped yet", isAlive(p));
+        assertFalse("The process has not been stopped yet", p.isAlive());
 
         try {
             new WinProcess(p).getCommandLine();
@@ -114,7 +113,7 @@ public void getEnvironmentVariables_shouldFailIfTheProcessIsDead() throws Except
         int pid = wp.getPid();
         wp.killRecursively();
         Thread.sleep(1000);
-        assertFalse("The process has not been stopped yet", isAlive(p));
+        assertFalse("The process has not been stopped yet", p.isAlive());
 
         try {
             new WinProcess(p).getEnvironmentVariables();

diff --git a/src/test/java/org/jvnet/winp/util/ProcessSpawningTest.java b/src/test/java/org/jvnet/winp/util/ProcessSpawningTest.java
@@ -25,8 +25,7 @@
 
 import java.io.File;
 import java.io.IOException;
-import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
+import edu.umd.cs.findbugs.annotations.CheckForNull;
 import static org.hamcrest.CoreMatchers.containsString;
 import org.junit.After;
 import org.junit.Assert;
@@ -47,10 +46,9 @@ public class ProcessSpawningTest extends NativeWinpTest {
 
     @After
     public void killSpawnedProcess() {
-        if (spawnedProcess != null && isAlive(spawnedProcess)) {
+        if (spawnedProcess != null && spawnedProcess.isAlive()) {
             System.err.println("Killing process " + spawnedProcess.toString());
-            //TODO: destroyForcibly() in Java8
-            spawnedProcess.destroy();
+            spawnedProcess.destroyForcibly();
         }
 
         spawnedProcess = null;
@@ -85,16 +83,6 @@ public Process spawnProcess(boolean delayAfterCreate, boolean spotcheckProcess,
         return spawnedProcess;
     }
 
-    //TODO: replace by Process#isAlive() in Java8
-    public static boolean isAlive(@Nonnull Process proc) {
-        try {
-            int exitCode = proc.exitValue();
-            return false;
-        } catch (IllegalThreadStateException ex) {
-           return true;
-        }
-    }
-
     protected static File getTestAppExecutable(ExecutablePlatform executablePlatform) {
         final String executable;
         switch (executablePlatform) {