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.

Sign up for GitHub

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

Jump to bottom

1423 - Better PyApp integration #1547

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

1423 - Better PyApp integration #1547

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f1db91c
First implementation draft and test
johannesloibl Jun 3, 2024
6370530
Address review comments
johannesloibl Jun 3, 2024
09931cd
Fix typo in key name
johannesloibl Jun 3, 2024
8a5b56d
Fix another typo
johannesloibl Jun 3, 2024
5b233b0
refactor to use cached_property
johannesloibl Jun 3, 2024
48695df
Add context formatting for env vars
johannesloibl Jun 3, 2024
5dcbd86
Update documentation
johannesloibl Jun 3, 2024
96629d9
Merge branch 'master' into 1423-better-pyapp-integration
johannesloibl Jun 24, 2024
1dd0df1
Merge branch 'master' into 1423-better-pyapp-integration
johannesloibl Jul 17, 2024
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
186 changes: 125 additions & 61 deletions backend/src/hatchling/builders/binary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import sys
from typing import Any, Callable

from hatch.utils.structures import EnvVars
johannesloibl marked this conversation as resolved.
Show resolved Hide resolved
from hatchling.builders.config import BuilderConfig
from hatchling.builders.plugin.interface import BuilderInterface

Expand All @@ -17,6 +18,8 @@ def __init__(self, *args: Any, **kwargs: Any) -> None:
self.__scripts: list[str] | None = None
self.__python_version: str | None = None
self.__pyapp_version: str | None = None
self.__options: dict[str, str] | None = None
self.__build_targets: dict[str, dict[str, Any]] | None = None

@property
def scripts(self) -> list[str]:
Expand Down Expand Up @@ -78,6 +81,56 @@ def pyapp_version(self) -> str:

return self.__pyapp_version

@property
def options(self) -> dict[str, str]:
if self.__options is None:
options = self.target_config.get('options')

if options is None:
self.__options = {}
elif isinstance(options, dict):
self.__options = options
else:
message = f'Field `tool.hatch.build.targets.{self.plugin_name}.options` must be a table'
raise TypeError(message)

return self.__options

@property
def build_targets(self) -> dict[str, dict[str, Any]]:
"""
Allows specifying multiple build targets, each with its own options/environment variables.

This extends the previously non-customizable script build targets by full control over what is built.
"""
if self.__build_targets is None:
build_targets = self.target_config.get('build-targets')

if not build_targets: # None or empty table
# Fill in the default build targets.
# First check the scripts section, if it is empty, fall-back to the default build target.
if self.scripts:
self.__build_targets = {
script: {
'exe_stem': f'{script}-{{version}}', # version will be interpolated later
'options': {'exec-spec': self.builder.metadata.core.scripts[script]},
}
for script in self.scripts
}
else: # the default if nothing is defined
self.__build_targets = {
'default': {
'exe_stem': '{name}-{version}' # name & version will be interpolated later
}
}
elif isinstance(build_targets, dict):
self.__build_targets = build_targets
else:
message = f'Field `tool.hatch.build.targets.{self.plugin_name}.build_targets` must be a table'
raise TypeError(message)

return self.__build_targets


class BinaryBuilder(BuilderInterface):
"""
Expand Down Expand Up @@ -111,74 +164,66 @@ def build_bootstrap(
import shutil
import tempfile

cargo_path = os.environ.get('CARGO', '')
if not cargo_path:
if not shutil.which('cargo'):
message = 'Executable `cargo` could not be found on PATH'
raise OSError(message)

cargo_path = 'cargo'

app_dir = os.path.join(directory, self.PLUGIN_NAME)
if not os.path.isdir(app_dir):
os.makedirs(app_dir)

on_windows = sys.platform == 'win32'
base_env = dict(os.environ)
base_env['PYAPP_PROJECT_NAME'] = self.metadata.name
base_env['PYAPP_PROJECT_VERSION'] = self.metadata.version

if self.config.python_version:
base_env['PYAPP_PYTHON_VERSION'] = self.config.python_version
for build_target_name, build_target_spec in self.config.build_targets.items():
options = { # merge options from the parent options table and the build target options table
**options_to_env_vars(self.config.options),
**options_to_env_vars(build_target_spec.get('options', {})),
}

with EnvVars(options):
cargo_path = os.environ.get('CARGO', '')
if not cargo_path:
if not shutil.which('cargo'):
message = 'Executable `cargo` could not be found on PATH'
raise OSError(message)

cargo_path = 'cargo'

on_windows = sys.platform == 'win32'
base_env = dict(os.environ)
base_env['PYAPP_PROJECT_NAME'] = self.metadata.name
Copy link

@mmorys mmorys Aug 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I have tested the proposed changes and this does not fix the issue defined in #1436. The project is not actually installed within the PyApp binary, resulting in a No matching distribution error.

According to the PyApp Docs there are 3 ways to define the project path. Here, PYAPP_PROJECT_NAME and PYAPP_PROJECT_VERSION are defined, meaning you are using the Identifier configuration which means "the package will be installed from a package index like PyPI". This is not what we want, as the package is local and not necessarily on PyPi.

Rather than defining these two variables here, the builder should utilize the Embedding config method:

  • Build an sdist or wheel in a temporary directory
  • Set PYAPP_PROJECT_PATH to point to this file
    • Name and version are automatically derived from the sdist/wheel metadata.

See my comment on the related issue for how I got around this issue by building and pointing to an sdist.

base_env['PYAPP_PROJECT_VERSION'] = self.metadata.version

if self.config.python_version:
base_env['PYAPP_PYTHON_VERSION'] = self.config.python_version

# https://doc.rust-lang.org/cargo/reference/config.html#buildtarget
build_target = os.environ.get('CARGO_BUILD_TARGET', '')

# This will determine whether we install from crates.io or build locally and is currently required for
# cross compilation: https://github.com/cross-rs/cross/issues/1215
repo_path = os.environ.get('PYAPP_REPO', '')

with tempfile.TemporaryDirectory() as temp_dir:
exe_name = 'pyapp.exe' if on_windows else 'pyapp'
if repo_path:
context_dir = repo_path
target_dir = os.path.join(temp_dir, 'build')
if build_target:
temp_exe_path = os.path.join(target_dir, build_target, 'release', exe_name)
else:
temp_exe_path = os.path.join(target_dir, 'release', exe_name)
install_command = [cargo_path, 'build', '--release', '--target-dir', target_dir]
else:
context_dir = temp_dir
temp_exe_path = os.path.join(temp_dir, 'bin', exe_name)
install_command = [cargo_path, 'install', 'pyapp', '--force', '--root', temp_dir]
if self.config.pyapp_version:
install_command.extend(['--version', self.config.pyapp_version])

self.cargo_build(install_command, cwd=context_dir, env=base_env)

exe_stem_template = build_target_spec.get('exe_stem', build_target_name)
exe_stem = exe_stem_template.format(name=self.metadata.name, version=self.metadata.version)
if build_target:
exe_stem = f'{exe_stem}-{build_target}'

# https://doc.rust-lang.org/cargo/reference/config.html#buildtarget
build_target = os.environ.get('CARGO_BUILD_TARGET', '')

# This will determine whether we install from crates.io or build locally and is currently required for
# cross compilation: https://github.com/cross-rs/cross/issues/1215
repo_path = os.environ.get('PYAPP_REPO', '')

with tempfile.TemporaryDirectory() as temp_dir:
exe_name = 'pyapp.exe' if on_windows else 'pyapp'
if repo_path:
context_dir = repo_path
target_dir = os.path.join(temp_dir, 'build')
if build_target:
temp_exe_path = os.path.join(target_dir, build_target, 'release', exe_name)
else:
temp_exe_path = os.path.join(target_dir, 'release', exe_name)
install_command = [cargo_path, 'build', '--release', '--target-dir', target_dir]
else:
context_dir = temp_dir
temp_exe_path = os.path.join(temp_dir, 'bin', exe_name)
install_command = [cargo_path, 'install', 'pyapp', '--force', '--root', temp_dir]
if self.config.pyapp_version:
install_command.extend(['--version', self.config.pyapp_version])

if self.config.scripts:
for script in self.config.scripts:
env = dict(base_env)
env['PYAPP_EXEC_SPEC'] = self.metadata.core.scripts[script]

self.cargo_build(install_command, cwd=context_dir, env=env)

exe_stem = (
f'{script}-{self.metadata.version}-{build_target}'
if build_target
else f'{script}-{self.metadata.version}'
)
exe_path = os.path.join(app_dir, f'{exe_stem}.exe' if on_windows else exe_stem)
shutil.move(temp_exe_path, exe_path)
else:
self.cargo_build(install_command, cwd=context_dir, env=base_env)

exe_stem = (
f'{self.metadata.name}-{self.metadata.version}-{build_target}'
if build_target
else f'{self.metadata.name}-{self.metadata.version}'
)
exe_path = os.path.join(app_dir, f'{exe_stem}.exe' if on_windows else exe_stem)
shutil.move(temp_exe_path, exe_path)

return app_dir

Expand All @@ -197,3 +242,22 @@ def cargo_build(self, *args: Any, **kwargs: Any) -> None:
@classmethod
def get_config_class(cls) -> type[BinaryBuilderConfig]:
return BinaryBuilderConfig


def options_to_env_vars(options: dict[str, str]) -> dict[str, str]:
"""
Converts a dictionary of options to environment variables by uppercasing the keys and replacing dashes with
underscores.
Keys that no start with "CARGO_" will be prefixed with "PYAPP_".

Examples:

{'full-isolation': 'true', 'cargo-target-dir': 'tmp'} --> {'PYAPP_FULL_ISOLATION': 'true', 'CARGO_TARGET_DIR': 'tmp'}
"""
env_vars = {}
for key, value in options.items():
sluggified_key = key.replace('-', '_').upper()
if not sluggified_key.startswith('CARGO_'):
sluggified_key = f'PYAPP_{sluggified_key}'
env_vars[sluggified_key] = str(value)
return env_vars
65 changes: 65 additions & 0 deletions tests/backend/builders/test_binary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,3 +727,68 @@ def test_legacy(self, hatch, temp_dir, mocker):
assert len(build_artifacts) == 1
assert expected_artifact == str(build_artifacts[0])
assert (build_path / 'app' / ('my-app-0.1.0.exe' if sys.platform == 'win32' else 'my-app-0.1.0')).is_file()

def test_custom_build_targets(self, hatch, temp_dir, mocker):
subprocess_run = mocker.patch('subprocess.run', side_effect=cargo_install)

project_name = 'My.App'

with temp_dir.as_cwd():
result = hatch('new', project_name)

assert result.exit_code == 0, result.output

project_path = temp_dir / 'my-app'
config = {
'project': {'name': project_name, 'version': '0.1.0'},
'tool': {
'hatch': {
'build': {
'targets': {
'binary': {
'versions': ['bootstrap'],
'options': {
'distibution-embed': 'true',
'pip-extra-index-args': '--index-url foobar',
'cargo-target-dir': (project_path / 'pyapp_cargo').as_posix(),
},
'build-targets': {
'myapp-gui': {
'exe_stem': '{name}-{version}-gui',
'options': {'is-gui': 'true', 'exec-module': 'myapp'},
},
},
}
}
},
},
},
}
builder = BinaryBuilder(str(project_path), config=config)

build_path = project_path / 'dist'

with project_path.as_cwd():
artifacts = list(builder.build())

subprocess_run.assert_called_once_with(
['cargo', 'install', 'pyapp', '--force', '--root', mocker.ANY],
cwd=mocker.ANY,
env=ExpectedEnvVars({
'CARGO_TARGET_DIR': (temp_dir / 'my-app/pyapp_cargo').as_posix(),
'PYAPP_DISTIBUTION_EMBED': 'true',
'PYAPP_EXEC_MODULE': 'myapp',
'PYAPP_IS_GUI': 'true',
'PYAPP_PIP_EXTRA_INDEX_ARGS': '--index-url foobar',
}),
)

assert len(artifacts) == 1
expected_artifact = artifacts[0]

build_artifacts = list(build_path.iterdir())
assert len(build_artifacts) == 1
assert expected_artifact == str(build_artifacts[0])
assert (
build_path / 'binary' / ('my-app-0.1.0-gui.exe' if sys.platform == 'win32' else 'my-app-0.1.0-gui')
).is_file()