search

beeware / briefcase

Tools to support converting a Python project into a standalone native application.
https://briefcase.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
2.66k stars 372 forks source link

Allow pyproject.toml to reference a requirements file #1275

Open mhsmith opened 1 year ago

mhsmith commented 1 year ago

In the context of the requires_lock feature added by #1209, we had the following conversation:

@mhsmith

How about generalizing this to requires_file, so it can be used for any requirements file, not just lock files? See #1270 for an example of where this would be useful.

@freakboy3742

Agreed that it would be nice to support requirements files in general, as an alternative to pyproject.toml requires definitions. However, I think this is a supplement to, rather than a replacement for requires_lock.

The choice to introduce requires_lock was to allow a project to define loose requirements based on core functionality, but then lock a specific solution to the full package set required at runtime. This makes the maintenance task as an author simpler - just define the high level packages you need, and then periodically generate a new lock of the full solution so that builds are reproducible.

Before we introduced requires_lock, one ideas I had was to allow requires to be either a list or a string; if it's a list, it's a list of packages; if it's a string, it's a path to a file. I can't see any reason we couldn't still do this - requires accepts both list and str forms, and requires_lock defines a locking file. This would allow for requires to continue to be an additive setting (so you can define global, macOS and Windows requirements files) but requires_lock is a complete solution file.

An alternative to having 2 type alternatives for requires would be to add requires_file; however, I'm not 100% about the ergonomics of having 1 setting with 2 possible types, vs having 2 settings and then needing to document and implement error handling (or at least a resolution order) around projects that define both.

@mhsmith

I think it's better to have two separate settings. Not only would that be more explicit and less error-prone, it could also be useful to define both requirements files and explicit requirements, especially when adding a BeeWare app to an established project. Such projects might also find it useful if the requires_file setting accepted a list of multiple files.

But I agree it's better for this PR to only deal with the locking issue: I misunderstood the title and thought it had a larger scope.

@freakboy3742

To clarify - are you suggestion that we support both settings, and the final list of installed packages is the union of requires and requires_file, merged across all levels (app, platform, format etc)? I was originally thinking it would require picking one to have primacy (or, alternatively, raise an error if both are defined); supporting both is an interesting option I hadn't considered.

@mhsmith

Yes, and I'm basing this on the experience I had developing the Android app for Electron Cash. That project has a core Python library with its own requirements file, and three apps using the library (PyQt, iOS and Android). So the app needs to use the library's requirements file, and then add some requirements of its own, either directly or via a second requirements file. I think this would be a fairly common scenario.

freakboy3742 commented 1 year ago

I'm not sure this is a completely disjunct feature from #1209. In order to implement this, we'll need to introduce the ability to include requirements files in direct pip installs (i.e., when Briefcase invokes pip), and merge requirements files for indirect pip installs (e.g., where Gradle or Docker invokes pip). That use case is almost identical to the locking case (or, at least, the pieces flagged as missing from #1270) - except that the existence of a lock file will supercede the per-platform, per-format requirements list and requirements files.

To ensure we're all on the same page: consider the following config file:

[tool.briefcase.app.myapp]
requires = ['a', 'b']
requires_file = 'requirements-base.txt'

[tool.briefcase.app.myapp.macOS]
requires = ['c', 'd']
requires_file = 'requirements-macOS.txt'

[tool.briefcase.app.myapp.windows]
requires = ['c', 'e']
requires_file = 'requirements-windows.txt'
requires_lock = 'requirements-windows.lock'

In this situation, the macOS app would get a, b, c, d, plus all the requirements listed in requirements-base.txt and requirements-macOS.txt. The Windows app would get a, b, c, e, plus all the requirements listed in requirements-base.txt and requirements-windows.txt - however, the specific solution resulting from those requirements would be written into requirements-windows.lock, and it is the full solution baked into that file that would be installed into the Windows app.

Analogous logic would also be used for test_requires, test_requires_file and test_requires_lock.

Have I missed anything?

mhsmith commented 1 year ago

That all sounds good, but for the reasons in my comment above, I think it would be better to makerequires_file a list, and call it requires_files instead. That would be consistent with other list settings like requires and sources, which, if they appear in multiple sections, are concatenated and not overridden.

The easiest way to implement this might be to take the requirements file generator currently used for Android and Flatpak, and extend that to all platforms. That is, every platform would first create a single merged requirements file, and then pass that to pip. Materializing the merge output in this way would also simplify testing and debugging.

freakboy3742 commented 1 year ago

Hrm - I can see what you're saying about consistency; my concern is that it's slightly contrary to user expectation. Generally, you have a single requirements file. We're already bending that a little bit by having macOS specific requirements files; I'm not sure how I feel about the optics of encouraging/expecting users to have multiple "core" requirements files.

That said, making requires_file a single string would be more work, as it's something we'd need to read as a single string, then turn into a list...

Another option would be to not make requires_file a cumulative setting, and rely on the fact that a requirements file can include -r base.txt as a directive. That allows the end user to define their own "cumulative" requirement definition using the native features of pip and requirements files.