You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@beam.apache.org by da...@apache.org on 2022/10/18 15:43:03 UTC

[beam] branch revert-23165-jamm created (now 70031f7e8e1)

This is an automated email from the ASF dual-hosted git repository.

damccorm pushed a change to branch revert-23165-jamm
in repository https://gitbox.apache.org/repos/asf/beam.git


      at 70031f7e8e1 Revert "Automatically open module/packages for Java 11+"

This branch includes the following new commits:

     new 70031f7e8e1 Revert "Automatically open module/packages for Java 11+"

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[beam] 01/01: Revert "Automatically open module/packages for Java 11+"

Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

damccorm pushed a commit to branch revert-23165-jamm
in repository https://gitbox.apache.org/repos/asf/beam.git

commit 70031f7e8e1735b73f90c090a31f6ae1f9485e8a
Author: Danny McCormick <da...@google.com>
AuthorDate: Tue Oct 18 11:42:55 2022 -0400

    Revert "Automatically open module/packages for Java 11+"
---
 .../org/apache/beam/gradle/BeamModulePlugin.groovy | 44 +++++++--------
 .../src/main/scripts/build_release_candidate.sh    | 13 +----
 sdks/go/test/run_validatesrunner_tests.sh          |  7 +--
 sdks/java/container/Dockerfile                     |  4 +-
 sdks/java/container/agent/build.gradle             | 64 ----------------------
 .../org/apache/beam/agent/OpenModuleAgent.java     | 64 ----------------------
 sdks/java/container/boot.go                        |  5 --
 sdks/java/container/common.gradle                  | 14 -----
 sdks/java/container/java11/build.gradle            |  4 --
 sdks/java/container/java11/option-jamm.json        |  5 +-
 sdks/java/container/java17/build.gradle            |  6 +-
 sdks/java/container/java17/option-jamm.json        |  5 +-
 settings.gradle.kts                                |  1 -
 .../site/content/en/contribute/release-guide.md    |  3 +-
 14 files changed, 32 insertions(+), 207 deletions(-)

diff --git a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
index d20222b8197..8541cf75ae6 100644
--- a/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
+++ b/buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
@@ -39,7 +39,6 @@ import org.gradle.api.tasks.Exec
 import org.gradle.api.tasks.JavaExec
 import org.gradle.api.tasks.TaskProvider
 import org.gradle.api.tasks.bundling.Jar
-import org.gradle.api.tasks.compile.CompileOptions
 import org.gradle.api.tasks.compile.JavaCompile
 import org.gradle.api.tasks.javadoc.Javadoc
 import org.gradle.api.tasks.testing.Test
@@ -756,29 +755,6 @@ class BeamModulePlugin implements Plugin<Project> {
           + suffix)
     }
 
-    project.ext.setJava17Options = { CompileOptions options ->
-      def java17Home = project.findProperty("java17Home")
-      options.fork = true
-      options.forkOptions.javaHome = java17Home as File
-      options.compilerArgs += ['-Xlint:-path']
-      // Error prone requires some packages to be exported/opened for Java 17
-      // Disabling checks since this property is only used for Jenkins tests
-      // https://github.com/tbroyer/gradle-errorprone-plugin#jdk-16-support
-      options.errorprone.errorproneArgs.add("-XepDisableAllChecks")
-      options.forkOptions.jvmArgs += [
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
-        "-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
-        "-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
-        "-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED"
-      ]
-    }
-
     project.ext.repositories = {
       maven {
         name "testPublicationLocal"
@@ -1264,8 +1240,26 @@ class BeamModulePlugin implements Plugin<Project> {
       if (project.hasProperty("compileAndRunTestsWithJava17")) {
         def java17Home = project.findProperty("java17Home")
         project.tasks.compileTestJava {
+          options.fork = true
+          options.forkOptions.javaHome = java17Home as File
+          options.compilerArgs += ['-Xlint:-path']
           options.compilerArgs.addAll(['--release', '17'])
-          project.ext.setJava17Options(options)
+          // Error prone requires some packages to be exported/opened for Java 17
+          // Disabling checks since this property is only used for Jenkins tests
+          // https://github.com/tbroyer/gradle-errorprone-plugin#jdk-16-support
+          options.errorprone.errorproneArgs.add("-XepDisableAllChecks")
+          options.forkOptions.jvmArgs += [
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
+            "-J--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
+            "-J--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
+            "-J--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED"
+          ]
         }
         project.tasks.withType(Test) {
           useJUnit()
diff --git a/release/src/main/scripts/build_release_candidate.sh b/release/src/main/scripts/build_release_candidate.sh
index 0c6da71c76c..8fe434edb97 100755
--- a/release/src/main/scripts/build_release_candidate.sh
+++ b/release/src/main/scripts/build_release_candidate.sh
@@ -77,7 +77,6 @@ RC_NUM=
 SIGNING_KEY=
 USER_GITHUB_ID=
 DEBUG=
-JAVA11_HOME=
 
 while [[ $# -gt 0 ]] ; do
   arg="$1"
@@ -104,10 +103,6 @@ while [[ $# -gt 0 ]] ; do
       shift; USER_GITHUB_ID=$1; shift
       ;;
 
-      --java11-home)
-      shift; JAVA11_HOME=$1; shift
-      ;;
-
       *)
       echo "Unrecognized argument: $1"
       usage
@@ -140,12 +135,6 @@ if [[ -z "$USER_GITHUB_ID" ]] ; then
   exit 1
 fi
 
-if [[ -z "$JAVA11_HOME" ]] ; then
-  echo 'Please provide Java 11 home. Required to build sdks/java/container/agent for Java 11+ containers.'
-  usage
-  exit 1
-fi
-
 if [[ -z "$SIGNING_KEY" ]] ; then
   echo "=================Pre-requirements===================="
   echo "Please make sure you have configured and started your gpg-agent by running ./preparation_before_release.sh."
@@ -344,7 +333,7 @@ if [[ $confirmation = "y" ]]; then
   cd ${BEAM_ROOT_DIR}
   git checkout ${RC_TAG}
 
-  ./gradlew :pushAllDockerImages -Pdocker-pull-licenses -Pdocker-tag=${RELEASE}rc${RC_NUM} -Pjava11Home=${JAVA11_HOME}
+  ./gradlew :pushAllDockerImages -Pdocker-pull-licenses -Pdocker-tag=${RELEASE}rc${RC_NUM}
 
   wipe_local_clone_dir
 fi
diff --git a/sdks/go/test/run_validatesrunner_tests.sh b/sdks/go/test/run_validatesrunner_tests.sh
index 0d0531c8234..d9d94b3a73a 100755
--- a/sdks/go/test/run_validatesrunner_tests.sh
+++ b/sdks/go/test/run_validatesrunner_tests.sh
@@ -237,11 +237,6 @@ case $key in
         shift # past argument
         shift # past value
         ;;
-    --java11_home)
-        JAVA11_HOME="$2"
-        shift # past argument
-        shift # past value
-        ;;
     *)    # unknown option
         echo "Unknown option: $1"
         exit 1
@@ -387,7 +382,7 @@ if [[ "$RUNNER" == "dataflow" ]]; then
       JAVA_TAG=$(date +%Y%m%d-%H%M%S)
       JAVA_CONTAINER=us.gcr.io/$PROJECT/$USER/beam_java11_sdk
       echo "Using container $JAVA_CONTAINER for cross-language java transforms"
-      ./gradlew :sdks:java:container:java11:docker -Pdocker-repository-root=us.gcr.io/$PROJECT/$USER -Pdocker-tag=$JAVA_TAG -Pjava11Home=$JAVA11_HOME
+      ./gradlew :sdks:java:container:java11:docker -Pdocker-repository-root=us.gcr.io/$PROJECT/$USER -Pdocker-tag=$JAVA_TAG
 
       # Verify it exists
       docker images | grep $JAVA_TAG
diff --git a/sdks/java/container/Dockerfile b/sdks/java/container/Dockerfile
index c29b7f7910b..179a7e22979 100644
--- a/sdks/java/container/Dockerfile
+++ b/sdks/java/container/Dockerfile
@@ -31,9 +31,7 @@ ADD target/beam-sdks-java-io-kafka.jar /opt/apache/beam/jars/
 ADD target/kafka-clients.jar /opt/apache/beam/jars/
 
 # Required to use jamm as a javaagent to get accurate object size measuring
-# COPY fails if file is not found, so use a wildcard for open-module-agent.jar
-# since it is only included in Java 9+ containers
-COPY target/jamm.jar target/open-module-agent*.jar /opt/apache/beam/jars/
+ADD target/jamm.jar /opt/apache/beam/jars/
 
 ADD target/linux_amd64/boot /opt/apache/beam/
 
diff --git a/sdks/java/container/agent/build.gradle b/sdks/java/container/agent/build.gradle
deleted file mode 100644
index 9d86fd430a6..00000000000
--- a/sdks/java/container/agent/build.gradle
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * 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.
- */
-
-plugins {
-    id 'org.apache.beam.module'
-}
-applyJavaNature(
-    exportJavadoc: false,
-    publish: false
-)
-
-description = "Apache Beam :: SDKs :: Java :: Container :: Agent"
-
-jar {
-    manifest {
-        attributes("Agent-Class": "org.apache.beam.agent.OpenModuleAgent",
-                "Can-Redefine-Classes": true,
-                "Can-Retransform-Classes": true,
-                "Premain-Class": "org.apache.beam.agent.OpenModuleAgent")
-    }
-}
-
-
-if (project.hasProperty('java11Home')) {
-    javaVersion = "1.11"
-    def java11Home = project.findProperty('java11Home')
-    project.tasks.withType(JavaCompile) {
-        options.fork = true
-        options.forkOptions.javaHome = java11Home as File
-        options.compilerArgs += ['-Xlint:-path']
-    }
-} else if (project.hasProperty('java17Home')) {
-    javaVersion = "1.17"
-    project.tasks.withType(JavaCompile) {
-        setJava17Options(options)
-
-        checkerFramework {
-            skipCheckerFramework = true
-        }
-    }
-}
-
-// Module classes requires JDK > 8
-project.tasks.each {
-    it.onlyIf {
-        project.hasProperty('java11Home') || project.hasProperty('java17Home')
-                || JavaVersion.VERSION_1_8.compareTo(JavaVersion.current()) < 0
-    }
-}
diff --git a/sdks/java/container/agent/src/main/java/org/apache/beam/agent/OpenModuleAgent.java b/sdks/java/container/agent/src/main/java/org/apache/beam/agent/OpenModuleAgent.java
deleted file mode 100644
index 0440475e361..00000000000
--- a/sdks/java/container/agent/src/main/java/org/apache/beam/agent/OpenModuleAgent.java
+++ /dev/null
@@ -1,64 +0,0 @@
-/*
- * 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.beam.agent;
-
-import java.lang.instrument.Instrumentation;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
-
-/*
- * Opens all base modules in the JDK to jamm (used to accurately measure object sizes)
- */
-public class OpenModuleAgent {
-  public static void premain(String argument, Instrumentation instrumentation) {
-    Set<Module> modulesToOpen = new HashSet<>();
-    Optional<Module> jamm = ModuleLayer.boot().findModule("jamm");
-    if (!jamm.isPresent()) {
-      System.err.println("Jamm module expected, but not found");
-      return;
-    }
-
-    ModuleLayer.boot()
-        .modules()
-        .forEach(
-            module -> {
-              if (!module.getDescriptor().isOpen()) {
-                modulesToOpen.add(module);
-              }
-            });
-
-    Set<Module> openModules = Set.of(jamm.get());
-    for (Module module : modulesToOpen) {
-      Map<String, Set<Module>> addOpens = new HashMap<>();
-      for (String pkg : module.getPackages()) {
-        addOpens.put(pkg, openModules);
-      }
-      instrumentation.redefineModule(
-          module,
-          Collections.emptySet(),
-          Collections.emptyMap(),
-          addOpens,
-          Collections.emptySet(),
-          Collections.emptyMap());
-    }
-  }
-}
diff --git a/sdks/java/container/boot.go b/sdks/java/container/boot.go
index 5fa85c77dd5..d0411331874 100644
--- a/sdks/java/container/boot.go
+++ b/sdks/java/container/boot.go
@@ -224,11 +224,6 @@ func main() {
 			}
 		}
 	}
-	// Automatically open modules for Java 11+
-	openModuleAgentJar := "/opt/apache/beam/jars/open-module-agent.jar"
-	if _, err := os.Stat(openModuleAgentJar); err == nil {
-		args = append(args, "-javaagent:" + openModuleAgentJar)
-	}
 	args = append(args, "org.apache.beam.fn.harness.FnHarness")
 	log.Printf("Executing: java %v", strings.Join(args, " "))
 
diff --git a/sdks/java/container/common.gradle b/sdks/java/container/common.gradle
index 265d14fbe9c..b9f8da73cbd 100644
--- a/sdks/java/container/common.gradle
+++ b/sdks/java/container/common.gradle
@@ -48,9 +48,6 @@ task copyDockerfileDependencies(type: Copy) {
     from configurations.dockerDependency
     rename 'slf4j-api.*', 'slf4j-api.jar'
     rename 'slf4j-jdk14.*', 'slf4j-jdk14.jar'
-    if (imageJavaVersion == "11" || imageJavaVersion == "17") {
-        rename 'beam-sdks-java-container-agent.*.jar', 'open-module-agent.jar'
-    }
     rename 'beam-sdks-java-harness-.*.jar', 'beam-sdks-java-harness.jar'
     rename 'beam-sdks-java-io-kafka.*.jar', 'beam-sdks-java-io-kafka.jar'
     rename 'kafka-clients.*.jar', 'kafka-clients.jar'
@@ -89,16 +86,6 @@ task skipPullLicenses(type: Exec) {
     args "-c", "mkdir -p build/target/go-licenses build/target/options build/target/third_party_licenses && touch build/target/third_party_licenses/skip"
 }
 
-task validateJavaHome {
-    if (imageJavaVersion == "11" || imageJavaVersion == "17") {
-        doFirst {
-            if (!project.hasProperty('java17Home') && !project.hasProperty('java11Home')) {
-                throw new GradleException('java17Home or java11Home property required. Re-run with -Pjava17Home or -Pjava11Home')
-            }
-        }
-    }
-}
-
 docker {
     name containerImageName(
             name: "${project.docker_image_default_repo_prefix}java${imageJavaVersion}_sdk",
@@ -129,4 +116,3 @@ dockerPrepare.dependsOn copySdkHarnessLauncher
 dockerPrepare.dependsOn copyDockerfileDependencies
 dockerPrepare.dependsOn ":sdks:java:container:downloadCloudProfilerAgent"
 dockerPrepare.dependsOn copyJdkOptions
-dockerPrepare.dependsOn validateJavaHome
\ No newline at end of file
diff --git a/sdks/java/container/java11/build.gradle b/sdks/java/container/java11/build.gradle
index bbf7bb1f971..b9f70facdf1 100644
--- a/sdks/java/container/java11/build.gradle
+++ b/sdks/java/container/java11/build.gradle
@@ -22,7 +22,3 @@ project.ext {
 
 // Load the main build script which contains all build logic.
 apply from: "../common.gradle"
-
-dependencies {
-    dockerDependency project(path: ":sdks:java:container:agent")
-}
\ No newline at end of file
diff --git a/sdks/java/container/java11/option-jamm.json b/sdks/java/container/java11/option-jamm.json
index ac9a8e60063..e994648a8e2 100644
--- a/sdks/java/container/java11/option-jamm.json
+++ b/sdks/java/container/java11/option-jamm.json
@@ -4,7 +4,10 @@
   "options": {
     "java_arguments": [
       "--add-modules=jamm",
-      "--module-path=/opt/apache/beam/jars/jamm.jar"
+      "--module-path=/opt/apache/beam/jars/jamm.jar",
+      "--add-opens=java.base/java.lang=jamm",
+      "--add-opens=java.base/java.lang.ref=jamm",
+      "--add-opens=java.base/java.util=jamm"
     ]
   }
 }
\ No newline at end of file
diff --git a/sdks/java/container/java17/build.gradle b/sdks/java/container/java17/build.gradle
index 24e7af6c2c5..54e2a283c59 100644
--- a/sdks/java/container/java17/build.gradle
+++ b/sdks/java/container/java17/build.gradle
@@ -21,8 +21,4 @@ project.ext {
 }
 
 // Load the main build script which contains all build logic.
-apply from: "../common.gradle"
-
-dependencies {
-    dockerDependency project(path: ":sdks:java:container:agent")
-}
\ No newline at end of file
+apply from: "../common.gradle"
\ No newline at end of file
diff --git a/sdks/java/container/java17/option-jamm.json b/sdks/java/container/java17/option-jamm.json
index ac9a8e60063..e994648a8e2 100644
--- a/sdks/java/container/java17/option-jamm.json
+++ b/sdks/java/container/java17/option-jamm.json
@@ -4,7 +4,10 @@
   "options": {
     "java_arguments": [
       "--add-modules=jamm",
-      "--module-path=/opt/apache/beam/jars/jamm.jar"
+      "--module-path=/opt/apache/beam/jars/jamm.jar",
+      "--add-opens=java.base/java.lang=jamm",
+      "--add-opens=java.base/java.lang.ref=jamm",
+      "--add-opens=java.base/java.util=jamm"
     ]
   }
 }
\ No newline at end of file
diff --git a/settings.gradle.kts b/settings.gradle.kts
index 52f5a9431cc..d17d4caead0 100644
--- a/settings.gradle.kts
+++ b/settings.gradle.kts
@@ -116,7 +116,6 @@ include(":sdks:java:bom")
 include(":sdks:java:bom:gcp")
 include(":sdks:java:build-tools")
 include(":sdks:java:container")
-include(":sdks:java:container:agent")
 include(":sdks:java:container:java8")
 include(":sdks:java:container:java11")
 include(":sdks:java:container:java17")
diff --git a/website/www/site/content/en/contribute/release-guide.md b/website/www/site/content/en/contribute/release-guide.md
index c5ea4442145..72b461d9c5d 100644
--- a/website/www/site/content/en/contribute/release-guide.md
+++ b/website/www/site/content/en/contribute/release-guide.md
@@ -500,7 +500,6 @@ Consider adding known issues there for minor issues instead of accepting cherry
 * Originating branch has the version information updated to the new version;
 * Nightly snapshot is in progress (do revisit it continually);
 * Set `JAVA_HOME` to JDK 8 (Example: `export JAVA_HOME=/example/path/to/java/jdk8`).
-* Have Java 11 installed.
 
 The core of the release process is the build-vote-fix cycle.
 Each cycle produces one release candidate.
@@ -544,7 +543,7 @@ See the source of the script for more details, or to run commands manually in ca
 
 * **Usage**
 
-      ./beam/release/src/main/scripts/build_release_candidate.sh --release "${RELEASE_VERSION}" --rc "${RC_NUM}" --github-user "${GITHUB_USER}" --java11-home "${JAVA11_HOME}"
+      ./beam/release/src/main/scripts/build_release_candidate.sh --release "${RELEASE_VERSION}" --rc "${RC_NUM}" --github-user "${GITHUB_USER}"
 
 * **The script will:**
   1. Clone the repo at the selected RC tag.