Merge pull request cloudfoundry#1288 from stokpop/issue-1264-base-jre-refactor

refactor: extract BaseJRE to eliminate duplication across standard JRE providers

Author: tnikolova82

adoption-migration-details.md

@@ -142,7 +142,30 @@ There are two main aspects to consider when migrating to the Go-based Java Build
 2. Plan migration before EOL dates
 3. Monitor EOL announcements for your versions
 
-## Additional Resources
+## Behavioral Differences vs Ruby Buildpack
+
+This section documents Go buildpack behaviors that differ from the Ruby buildpack, including new features and known fixes applied during the migration.
+
+### `JAVA_HOME` is now set during staging
+
+The Ruby buildpack never set `JAVA_HOME`. The Go buildpack writes it as an env file (sourced between supply steps) and in `profile.d/java.sh` at runtime. This is a **positive new feature** — apps and tasks that previously hard-coded `.java-buildpack/open_jdk_jre/bin/java` can now use `$JAVA_HOME/bin/java` instead.
+
+See also: [issue #1151](https://github.com/cloudfoundry/java-buildpack/issues/1151).
+
+### Tomcat version auto-selection based on Java version
+
+The Ruby buildpack always used the manifest default (`9.+`) unless the user explicitly set a Tomcat version. The Go buildpack adds a new convenience feature: when `JAVA_HOME` is available and readable it auto-selects:
+
+- **Java 11+** → Tomcat 10.x (Jakarta EE 9+)
+- **Java ≤ 10** → Tomcat 9.x (Java EE 8)
+
+When Java version detection fails (e.g. unreadable `release` file, non-standard JRE layout), the buildpack falls back to `DefaultVersion("tomcat")` from the manifest — matching the Ruby buildpack's behaviour in all cases. An explicitly configured `JBP_CONFIG_TOMCAT` version is always honoured.
+
+### `-XX:ActiveProcessorCount` is HotSpot-only
+
+The Ruby buildpack only added `-XX:ActiveProcessorCount=$(nproc)` in the OpenJDK-like JRE. The Go buildpack matches this: only HotSpot-based JREs (OpenJDK, Oracle, SapMachine, Zulu, GraalVM) receive this flag. IBM JRE continues to use `-Xtune:virtualized -Xshareclasses:none` as in Ruby. Note that IBM OpenJ9 does support `-XX:ActiveProcessorCount` (see [OpenJ9 docs](https://eclipse.dev/openj9/docs/xxactiveprocessorcount/)), but it is deliberately omitted here to stay compatible with the Ruby buildpack behaviour.
+
+
 
 - [OpenRewrite: JavaxMigrationToJakarta Recipe](https://docs.openrewrite.org/recipes/java/migrate/jakarta/javaxmigrationtojakarta)
 - [Apache Tomcat Jakarta EE Migration Tool](https://github.com/apache/tomcat-jakartaee-migration)

src/java/common/context.go

@@ -66,11 +66,7 @@ func DetermineJavaVersion(javaHome string) (int, error) {
 	releaseFile := filepath.Join(javaHome, "release")
 	content, err := os.ReadFile(releaseFile)
 	if err != nil {
-		// Default to Java 17 if release file is missing
-		if os.IsNotExist(err) {
-			return 17, nil
-		}
-		return 0, fmt.Errorf("failed to read release file: %w", err)
+		return 17, fmt.Errorf("failed to read release file: %w", err)
 	}
 
 	// Parse JAVA_VERSION from release file
@@ -103,7 +99,7 @@ func DetermineJavaVersion(javaHome string) (int, error) {
 		}
 	}
 
-	return 0, fmt.Errorf("unable to parse Java version from release file")
+	return 17, fmt.Errorf("unable to parse Java version from release file")
 }
 
 // GetJavaMajorVersion returns the Java major version from the JAVA_HOME environment variable.

src/java/containers/tomcat.go

@@ -63,45 +63,22 @@ func (t *TomcatContainer) Supply() error {
 	}
 
 	if javaHome != "" {
-		javaMajorVersion, versionErr := common.DetermineJavaVersion(javaHome)
-		if versionErr == nil {
-			tomcatVersion := DetermineTomcatVersion(t.config.Tomcat.Version)
-			t.context.Log.Debug("Detected Java major version: %d", javaMajorVersion)
-
-			// Select Tomcat version pattern based on Java version
-			var versionPattern string
-			if tomcatVersion == "" {
-				t.context.Log.Info("Tomcat version not specified")
-				if javaMajorVersion >= 11 {
-					// Java 11+: Use Tomcat 10.x (Jakarta EE 9+)
-					versionPattern = "10.x"
-					t.context.Log.Info("Using Tomcat 10.x for Java %d", javaMajorVersion)
-				} else {
-					// Java 8-10: Use Tomcat 9.x (Java EE 8)
-					versionPattern = "9.x"
-					t.context.Log.Info("Using Tomcat 9.x for Java %d", javaMajorVersion)
-				}
-			} else {
-				versionPattern = tomcatVersion
-				t.context.Log.Info("Using Tomcat %s for Java %d", versionPattern, javaMajorVersion)
-			}
-
-			if strings.HasPrefix(versionPattern, "10.") && javaMajorVersion < 11 {
-				return fmt.Errorf("Tomcat 10.x requires Java 11+, but Java %d detected", javaMajorVersion)
-			}
+		versionPattern, versionErr := SelectTomcatVersionPattern(javaHome, DetermineTomcatVersion(t.config.Tomcat.Version))
+		if versionErr != nil {
+			return versionErr
+		}
 
-			// Resolve the version pattern to actual version using libbuildpack
+		if versionPattern != "" {
 			allVersions := t.context.Manifest.AllDependencyVersions("tomcat")
 			resolvedVersion, err := libbuildpack.FindMatchingVersion(versionPattern, allVersions)
 			if err != nil {
 				return fmt.Errorf("tomcat version resolution error for pattern %q: %w", versionPattern, err)
 			}
-
 			dep.Name = "tomcat"
 			dep.Version = resolvedVersion
 			t.context.Log.Debug("Resolved Tomcat version pattern '%s' to %s", versionPattern, resolvedVersion)
 		} else {
-			t.context.Log.Warning("Unable to determine Java version: %s", versionErr.Error())
+			t.context.Log.Warning("Unable to determine Java version from JAVA_HOME, falling back to manifest default Tomcat version")
 		}
 	}
 
@@ -462,7 +439,37 @@ func getKeys(m map[string]string) []string {
 	return keys
 }
 
-// DetermineTomcatVersion determines the version of the tomcat
+// SelectTomcatVersionPattern determines the Tomcat version pattern to use based on the
+// detected Java version and any user-configured Tomcat version.
+// Returns ("", nil) when Java version cannot be determined — the caller should fall back
+// to the manifest default (matching Ruby buildpack behaviour).
+func SelectTomcatVersionPattern(javaHome, configVersion string) (string, error) {
+	if javaHome == "" {
+		return "", nil
+	}
+
+	javaMajorVersion, err := common.DetermineJavaVersion(javaHome)
+	if err != nil {
+		if configVersion != "" {
+			return configVersion, nil
+		}
+		return "", nil
+	}
+
+	if configVersion != "" {
+		if strings.HasPrefix(configVersion, "10.") && javaMajorVersion < 11 {
+			return "", fmt.Errorf("Tomcat 10.x requires Java 11+, but Java %d detected", javaMajorVersion)
+		}
+		return configVersion, nil
+	}
+
+	if javaMajorVersion >= 11 {
+		return "10.x", nil
+	}
+	return "9.x", nil
+}
+
+
 // based on the JBP_CONFIG_TOMCAT field from manifest.
 // It looks for a tomcat block with a version of the form "<major>.+" (e.g. "9.+", "10.+", "10.1.+").
 // Returns the pattern with "+" replaced by "*" (e.g. "9.*", "10.*", "10.1.*") so libbuildpack can resolve it.

src/java/containers/tomcat_test.go

@@ -197,6 +197,62 @@ var _ = Describe("Tomcat Container", func() {
 		})
 	})
 
+	Describe("SelectTomcatVersionPattern", func() {
+		var javaHome string
+
+		BeforeEach(func() {
+			var err error
+			javaHome, err = os.MkdirTemp("", "javahome")
+			Expect(err).NotTo(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			os.RemoveAll(javaHome)
+		})
+
+		writeReleaseFile := func(content string) {
+			err := os.WriteFile(filepath.Join(javaHome, "release"), []byte(content), 0644)
+			Expect(err).NotTo(HaveOccurred())
+		}
+
+		Context("when release file is missing", func() {
+			It("returns empty pattern to fall back to manifest default, not assume Java 17", func() {
+				pattern, err := containers.SelectTomcatVersionPattern(javaHome, "")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(pattern).To(Equal(""))
+			})
+
+			It("still honours an explicitly configured tomcat version", func() {
+				pattern, err := containers.SelectTomcatVersionPattern(javaHome, "9.*")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(pattern).To(Equal("9.*"))
+			})
+		})
+
+		Context("happy path version selection", func() {
+			It("selects Tomcat 10.x for Java 11+", func() {
+				writeReleaseFile("JAVA_VERSION=\"11.0.20\"\n")
+				pattern, err := containers.SelectTomcatVersionPattern(javaHome, "")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(pattern).To(Equal("10.x"))
+			})
+
+			It("selects Tomcat 9.x for Java 8", func() {
+				writeReleaseFile("JAVA_VERSION=\"1.8.0_372\"\n")
+				pattern, err := containers.SelectTomcatVersionPattern(javaHome, "")
+				Expect(err).NotTo(HaveOccurred())
+				Expect(pattern).To(Equal("9.x"))
+			})
+
+			It("errors when Tomcat 10.x is requested but Java 8 detected", func() {
+				writeReleaseFile("JAVA_VERSION=\"1.8.0_372\"\n")
+				_, err := containers.SelectTomcatVersionPattern(javaHome, "10.*")
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(ContainSubstring("Java 11+"))
+			})
+		})
+	})
+
 	Describe("determineTomcatVersion", func() {
 		It("returns empty string when JBP_CONFIG_TOMCAT is empty", func() {
 			v := containers.DetermineTomcatVersion("")