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

Better PyApp integration #1423

Open
johannesloibl opened this issue Apr 24, 2024 · 18 comments
Open

Better PyApp integration #1423

johannesloibl opened this issue Apr 24, 2024 · 18 comments

Comments

@johannesloibl
Copy link

johannesloibl commented Apr 24, 2024

Hey, me again ;)

I'd like to request (and maybe even contribute) more integration options for PyApp.
Hatch right now only offers the scripts option, which is transforming your project scripts into a bunch of executables. While this may work for some easy use cases, it has following disadvantages:

  • Only [project.scripts] is taken into account and [project.gui-scripts] is ignored.
    So if your project needs to build both, it is hard to configure different PyApp environment variables for both.
  • No control over the naming of built executables. Right now the version number is appended and the script name is prepended. I'd like to make it template-able, something like mycustomname, mycustomname-{version} or {dist_name}-{version}.
  • __main__.py cannot be built if script is left empty. Also other options PyApp provides (PYAPP_EXEC_*) are not supported.

Here an example TOML i could imagine:

[tool.hatch.build.targets.binary]

[tool.hatch.build.targets.binary.variables]
CARGO_TARGET_DIR = ".tmp/pyapp_cargo"
PYAPP_DISTRIBUTION_EMBED = "false"
PYAPP_FULL_ISOLATION = "true"
PYAPP_PIP_EXTRA_ARGS = "--index-url ..."
PYAPP_UV_ENABLED = "true"

[tool.hatch.build.targets.binary.build-targets]
# Build the CLI from myapp.cli:cli
myapp-cli = { name = "{dist_name}-cli.exe", variables = {PYAPP_IS_GUI = "false", PYAPP_EXEC_SPEC = "myapp.cli:cli"}}
# Build the GUI from myapp.__main__
myapp-gui = { name = "{dist_name}.exe", variables = {PYAPP_IS_GUI = "true", PYAPP_EXEC_MODULE = "myapp"}}

This would enable all possible options of PyApp within Hatch and integrate it better.
What do you think? If you like it i could contribute this feature.

@johannesloibl
Copy link
Author

@ofek any thoughts on this so I could start implementing?

@ofek
Copy link
Sponsor Collaborator

ofek commented Jun 2, 2024

Hey there, sorry I've been busy! I don't have time today to think about the binary naming section but for the PYAPP_* environment variables please treat these separately as options, strip off the prefix, make lowercase and replace underscores with hyphens so e.g. this:

[tool.hatch.build.targets.binary.variables]
PYAPP_FULL_ISOLATION = "true"

would become

[tool.hatch.build.targets.binary.options]
full-isolation = "true"

edit: then every key you can transform to the equivalent environment variable using the reverse process and you wouldn't have to document each one but rather link out to the other documentation

@johannesloibl
Copy link
Author

then how would i treat other options that are Cargo-related like CARGO_TARGET_DIR?

@ofek
Copy link
Sponsor Collaborator

ofek commented Jun 3, 2024

A variables table like you proposed is fine for arbitrary environment variables.

@johannesloibl
Copy link
Author

johannesloibl commented Jun 3, 2024

So to understand you correctly. Should i split up generic variables and pyapp environment variables in two different sections variables and options?
In my current PR i support both CARGO_* and PYAPP_*, maybe have a look to clarify ;)

@ofek
Copy link
Sponsor Collaborator

ofek commented Jun 3, 2024

I would say two different sections yes but the former can contain arbitrary environment variables and we would recommend in the documentation that for options specific to PyApp use the options table whereas the other table we would just mention that it's possible to set arbitrary environment variables and give a small example.

Basically, although the variables table can support the use case of options we just don't document in the example that it's possible.

@johannesloibl
Copy link
Author

Alrighty, then i'll make this small change.

@johannesloibl
Copy link
Author

I'll name the table env-vars to be consistent with known syntax ok?

@ofek
Copy link
Sponsor Collaborator

ofek commented Jun 3, 2024

Sounds good! That might be useful to have as a fundamental build option in future.

@johannesloibl
Copy link
Author

johannesloibl commented Jun 3, 2024

Should the specific build targets also have a differentiation between options and env-vars?

'binary': {
                                'versions': ['bootstrap'],
                                'options': {
                                    'distibution-embed': 'true',
                                    'pip-extra-index-args': '--index-url foobar',
                                },
                                'env-vars': {
                                    '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'},
                                        'env-vars': {
                                            'CARGO_TARGET_DIR': (project_path / 'other_pyapp_cargo').as_posix(),
                                        }
                                    },
                                },
                            }

and how should the prios be resolved?
target config > target env var > global config > global env var?^^

I start to think that only keeping the env-vars table would be less confusing, since the options table is just for "writing less". What do you think?

@johannesloibl
Copy link
Author

johannesloibl commented Jun 3, 2024

image

would be nicer to read and write actually

@ofek
Copy link
Sponsor Collaborator

ofek commented Jun 3, 2024

  • sounds good to drop the options idea
  • for environment variables let's use the full capitalized names to avoid any confusion then
  • let's get rid of top-level option ideas and put everything under an outputs key which is an array of tables

@johannesloibl
Copy link
Author

so like this?
image

@ofek
Copy link
Sponsor Collaborator

ofek commented Jun 3, 2024

Yes but get rid of the top level environment variables

@johannesloibl
Copy link
Author

Why not keep them to have a place to not repeat stuff? This would be frequently used I think, e.g. to set an index url extra arg for all outputs.

@ofek
Copy link
Sponsor Collaborator

ofek commented Jun 3, 2024

Okay, that's fine 👍

@johannesloibl
Copy link
Author

Implementation and docs are done. Please have a look ;)

@mmorys
Copy link

mmorys commented Aug 27, 2024

Hi @johannesloibl, I posted on a related issue before seeing this one here:
#1436 (comment)

Looks like you have an implementation in the pipeline #1547. I will test to see if this addresses the issue I was having with the project not included in the binary. Posting here to cross-link these related issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants