Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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
110 changes: 110 additions & 0 deletions constructor/briefcase.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,115 @@ def get_license(info):
return {"file": str(placeholder_license)} # convert to str for TOML serialization


def is_bat_file(file_path: Path) -> bool:
return file_path.is_file() and file_path.suffix.lower() == ".bat"


def create_install_options_list(info: dict) -> list[dict]:
"""Returns a list of dicts with data formatted for the installation options page."""
options = []

# Register Python (if Python is bundled)
has_python = False
for item in info.get("_dists", []):
if item.startswith("python-"):
components = item.split("-") # python-x.y.z-<build number>.suffix
python_version = ".".join(components[1].split(".")[:-1]) # create the string "x.y"
has_python = True
break

if has_python:
register_python = info.get("register_python", True)
if register_python:
options.append(
{
"name": "register_python",
"title": f"Register {info['name']} as my default Python {python_version}.",
"description": "Allows other programs, such as VSCode, PyCharm, etc. to automatically "
f"detect {info['name']} as the primary Python {python_version} on the system.",
"default": info.get("register_python_default", False),
}
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if has_python:
register_python = info.get("register_python", True)
if register_python:
options.append(
{
"name": "register_python",
"title": f"Register {info['name']} as my default Python {python_version}.",
"description": "Allows other programs, such as VSCode, PyCharm, etc. to automatically "
f"detect {info['name']} as the primary Python {python_version} on the system.",
"default": info.get("register_python_default", False),
}
)
if has_python and info.get("register_python", True)
options.append(
{
"name": "register_python",
"title": f"Register {info['name']} as my default Python {python_version}.",
"description": "Allows other programs, such as VSCode, PyCharm, etc. to automatically "
f"detect {info['name']} as the primary Python {python_version} on the system.",
"default": info.get("register_python_default", False),
}
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1095957


# Initialize conda
initialize_conda = info.get("initialize_conda", "classic")
if initialize_conda:
if initialize_conda == "condabin":
description = (
"Adds condabin, which only contains the 'conda' executables, to PATH. "
"Does not require special shortcuts but activation needs "
"to be performed manually."
)
else:
description = (
"NOT recommended. This can lead to conflicts with other applications. "
"Instead, use the Command Prompt and Powershell menus added to the Windows Start Menu."
)
options.append(
{
"name": "initialize_conda",
"title": "Add installation to my PATH environment variable",
"description": description,
"default": info.get("initialize_by_default", False),
}
)

# Keep package option (presented to the user as a negation (clear package cache))
clear_package_cache = not info.get("keep_pkgs", False)
options.append(
{
"name": "clear_package_cache",
"title": "Clear the package cache upon completion",
"description": "Recommended. Recovers some disk space without harming functionality.",
"default": clear_package_cache,
}
)

# Enable shortcuts
if info.get("_enable_shortcuts", False) is True:
options.append(
{
"name": "enable_shortcuts",
"title": "Create shortcuts",
"description": "Create shortcuts (supported packages only).",
"default": False,
}
)

# Pre/Post install script
for script_type in ["pre", "post"]:
script_description = info.get(f"{script_type}_install_desc", "")
script = info.get(f"{script_type}_install", "")
if script_description and not script:
raise ValueError(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise ValueError(
raise KeyError(

Technically since it is a key in construct.yaml?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point but it seemed a bit odd with a KeyError here, to me a KeyError always looks like an uncaught exception. I was also thinking of creating a custom exception but since constructor only use default exceptions I decided to stick with it.
Do we raise KeyError in general for yaml-files that are not "proper"?

f"{script_type}_install_desc was set, but {script_type}_install was not!"
)

if script:
script_path = Path(script)
if not script_path.is_file():
raise FileNotFoundError(
f"Specified {script_type}-install script '{script}' does not exist."
)
if not is_bat_file(script_path):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just check for the suffix here. The only thing is_bat_file does additionally is checking whether the file exists - which you do immediately before this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the first file check and updated the error message. Let me know if that works, 1095957.
Initially I coped the function is_bat_file from from the Uninstallation MSI branch, which is why I forgot to adjust to the double is_file check.

raise OSError(
f"Specified {script_type}-install script '{script}' must be a '.bat' file."
)

# The UI option is only displayed if a description is set
if script_description:
options.append(
{
"name": f"{script_type}_install_script",
"title": f"{script_type.capitalize()}-install script",
"description": script_description,
"default": False,
}
)

return options


# Create a Briefcase configuration file. Using a full TOML writer rather than a Jinja
# template allows us to avoid escaping strings everywhere.
def write_pyproject_toml(tmp_dir, info):
Expand All @@ -134,6 +243,7 @@ def write_pyproject_toml(tmp_dir, info):
"use_full_install_path": False,
"install_launcher": False,
"post_install_script": str(BRIEFCASE_DIR / "run_installation.bat"),
"install_option": create_install_options_list(info),
}
},
}
Expand Down
2 changes: 1 addition & 1 deletion constructor/winexe.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ def make_nsi(
variables["initialize_by_default"] = info.get("initialize_by_default", None)
variables["check_path_length"] = info.get("check_path_length", False)
variables["check_path_spaces"] = info.get("check_path_spaces", True)
variables["keep_pkgs"] = info.get("keep_pkgs") or False
variables["keep_pkgs"] = info.get("keep_pkgs", False)
variables["pre_install_exists"] = bool(info.get("pre_install"))
variables["post_install_exists"] = bool(info.get("post_install"))
variables["with_conclusion_text"] = bool(conclusion_text)
Expand Down
Loading