Repository: beeware/briefcase

Issue:
Confusing message about JAVA_HOME
### Describe the bug

I got this message on macOS:
```
*************************************************************************
** WARNING: JAVA_HOME does not point to a Java 17 JDK                  **
*************************************************************************

    Android requires a Java 17 JDK, but the location pointed to by the
    JAVA_HOME environment variable:

    /Library/Java/JavaVirtualMachines/amazon-corretto-8.jdk/Contents/Home

    isn't a Java 17 JDK (it appears to be Java 1.8.0_352).

    Briefcase will use its own JDK instance.
```

This didn't make any sense, because I didn't have any JAVA_HOME environment variable. It turns out it was actually getting the path by running /usr/libexec/java_home.

### Steps to reproduce

* Install a Java version other than 17, and make sure its location is returned by /usr/libexec/java_home
* Remove or rename Briefcase's existing Java 17 installation, if any
* Unset JAVA_HOME
* Run `briefcase build android`

### Expected behavior

The message should correctly indicate where the path came from.

### Screenshots

_No response_

### Environment

- Operating System: macOS
- Python version: 3.8
- Software versions:
  - Briefcase: 96479a0cab4814ea5ed359d2431628d145e86698


### Logs

_No response_

### Additional context

_No response_

Discussion:

Patch:
diff --git a/changes/1305.bugfix.rst b/changes/1305.bugfix.rst
new file mode 100644
index 00000000..c4729a0c
--- /dev/null
+++ b/changes/1305.bugfix.rst
@@ -0,0 +1,1 @@
+An warning is no longer logged if the Java identified by macOS is not usable by Briefcase.
diff --git a/src/briefcase/integrations/java.py b/src/briefcase/integrations/java.py
index 5a465d5f..4f041349 100644
--- a/src/briefcase/integrations/java.py
+++ b/src/briefcase/integrations/java.py
@@ -51,6 +51,30 @@ class JDK(ManagedTool):
             f"{self.JDK_RELEASE}_{self.JDK_BUILD}.{extension}"
         )
 
+    @classmethod
+    def version_from_path(cls, tools: ToolCache, java_path: str | Path) -> str:
+        """Return a JDK's version from a path by running ``<java_path>/bin/javac``.
+
+        This will fail if the path contains a JRE instead of a JDK.
+
+        Several exceptions can be raised for issues:
+         - OSError - the ``javac`` executable doesn't exist
+         - CalledProcessError - ``javac`` returned a non-zero value
+         - IndexError - unparsable version value from ``javac``
+
+        :param tools: ToolCache of available tools
+        :param java_path: File path to a candidate JDK install
+        :return: JDK release version; e.g. "17.0.7"
+        """
+        output = tools.subprocess.check_output(
+            [
+                os.fsdecode(Path(java_path) / "bin" / "javac"),
+                "-version",
+            ],
+        )
+        # javac's output should look like "javac 17.0.7\n"
+        return output.strip("\n").split(" ")[1]
+
     @classmethod
     def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> JDK:
         """Verify that a Java JDK exists.
@@ -74,38 +98,14 @@ class JDK(ManagedTool):
             return tools.java
 
         java = None
-        java_home = tools.os.environ.get("JAVA_HOME", "")
         install_message = None
 
-        # macOS has a helpful system utility to determine JAVA_HOME. Try it.
-        if not java_home and tools.host_os == "Darwin":
-            try:
-                # If no JRE/JDK is installed, /usr/libexec/java_home
-                # raises an error.
-                java_home = tools.subprocess.check_output(
-                    ["/usr/libexec/java_home"],
-                ).strip("\n")
-            except subprocess.CalledProcessError:
-                # No java on this machine.
-                pass
-
-        if java_home:
+        if java_home := tools.os.environ.get("JAVA_HOME", ""):
             try:
-                # If JAVA_HOME is defined, try to invoke javac.
-                # This verifies that we have a JDK, not a just a JRE.
-                output = tools.subprocess.check_output(
-                    [
-                        os.fsdecode(Path(java_home) / "bin" / "javac"),
-                        "-version",
-                    ],
-                )
-                # This should be a string of the form "javac 17.0.7\n"
-                version_str = output.strip("\n").split(" ")[1]
-                if version_str == cls.JDK_RELEASE:
-                    # It appears to be a Java JDK
+                version_str = cls.version_from_path(tools, java_home)
+                if version_str.split(".")[0] == cls.JDK_MAJOR_VER:
                     java = JDK(tools, java_home=Path(java_home))
                 else:
-                    # It's not a Java JDK.
                     install_message = f"""
 *************************************************************************
 ** WARNING: JAVA_HOME does not point to a Java {cls.JDK_MAJOR_VER} JDK                  **
@@ -121,7 +121,6 @@ class JDK(ManagedTool):
     Briefcase will use its own JDK instance.
 
 *************************************************************************
-
 """
 
             except OSError:
@@ -139,7 +138,6 @@ class JDK(ManagedTool):
     Briefcase will use its own JDK instance.
 
 *************************************************************************
-
 """
 
             except subprocess.CalledProcessError:
@@ -166,7 +164,6 @@ class JDK(ManagedTool):
     from the command prompt.
 
 *************************************************************************
-
 """
 
             except IndexError:
@@ -193,12 +190,32 @@ class JDK(ManagedTool):
     from the command prompt.
 
 *************************************************************************
-
 """
 
+        # macOS has a helpful system utility to determine JAVA_HOME. Try it.
+        elif tools.host_os == "Darwin":
+            try:
+                # If /usr/libexec/java_home doesn't exist, OSError will be raised
+                # If no JRE/JDK is installed, /usr/libexec/java_home raises an error
+                java_home = tools.subprocess.check_output(
+                    ["/usr/libexec/java_home"],
+                ).strip("\n")
+            except (OSError, subprocess.CalledProcessError):
+                pass  # No java on this machine
+            else:
+                try:
+                    version_str = cls.version_from_path(tools, java_home)
+                    if version_str.split(".")[0] == cls.JDK_MAJOR_VER:
+                        java = JDK(tools, java_home=Path(java_home))
+                except (OSError, subprocess.CalledProcessError, IndexError):
+                    pass  # do not alert user if macOS found an unqualified JDK
+
         if java is None:
-            # If we've reached this point, any user-provided JAVA_HOME is broken;
-            # use the Briefcase one.
+            # Inform the user if the user-specified JDK wasn't valid
+            if install_message:
+                tools.logger.warning(install_message)
+
+            # Use the Briefcase JDK install
             java_home = tools.base_path / cls.JDK_INSTALL_DIR_NAME
 
             # The macOS download has a weird layout (inherited from the official Oracle
@@ -210,10 +227,6 @@ class JDK(ManagedTool):
 
             if not java.exists():
                 if install:
-                    # We only display the warning messages on the pass where we actually
-                    # install the JDK.
-                    if install_message:
-                        tools.logger.warning(install_message)
                     tools.logger.info(
                         f"A Java {cls.JDK_MAJOR_VER} JDK was not found; downloading and installing...",
                         prefix=cls.name,


Test patch:
diff --git a/tests/integrations/java/test_JDK__verify.py b/tests/integrations/java/test_JDK__verify.py
index 36d6886b..732057be 100644
--- a/tests/integrations/java/test_JDK__verify.py
+++ b/tests/integrations/java/test_JDK__verify.py
@@ -94,6 +94,70 @@ def test_macos_tool_failure(mock_tools, tmp_path, capsys):
     assert output.err == ""
 
 
+def test_macos_wrong_jdk_version(mock_tools, tmp_path, capsys):
+    """On macOS, if the libexec tool returns an unqualified JDK, the Briefcase JDK is
+    used."""
+    # Mock being on macOS
+    mock_tools.host_os = "Darwin"
+
+    # Mock 2 calls to check_output.
+    mock_tools.subprocess.check_output.side_effect = [
+        "/path/to/java",
+        "java 1.8.0_352\n",
+    ]
+
+    # Create a directory to make it look like the Briefcase Java already exists.
+    (tmp_path / "tools" / "java17" / "Contents" / "Home" / "bin").mkdir(parents=True)
+
+    # Create a JDK wrapper by verification
+    jdk = JDK.verify(mock_tools)
+
+    # The JDK should have the briefcase JAVA_HOME
+    assert jdk.java_home == tmp_path / "tools" / "java17" / "Contents" / "Home"
+
+    assert mock_tools.subprocess.check_output.mock_calls == [
+        CALL_JAVA_HOME,
+        mock.call([os.fsdecode(Path("/path/to/java/bin/javac")), "-version"]),
+    ]
+
+    # No console output
+    output = capsys.readouterr()
+    assert output.out == ""
+    assert output.err == ""
+
+
+def test_macos_invalid_jdk_path(mock_tools, tmp_path, capsys):
+    """On macOS, if the libexec tool points to non-JDK path, the Briefcase JDK is
+    used."""
+    # Mock being on macOS
+    mock_tools.host_os = "Darwin"
+
+    # Mock 2 calls to check_output.
+    mock_tools.subprocess.check_output.side_effect = [
+        "/path/to/java",
+        IndexError,
+    ]
+
+    # Create a directory to make it look like the Briefcase Java already exists.
+    (tmp_path / "tools" / "java17" / "Contents" / "Home" / "bin").mkdir(parents=True)
+
+    # Create a JDK wrapper by verification
+    jdk = JDK.verify(mock_tools)
+
+    # The JDK should have the briefcase JAVA_HOME
+    assert jdk.java_home == tmp_path / "tools" / "java17" / "Contents" / "Home"
+
+    assert mock_tools.subprocess.check_output.mock_calls == [
+        CALL_JAVA_HOME,
+        mock.call([os.fsdecode(Path("/path/to/java/bin/javac")), "-version"]),
+    ]
+
+    # No console output
+    output = capsys.readouterr()
+    assert output.out == ""
+    assert output.err == ""
+
+
 def test_macos_provided_overrides_tool_java_home(mock_tools, capsys):
     """On macOS, an explicit JAVA_HOME overrides /usr/libexec/java_home."""
     # Mock being on macOS
@@ -182,9 +246,10 @@ def test_invalid_jdk_version(mock_tools, host_os, java_home, tmp_path, capsys):
         [os.fsdecode(Path("/path/to/java/bin/javac")), "-version"],
     )
 
-    # No console output (because Briefcase JDK exists)
+    # Warning is shown for invalid JAVA_HOME
     output = capsys.readouterr()
-    assert output.out == ""
+    assert "WARNING: JAVA_HOME does not point to a Java 17 JDK" in output.out
+    assert output.out.endswith("****\n")
     assert output.err == ""
 
 
@@ -228,9 +293,10 @@ def test_no_javac(mock_tools, host_os, java_home, error_type, tmp_path, capsys):
         [os.fsdecode(Path("/path/to/nowhere/bin/javac")), "-version"],
     ),
 
-    # No console output (because Briefcase JDK exists)
+    # Warning is shown for invalid JAVA_HOME
     output = capsys.readouterr()
-    assert output.out == ""
+    assert "WARNING: JAVA_HOME does not point to a JDK" in output.out
+    assert output.out.endswith("****\n")
     assert output.err == ""
 
 
@@ -269,9 +335,10 @@ def test_javac_error(mock_tools, host_os, java_home, tmp_path, capsys):
         [os.fsdecode(Path("/path/to/nowhere/bin/javac")), "-version"],
     ),
 
-    # No console output (because Briefcase JDK exists)
+    # Warning is shown for invalid JAVA_HOME
     output = capsys.readouterr()
-    assert output.out == ""
+    assert "WARNING: Unable to invoke the Java compiler" in output.out
+    assert output.out.endswith("****\n")
     assert output.err == ""
 
 
@@ -310,7 +377,11 @@ def test_unparseable_javac_version(mock_tools, host_os, java_home, tmp_path, cap
 
     # No console output (because Briefcase JDK exists)
     output = capsys.readouterr()
-    assert output.out == ""
+    assert (
+        "WARNING: Unable to determine the version of Java that is installed"
+        in output.out
+    )
+    assert output.out.endswith("****\n")
     assert output.err == ""