Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Tool verification logging and feedback #1382

Merged
merged 1 commit into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/1382.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Tool verification for Java, Android SDK, and WiX were improved to provide more informative errors and debug logging.
17 changes: 16 additions & 1 deletion src/briefcase/integrations/android_sdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,8 @@ def verify_install(
sdk_root, sdk_env_source = cls.sdk_path_from_env(tools=tools)

if sdk_root:
tools.logger.debug("Evaluating ANDROID_HOME...", prefix=cls.full_name)
tools.logger.debug(f"{sdk_env_source}={sdk_root}")
sdk = AndroidSDK(tools=tools, root_path=Path(sdk_root))

if sdk.exists():
Expand Down Expand Up @@ -267,7 +269,14 @@ def verify_install(

doesn't appear to contain an Android SDK.

Briefcase will use its own SDK instance.
If {sdk_env_source} is an Android SDK, ensure it is the root directory
of the Android SDK instance such that

${sdk_env_source}/cmdline-tools/latest/bin/sdkmanager

is a valid filepath.

Briefcase will proceed using its own SDK instance.

*************************************************************************
"""
Expand Down Expand Up @@ -311,10 +320,16 @@ def verify_install(
"The Android SDK was not found; downloading and installing...",
prefix=cls.name,
)
tools.logger.info(
"To use an existing Android SDK instance, "
"specify its root directory path in the ANDROID_HOME environment variable."
)
tools.logger.info()
sdk.install()
else:
raise MissingToolError("Android SDK")

tools.logger.debug(f"Using Android SDK at {sdk.root_path}")
tools.android_sdk = sdk
return sdk

Expand Down
20 changes: 17 additions & 3 deletions src/briefcase/integrations/java.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> JDK
install_message = None

if java_home := tools.os.environ.get("JAVA_HOME", ""):
tools.logger.debug("Evaluating JAVA_HOME...", prefix=cls.full_name)
tools.logger.debug(f"JAVA_HOME={java_home}")
try:
version_str = cls.version_from_path(tools, java_home)
if version_str.split(".")[0] == cls.JDK_MAJOR_VER:
Expand All @@ -118,7 +120,7 @@ def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> JDK

isn't a Java {cls.JDK_MAJOR_VER} JDK (it appears to be Java {version_str}).

Briefcase will use its own JDK instance.
Briefcase will proceed using its own JDK instance.

*************************************************************************
"""
Expand All @@ -135,7 +137,10 @@ def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> JDK

does not appear to be a JDK. It may be a Java Runtime Environment.

Briefcase will use its own JDK instance.
If JAVA_HOME is a JDK, ensure it is the root directory of the JDK
instance such that $JAVA_HOME/bin/javac is a valid filepath.

Briefcase will proceed using its own JDK instance.

*************************************************************************
"""
Expand Down Expand Up @@ -194,14 +199,17 @@ def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> JDK

# macOS has a helpful system utility to determine JAVA_HOME. Try it.
elif tools.host_os == "Darwin":
tools.logger.debug(
"Evaluating /usr/libexec/java_home...", prefix=cls.full_name
)
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
tools.logger.debug("An existing JDK/JRE was not returned")
else:
try:
version_str = cls.version_from_path(tools, java_home)
Expand Down Expand Up @@ -231,10 +239,16 @@ def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> JDK
f"A Java {cls.JDK_MAJOR_VER} JDK was not found; downloading and installing...",
prefix=cls.name,
)
tools.logger.info(
f"To use an existing JDK {cls.JDK_MAJOR_VER} instance, "
f"specify its root directory path in the JAVA_HOME environment variable."
)
tools.logger.info()
java.install()
else:
raise MissingToolError("Java")

tools.logger.debug(f"Using JDK at {java.java_home}")
tools.java = java
return java

Expand Down
6 changes: 4 additions & 2 deletions src/briefcase/integrations/wix.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,9 @@ def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> WiX
return tools.wix

# Look for the WIX environment variable
wix_env = tools.os.environ.get("WIX")
if wix_env:
if wix_env := tools.os.environ.get("WIX"):
tools.logger.debug("Evaluating WIX...", prefix=cls.full_name)
tools.logger.debug(f"WIX={wix_env}")
wix_home = Path(wix_env)

# Set up the paths for the WiX executables we will use.
Expand Down Expand Up @@ -107,6 +108,7 @@ def verify_install(cls, tools: ToolCache, install: bool = True, **kwargs) -> WiX
else:
raise MissingToolError("WiX")

tools.logger.debug(f"Using WiX at {wix.wix_home}")
tools.wix = wix
return wix

Expand Down
12 changes: 10 additions & 2 deletions tests/integrations/android_sdk/AndroidSDK/test_verify.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,11 @@ def test_user_provided_sdk(mock_tools, env_var, tmp_path, capsys):
if env_var == "ANDROID_SDK_ROOT":
assert "Using Android SDK from ANDROID_SDK_ROOT" in capsys.readouterr().out
else:
assert capsys.readouterr().out == ""
assert capsys.readouterr().out == (
"\n>>> [Android SDK] Evaluating ANDROID_HOME...\n"
f">>> ANDROID_HOME={tmp_path / 'other_sdk'}\n"
f">>> Using Android SDK at {tmp_path / 'other_sdk'}\n"
)


def test_consistent_user_provided_sdk(mock_tools, tmp_path, capsys):
Expand Down Expand Up @@ -190,7 +194,11 @@ def test_consistent_user_provided_sdk(mock_tools, tmp_path, capsys):
assert sdk.root_path == existing_android_sdk_root_path

# User is not warned about configuration
assert capsys.readouterr().out == ""
assert capsys.readouterr().out == (
"\n>>> [Android SDK] Evaluating ANDROID_HOME...\n"
f">>> ANDROID_HOME={tmp_path / 'other_sdk'}\n"
f">>> Using Android SDK at {tmp_path / 'other_sdk'}\n"
)


def test_inconsistent_user_provided_sdk(mock_tools, tmp_path, capsys):
Expand Down
Loading