Skip to content

Commit

Permalink
Shore up filesystem interactions
Browse files Browse the repository at this point in the history
- When using user input to drive `subprocess` calls, the exception
  catching now includes `OSError` to account for unexpected aspects of
  the executables being run.
- For instance, if the filepath in `JAVA_HOME` is not exlusively made up
  of directories, then `subprocess` will raise `NotADirectoryError`. If
  `JAVA_HOME` points to a binary itself such as`/usr/lib/jvm/bin/java`,
  then the check for `javac`, i.e.
  `subprocess.check_output(['/usr/lib/jvm/bin/java/bin/javac', '-version'])`
  raises `NotADirectoryError` since `java` is not a directory.
- Additionally, checks for existence in the filesystem that require the
  path to be either a file or a directory, now check specifically for that
  characteristic.
  • Loading branch information
rmartin16 committed Mar 22, 2023
1 parent 8212026 commit a9f9d2a
Show file tree
Hide file tree
Showing 15 changed files with 94 additions and 68 deletions.
1 change: 1 addition & 0 deletions changes/1144.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When ``JAVA_HOME`` contains a path to a file instead of a directory, Briefcase will now warn the user and install an isolated copy of Java instead of logging a ``NotADirectoryError``.
16 changes: 11 additions & 5 deletions src/briefcase/commands/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ def validate_data_path(self, data_path):
briefcase_home = os.environ["BRIEFCASE_HOME"]
data_path = Path(briefcase_home).resolve()
# Path("") converts to ".", so check for that edge case.
if briefcase_home == "" or not data_path.exists():
if briefcase_home == "" or not data_path.is_dir():
raise BriefcaseCommandError(
"The path specified by BRIEFCASE_HOME does not exist."
)
Expand Down Expand Up @@ -332,8 +332,13 @@ def _load_path_index(self, app: BaseConfig):
:param app: The config object for the app
:return: The contents of the application path index.
"""
with (self.bundle_path(app) / "briefcase.toml").open("rb") as f:
self._path_index[app] = tomllib.load(f)["paths"]
try:
with (self.bundle_path(app) / "briefcase.toml").open("rb") as f:
self._path_index[app] = tomllib.load(f)["paths"]
except OSError as e:
raise BriefcaseCommandError(
f"Unable to find '{self.bundle_path(app) / 'briefcase.toml'}'"
) from e
return self._path_index[app]

def support_path(self, app: BaseConfig):
Expand Down Expand Up @@ -729,9 +734,10 @@ def parse_config(self, filename):
msg=f"Configuration for '{app_name}'",
)

except FileNotFoundError as e:
except OSError as e:
raise BriefcaseConfigError(
f"""Configuration file not found.
f"""\
Configuration file not found.
Did you run Briefcase in a project directory that contains {filename.name!r}?"""
) from e
Expand Down
3 changes: 1 addition & 2 deletions src/briefcase/commands/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ def _build_app(
:param no_update: Should automated updates be disabled?
:param test_mode: Is the app being build in test mode?
"""
target_file = self.bundle_path(app)
if not target_file.exists():
if not self.bundle_path(app).exists():
state = self.create_command(app, test_mode=test_mode, **options)
elif (
update # An explicit update has been requested
Expand Down
54 changes: 28 additions & 26 deletions src/briefcase/integrations/android_sdk.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,22 +206,22 @@ def verify(cls, tools: ToolCache, install=True):
if (sdk_root_path / "tools").exists():
tools.logger.warning(
f"""
*************************************************************************
** WARNING: Upgrading Android SDK tools **
*************************************************************************
*************************************************************************
** WARNING: Upgrading Android SDK tools **
*************************************************************************
Briefcase needs to replace the older Android SDK Tools with the
newer Android SDK Command-Line Tools. This will involve some large
downloads, as well as re-accepting the licenses for the Android
SDKs.
Briefcase needs to replace the older Android SDK Tools with the
newer Android SDK Command-Line Tools. This will involve some large
downloads, as well as re-accepting the licenses for the Android
SDKs.
Any emulators created with the older Android SDK Tools will not be
compatible with the new tools. You will need to create new
emulators. Old emulators can be removed by deleting the files
in {sdk.avd_path} matching the emulator name.
Any emulators created with the older Android SDK Tools will not be
compatible with the new tools. You will need to create new
emulators. Old emulators can be removed by deleting the files
in {sdk.avd_path} matching the emulator name.
*************************************************************************
"""
*************************************************************************
"""
)
tools.shutil.rmtree(sdk_root_path)

Expand All @@ -242,7 +242,7 @@ def exists(self):
Look for the sdkmanager; and, if necessary, confirm that it is executable.
"""
return self.sdkmanager_path.exists() and (
return self.sdkmanager_path.is_file() and (
self.tools.host_os == "Windows"
or self.tools.os.access(self.sdkmanager_path, self.tools.os.X_OK)
)
Expand Down Expand Up @@ -384,7 +384,7 @@ def verify_license(self):
check=True,
stream_output=False,
)
except subprocess.CalledProcessError as e:
except (subprocess.CalledProcessError, OSError) as e:
raise BriefcaseCommandError(
f"""\
Error while reviewing Android SDK licenses. Please run this command and examine
Expand Down Expand Up @@ -453,10 +453,7 @@ def verify_avd(self, avd):
# Read the AVD configuration to retrieve the system image.
# This is stored in the AVD configuration file with the key:
# image.sysdir.1=system-images/android-31/default/arm64-v8a/
try:
avd_config = self.avd_config(avd)
except FileNotFoundError:
raise BriefcaseCommandError(f"Unable to read configuration of AVD @{avd}")
avd_config = self.avd_config(avd)

try:
system_image_path = Path(avd_config["image.sysdir.1"])
Expand Down Expand Up @@ -1022,13 +1019,18 @@ def avd_config(self, avd):
"""
# Parse the existing config into key-value pairs
avd_config = {}
with self.avd_config_filename(avd).open("r") as f:
for line in f:
try:
key, value = line.rstrip().split("=", 1)
avd_config[key.strip()] = value.strip()
except ValueError:
pass
try:
with self.avd_config_filename(avd).open("r") as f:
for line in f:
try:
key, value = line.rstrip().split("=", 1)
avd_config[key.strip()] = value.strip()
except ValueError:
pass
except OSError as e:
raise BriefcaseCommandError(
f"Unable to read configuration of AVD @{avd}"
) from e

return avd_config

Expand Down
2 changes: 1 addition & 1 deletion src/briefcase/integrations/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ def verify(cls, tools: ToolCache):
tools.logger.warning(cls.UNKNOWN_DOCKER_VERSION_WARNING)
except subprocess.CalledProcessError:
tools.logger.warning(cls.DOCKER_INSTALLATION_STATUS_UNKNOWN_WARNING)
except FileNotFoundError as e:
except OSError as e:
# Docker executable doesn't exist.
raise BriefcaseCommandError(
cls.DOCKER_NOT_INSTALLED_ERROR.format(
Expand Down
4 changes: 2 additions & 2 deletions src/briefcase/integrations/flatpak.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def verify(cls, tools: ToolCache):
"""
)

except FileNotFoundError as e:
except OSError as e:
raise BriefcaseCommandError(
"""\
Briefcase requires the Flatpak toolchain, but it does not appear to be installed.
Expand Down Expand Up @@ -122,7 +122,7 @@ def verify(cls, tools: ToolCache):
"""
)

except FileNotFoundError as e:
except OSError as e:
raise BriefcaseCommandError(
"""\
Briefcase requires the full Flatpak development toolchain, but flatpak-builder
Expand Down
2 changes: 1 addition & 1 deletion src/briefcase/integrations/java.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def verify(cls, tools: ToolCache, install=True):
"""

except FileNotFoundError:
except OSError:
install_message = f"""
*************************************************************************
** WARNING: JAVA_HOME does not point to a JDK **
Expand Down
4 changes: 2 additions & 2 deletions src/briefcase/integrations/linuxdeploy.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,11 @@ def install(self):
self.local_path / self.file_name,
self.file_path / self.file_name,
)
except FileNotFoundError:
except OSError as e:
raise BriefcaseCommandError(
f"Could not locate linuxdeploy plugin {self.local_path / self.file_name}. "
"Is the path correct?"
)
) from e

self.prepare_executable()

Expand Down
5 changes: 3 additions & 2 deletions src/briefcase/integrations/visualstudio.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def verify(cls, tools: ToolCache):
pass
except subprocess.CalledProcessError as e:
raise BriefcaseCommandError(
"""MSBuild is on the path, but Briefcase cannot start it."""
"MSBuild is on the path, but Briefcase cannot start it."
) from e

# try to find Visual Studio
Expand Down Expand Up @@ -137,6 +137,7 @@ def verify(cls, tools: ToolCache):
KeyError,
CommandOutputParseError,
subprocess.CalledProcessError,
OSError,
) as e:
raise BriefcaseCommandError(
f"""\
Expand Down Expand Up @@ -174,7 +175,7 @@ def verify(cls, tools: ToolCache):
# Try to invoke MSBuild at the established location
try:
tools.subprocess.check_output([msbuild_path, "--version"])
except subprocess.CalledProcessError:
except (subprocess.CalledProcessError, OSError):
raise BriefcaseCommandError(
"MSBuild appears to exist, but Briefcase can't start it."
)
Expand Down
13 changes: 8 additions & 5 deletions src/briefcase/integrations/wix.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,11 @@ def verify(cls, tools: ToolCache, install=True):
if not wix.exists():
raise BriefcaseCommandError(
f"""\
The WIX environment variable does not point to an install of the
WiX Toolset. Current value: {wix_home!r}
The WIX environment variable:
{wix_home}
does not point to an install of the WiX Toolset.
"""
)

Expand All @@ -106,9 +109,9 @@ def verify(cls, tools: ToolCache, install=True):

def exists(self):
return (
self.heat_exe.exists()
and self.light_exe.exists()
and self.candle_exe.exists()
self.heat_exe.is_file()
and self.light_exe.is_file()
and self.candle_exe.is_file()
)

@property
Expand Down
4 changes: 2 additions & 2 deletions src/briefcase/platforms/linux/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ def _install_app_requirements(
self.tools.shutil.rmtree(local_requirements_path)
self.tools.os.mkdir(local_requirements_path)

# Iterate over every requirements, looking for local references
# Iterate over every requirement, looking for local references
for requirement in requires:
if _is_local_requirement(requirement):
if Path(requirement).is_dir():
Expand All @@ -156,7 +156,7 @@ def _install_app_requirements(
try:
# Requirement is an existing sdist or wheel file.
self.tools.shutil.copy(requirement, local_requirements_path)
except FileNotFoundError as e:
except OSError as e:
raise BriefcaseCommandError(
f"Unable to find local requirement {requirement}"
) from e
Expand Down
26 changes: 13 additions & 13 deletions src/briefcase/platforms/linux/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ def finalize_app_config(self, app: AppConfig):
# form of this to pass coverage, you get a shiny penny. For
# some reason, coverage generated on Py3.9, but reported on
# Py3.10+, finds a missing branch from the `with` statement
# to the first line after the `except FileNotFound` below.
# to the first line after the `except OSError` below.
# Since this is (a) a very simple file I/O sequence, and
# (b) will be removed once we're at a Python3.10 minimum,
# I can live with the Old Skool I/O calls.
Expand All @@ -208,11 +208,11 @@ def finalize_app_config(self, app: AppConfig):
else:
freedesktop_info = self.tools.platform.freedesktop_os_release()

except FileNotFoundError:
except OSError as e:
raise BriefcaseCommandError(
"Could not find the /etc/os-release file. "
"Is this a FreeDesktop-compliant Linux distribution?"
)
) from e

# Process the FreeDesktop content to give the vendor, codename and vendor base.
(
Expand Down Expand Up @@ -576,7 +576,7 @@ def build_app(self, app: AppConfig, **kwargs):

with self.input.wait_bar("Installing license..."):
license_file = self.base_path / "LICENSE"
if license_file.exists():
if license_file.is_file():
self.tools.shutil.copy(license_file, doc_folder / "copyright")
else:
raise BriefcaseCommandError(
Expand All @@ -590,7 +590,7 @@ def build_app(self, app: AppConfig, **kwargs):

with self.input.wait_bar("Installing changelog..."):
changelog = self.base_path / "CHANGELOG"
if changelog.exists():
if changelog.is_file():
with changelog.open() as infile:
outfile = gzip.GzipFile(
doc_folder / "changelog.gz", mode="wb", mtime=0
Expand Down Expand Up @@ -620,7 +620,7 @@ def build_app(self, app: AppConfig, **kwargs):

with self.input.wait_bar("Installing man page..."):
manpage_source = self.bundle_path(app) / f"{app.app_name}.1"
if manpage_source.exists():
if manpage_source.is_file():
with manpage_source.open() as infile:
outfile = gzip.GzipFile(
man_folder / f"{app.app_name}.1.gz", mode="wb", mtime=0
Expand Down Expand Up @@ -774,10 +774,12 @@ def _package_deb(self, app: AppConfig, **kwargs):

# Write the Debian metadata control file.
with self.input.wait_bar("Write Debian package control file..."):
if (self.project_path(app) / "DEBIAN").exists():
self.tools.shutil.rmtree(self.project_path(app) / "DEBIAN")
DEBIAN_path = self.project_path(app) / "DEBIAN"

if DEBIAN_path.exists():
self.tools.shutil.rmtree(DEBIAN_path)

(self.project_path(app) / "DEBIAN").mkdir()
DEBIAN_path.mkdir()

# Add runtime package dependencies. App config has been finalized,
# so this will be the target-specific definition, if one exists.
Expand All @@ -791,9 +793,7 @@ def _package_deb(self, app: AppConfig, **kwargs):
+ getattr(app, "system_runtime_requires", [])
)

with (self.project_path(app) / "DEBIAN" / "control").open(
"w", encoding="utf-8"
) as f:
with (DEBIAN_path / "control").open("w", encoding="utf-8") as f:
f.write(
"\n".join(
[
Expand Down Expand Up @@ -946,7 +946,7 @@ def _package_rpm(self, app: AppConfig, **kwargs):
# Add the changelog content to the bottom of the spec file.
f.write("\n%changelog\n")
changelog_source = self.base_path / "CHANGELOG"
if not changelog_source.exists():
if not changelog_source.is_file():
raise BriefcaseCommandError(
"""\
Your project does not contain a CHANGELOG file.
Expand Down
2 changes: 1 addition & 1 deletion src/briefcase/platforms/web/static.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ def end_headers(self):


class LocalHTTPServer(ThreadingHTTPServer):
"""A HTTP server that serves local static content."""
"""An HTTP server that serves local static content."""

def __init__(self, base_path, host, port, RequestHandlerClass=HTTPHandler):
self.base_path = base_path
Expand Down
8 changes: 8 additions & 0 deletions tests/commands/create/test_properties.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import pytest
import tomli_w

from briefcase.exceptions import BriefcaseCommandError


def test_template_url(create_command):
"""The template URL is a simple construction of the platform and format."""
Expand All @@ -9,6 +12,11 @@ def test_template_url(create_command):
)


def test_missing_briefcase_toml(create_command, myapp):
with pytest.raises(BriefcaseCommandError, match=r"Unable to find.*briefcase.toml'"):
_ = create_command.app_path(myapp)


def test_app_path(create_command, myapp):
bundle_path = create_command.bundle_path(myapp)
bundle_path.mkdir(parents=True)
Expand Down
Loading

0 comments on commit a9f9d2a

Please # to comment.