[
https://issues.apache.org/jira/browse/HADOOP-19970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18133576#comment-18133576
]
ASF GitHub Bot commented on HADOOP-19970:
-----------------------------------------
pjfanning commented on code in PR #8699:
URL: https://github.com/apache/hadoop/pull/8699#discussion_r4401017514
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/http/TestJettyClasspath.java:
##########
@@ -0,0 +1,40 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.http;
+
+import org.junit.jupiter.api.Test;
+
+import static
org.apache.hadoop.test.ClasspathTestUtils.assertSingleJettyRelease;
+import static org.apache.hadoop.test.ClasspathTestUtils.assertSingleServletApi;
+
+/**
+ * HADOOP-19970: this module's classpath resolves one servlet API and one
+ * Jetty release.
+ */
+public class TestJettyClasspath {
Review Comment:
There are six near-identical copies of this class (hadoop-common, shuffle,
yarn-common, AHS, nodemanager, router), and the nodemanager one has already
gained an extra test. Since `ClasspathTestUtils` already lives in the
hadoop-common test-jar, an `AbstractJettyClasspathTest` there with an empty
subclass per module would keep them in sync. JUnit 5 runs inherited `@Test`
methods.
##########
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/ClasspathTestUtils.java:
##########
@@ -0,0 +1,98 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.test;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.Collections;
+import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Assertions on the test classpath of a module, so a module can check that
+ * it resolves one servlet API and one Jetty release (HADOOP-19970).
+ */
+public final class ClasspathTestUtils {
+
+ /** The servlet API every module resolves. */
+ public static final String SERVLET_API_JAR = "/jetty-servlet-api-";
+
+ /**
+ * Jetty jars by their Maven repository path,
+ * .../org/eclipse/jetty/[websocket/]artifact/version/. Toolchain
+ * artifacts such as jetty-servlet-api have their own versions and
+ * do not match.
+ */
+ private static final Pattern JETTY_JAR = Pattern.compile(
Review Comment:
This only allows an optional `websocket/` segment, so jars from the other
Jetty groups never match. For example,
`.../org/eclipse/jetty/http2/http2-server/9.4.44.v20210927/http2-server-9.4.44.v20210927.jar!`
gives `group(1)=http2`, `group(2)=http2-server`, and then `/[^/]+\.jar!` can't
match the rest. Stale `http2-*`, `alpn`, or `fcgi` jars next to
`${jetty.version}` would therefore pass `assertSingleJettyRelease`, which is
the drift this is meant to catch (the old Solr stack carried some of these).
Maybe `(?:(?:websocket|http2|alpn|fcgi)/)?`, or anchor on the version directory
instead?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-applications-catalog/hadoop-yarn-applications-catalog-webapp/pom.xml:
##########
@@ -242,6 +289,53 @@
<groupId>org.eclipse.jetty</groupId>
<artifactId>jetty-alpn-client</artifactId>
</exclusion>
+ <!--
Review Comment:
This block of Solr Jetty and servlet exclusions, including the comment, is
repeated word for word on the other solr dependency (around line 192). The next
Solr bump that adds a Jetty artifact has to be fixed in both places, and
missing one brings back two Jetty releases. Could the exclusions live once on
the solr artifacts in `hadoop-project` `dependencyManagement`?
##########
hadoop-client-modules/hadoop-client-runtime/pom.xml:
##########
@@ -166,11 +166,22 @@
<!
> HADOOP-Move the servlet API to jetty-servlet-api 4.0.9
> ------------------------------------------------------
>
> Key: HADOOP-19970
> URL: https://issues.apache.org/jira/browse/HADOOP-19970
> Project: Hadoop Common
> Issue Type: Sub-task
> Components: build, hadoop-common
> Reporter: Jose Luis López
> Assignee: Jose Luis López
> Priority: Minor
> Labels: pull-request-available
>
> *Scope*:
> * hadoop-project/pom.xml: add the jetty-servlet-api.version property (4.0.9),
> and replace the managed jakarta.servlet:jakarta.servlet-api 4.0.4 with
> org.eclipse.jetty.toolchain:jetty-servlet-api.
> * hadoop-common/pom.xml: declare
> org.eclipse.jetty.toolchain:jetty-servlet-api at compile scope. HttpServer2
> and its filters compile against javax.servlet, which today arrives only as
> javax.servlet-api 3.1.0 through jetty-server. Ten modules that use
> javax.servlet without declaring an API get it through hadoop-common.
> * hadoop-project/pom.xml and hadoop-tools/hadoop-sls/pom.xml: the exclusion
> on jetty-server names org.eclipse.jetty:javax.servlet-api, a coordinate that
> matches nothing. Change it to javax.servlet:javax.servlet-api.
> * hadoop-project/pom.xml: on javax-websocket-server-impl, also exclude
> javax.servlet:javax.servlet-api (through websocket-servlet).
> * Switch the coordinate in the 17 module poms that declare the servlet API.
> * Rename the 5 jakarta.servlet-api exclusions: 4 in hadoop-client-minicluster
> and 1 in hadoop-yarn-common.
> * jersey-test-framework-core declares jakarta.servlet-api at compile scope.
> Exclude it there, and change the exclusion on the jdk-http provider to the
> same coordinate.
> * LICENSE-binary: replace jakarta.servlet:jakarta.servlet-api:4.0.4 with
> org.eclipse.jetty.toolchain:jetty-servlet-api:4.0.9.
> * LICENSE-binary and NOTICE-binary: drop javax.servlet:javax.servlet-api
> 3.1.0.
> *Why*: javax.servlet-api 3.1.0 and jakarta.servlet-api 4.0.4 both publish the
> javax.servlet packages, and which one a module compiles and runs against
> depends on jar order. jetty-servlet-api is the servlet API that Jetty 12's
> ee8 environment depends on. Switching now leaves one servlet API jar and
> takes the coordinate change out of HADOOP-19972.
> *Impact on Jetty 9*: none. 4.0 is a superset of 3.1, and Jetty 9.4 runs on
> Servlet 4.0. jetty-servlet-api and jakarta.servlet-api 4.0.4 ship the same
> javax.servlet 4.0 classes, and jetty-servlet-api also carries the web.xml
> schemas. The project's web.xml files declare versions 2.4, 2.5 and 3.0, which
> Servlet 4.0 accepts.
> *Downstream*:
> * hadoop-client-runtime: still relocates one servlet API under
> org.apache.hadoop.shaded.javax.servlet. Its source changes from
> jakarta.servlet-api 4.0.4 to jetty-servlet-api 4.0.9: same javax.servlet 4.0
> classes, plus the web.xml schemas. No public API change.
> * hadoop-client-api, hadoop-client-minicluster: unchanged; both already
> exclude the servlet API.
> * Non-shaded artifacts: the coordinate changes from
> jakarta.servlet:jakarta.servlet-api to
> org.eclipse.jetty.toolchain:jetty-servlet-api. Downstream exclusions of
> jakarta.servlet-api need the new coordinate.
> * Direct consumers of hadoop-common: the servlet API changes from
> javax.servlet:javax.servlet-api 3.1.0 to jetty-servlet-api 4.0.9. Same
> javax.servlet packages; 4.0 is a superset of 3.1.
> *Depends on*: HADOOP-20014 (exclusion on the jdk-http provider).
> *CI*: touches hadoop-project, so a full build.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
