From c6b372f517dc691910552f18db1ada3c3a4127f2 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Fri, 1 Mar 2019 19:48:29 +0100
Subject: [PATCH] Use maven plugin to prevent API/ABI breakage as part of build
 process (#8904)

Motivation:

Netty is very widely used which can lead to a lot of pain when we break API / ABI. We should make use japicmp-maven-plugin during the build to verify we do not introduce breakage by mistake.

Modifications:

- Add japicmp-maven-plugin to the build process
- Fix a method signature change in HttpProxyHandler that was flagged as a possible problem.

Result:

Ensure no API/ABI breakage accour between releases.
---
 all/pom.xml                                   |  1 +
 codec-http/pom.xml                            |  1 -
 example/pom.xml                               |  3 ++
 .../netty/handler/proxy/HttpProxyHandler.java |  2 +-
 microbench/pom.xml                            |  2 +-
 pom.xml                                       | 31 +++++++++++++++++++
 testsuite-autobahn/pom.xml                    |  4 +++
 testsuite-http2/pom.xml                       |  4 +++
 testsuite-osgi/pom.xml                        |  1 +
 testsuite-shading/pom.xml                     |  1 +
 testsuite/pom.xml                             |  1 +
 11 files changed, 48 insertions(+), 3 deletions(-)

diff --git a/all/pom.xml b/all/pom.xml
index 9031db1799..3844c5859d 100644
--- a/all/pom.xml
+++ b/all/pom.xml
@@ -31,6 +31,7 @@
   <properties>
     <generatedSourceDir>${project.build.directory}/src</generatedSourceDir>
     <dependencyVersionsDir>${project.build.directory}/versions</dependencyVersionsDir>
+    <skipJapicmp>true</skipJapicmp>
   </properties>
 
   <profiles>
diff --git a/codec-http/pom.xml b/codec-http/pom.xml
index 36606e269f..3c7048e51b 100644
--- a/codec-http/pom.xml
+++ b/codec-http/pom.xml
@@ -57,7 +57,6 @@
       <groupId>${project.groupId}</groupId>
       <artifactId>netty-handler</artifactId>
       <version>${project.version}</version>
-      <optional>true</optional>
     </dependency>
     <dependency>
       <groupId>com.jcraft</groupId>
diff --git a/example/pom.xml b/example/pom.xml
index d3c4757625..d0eb6665d2 100644
--- a/example/pom.xml
+++ b/example/pom.xml
@@ -29,6 +29,9 @@
 
   <name>Netty/Example</name>
 
+  <properties>
+    <skipJapicmp>true</skipJapicmp>
+  </properties>
   <dependencies>
     <dependency>
       <groupId>${project.groupId}</groupId>
diff --git a/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java b/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
index 278eb10316..69792958a2 100644
--- a/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
+++ b/handler-proxy/src/main/java/io/netty/handler/proxy/HttpProxyHandler.java
@@ -171,7 +171,7 @@ public final class HttpProxyHandler extends ProxyHandler {
     }
 
     @Override
-    protected boolean handleResponse(ChannelHandlerContext ctx, Object response) throws HttpProxyConnectException {
+    protected boolean handleResponse(ChannelHandlerContext ctx, Object response) throws Exception {
         if (response instanceof HttpResponse) {
             if (status != null) {
                 throw new HttpProxyConnectException(exceptionMessage("too many responses"), /*headers=*/ null);
diff --git a/microbench/pom.xml b/microbench/pom.xml
index 8d2d62c46f..000ee460ab 100644
--- a/microbench/pom.xml
+++ b/microbench/pom.xml
@@ -38,8 +38,8 @@
     <!-- This only be set when run on mac as on other platforms we just want to include the jar without native
          code -->
     <kqueue.classifier />
+    <skipJapicmp>true</skipJapicmp>
   </properties>
-
   <profiles>
     <profile>
       <id>linux</id>
diff --git a/pom.xml b/pom.xml
index b7c4a4e6d4..7bb20c4a3f 100644
--- a/pom.xml
+++ b/pom.xml
@@ -283,6 +283,8 @@
     <skipOsgiTestsuite>false</skipOsgiTestsuite>
     <skipAutobahnTestsuite>false</skipAutobahnTestsuite>
     <skipHttp2Testsuite>false</skipHttp2Testsuite>
+    <!-- Skip for now as we have no version released yet of netty 5 -->
+    <skipJapicmp>true</skipJapicmp>
   </properties>
 
   <modules>
@@ -650,6 +652,35 @@
     </extensions>
 
     <plugins>
+      <plugin>
+        <groupId>com.github.siom79.japicmp</groupId>
+        <artifactId>japicmp-maven-plugin</artifactId>
+        <version>0.13.1</version>
+        <configuration>
+          <parameter>
+            <breakBuildOnBinaryIncompatibleModifications>true</breakBuildOnBinaryIncompatibleModifications>
+            <breakBuildOnSourceIncompatibleModifications>true</breakBuildOnSourceIncompatibleModifications>
+            <oldVersionPattern>\d+\.\d+\.\d+\.Final</oldVersionPattern>
+            <ignoreMissingClassesByRegularExpressions>
+              <!-- ignore everything which is not part of netty itself as the plugin can not handle optional dependencies -->
+              <ignoreMissingClassesByRegularExpression>^(?!io\.netty\.).*</ignoreMissingClassesByRegularExpression>
+              <ignoreMissingClassesByRegularExpression>^io\.netty\.internal\.tcnative\..*</ignoreMissingClassesByRegularExpression>
+            </ignoreMissingClassesByRegularExpressions>
+            <excludes>
+              <exclude>@io.netty.util.internal.UnstableApi</exclude>
+            </excludes>
+          </parameter>
+          <skip>${skipJapicmp}</skip>
+        </configuration>
+        <executions>
+          <execution>
+            <phase>verify</phase>
+            <goals>
+              <goal>cmp</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
       <plugin>
         <artifactId>maven-enforcer-plugin</artifactId>
         <version>${enforcer.plugin.version}</version>
diff --git a/testsuite-autobahn/pom.xml b/testsuite-autobahn/pom.xml
index fa22b6a82c..8813a891b6 100644
--- a/testsuite-autobahn/pom.xml
+++ b/testsuite-autobahn/pom.xml
@@ -28,6 +28,10 @@
 
   <name>Netty/Testsuite/Autobahn</name>
 
+  <properties>
+    <skipJapicmp>true</skipJapicmp>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>${project.groupId}</groupId>
diff --git a/testsuite-http2/pom.xml b/testsuite-http2/pom.xml
index 3e0880937c..87aa65e054 100644
--- a/testsuite-http2/pom.xml
+++ b/testsuite-http2/pom.xml
@@ -28,6 +28,10 @@
 
   <name>Netty/Testsuite/Http2</name>
 
+  <properties>
+    <skipJapicmp>true</skipJapicmp>
+  </properties>
+
   <dependencies>
     <dependency>
       <groupId>${project.groupId}</groupId>
diff --git a/testsuite-osgi/pom.xml b/testsuite-osgi/pom.xml
index 2cc977e541..9c378200dd 100644
--- a/testsuite-osgi/pom.xml
+++ b/testsuite-osgi/pom.xml
@@ -31,6 +31,7 @@
   <properties>
     <exam.version>4.13.0</exam.version>
     <argLine.java9.extras>--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.security=ALL-UNNAMED</argLine.java9.extras>
+    <skipJapicmp>true</skipJapicmp>
   </properties>
 
   <profiles>
diff --git a/testsuite-shading/pom.xml b/testsuite-shading/pom.xml
index 5d69310255..1912e4de1a 100644
--- a/testsuite-shading/pom.xml
+++ b/testsuite-shading/pom.xml
@@ -38,6 +38,7 @@
 
     <jarName>${project.artifactId}-${project.version}.jar</jarName>
     <shadedPackagePrefix>io.netty.</shadedPackagePrefix>
+    <skipJapicmp>true</skipJapicmp>
   </properties>
 
   <build>
diff --git a/testsuite/pom.xml b/testsuite/pom.xml
index 1e38ee1d7d..168aa2acf3 100644
--- a/testsuite/pom.xml
+++ b/testsuite/pom.xml
@@ -99,6 +99,7 @@
   <properties>
     <!-- Needed for SSL tests as these use the SelfSignedCertificate --> 
     <argLine.java9.extras>--add-exports java.base/sun.security.x509=ALL-UNNAMED</argLine.java9.extras>
+    <skipJapicmp>true</skipJapicmp>
   </properties>
 
   <build>