takluyver / pynsist

Build Windows installers for Python applications
https://pynsist.readthedocs.io/
Other
896 stars 119 forks source link

Copy package metadata from wheels and from modules #161

Closed adferrand closed 6 years ago

adferrand commented 6 years ago

Following #160.

This PR modify the wheel and modules copy process to ensure that the package metadata (either .dist-info or .egg-info) are inserted in the pkgs directory before the installer is built.

This allows to use the plugins discovery mechanism inside a python application installed through the generated NSIS installer.

Regards, Adrien Ferrand

takluyver commented 6 years ago

Thanks! I'd actually prefer to do this only from wheels, though, and leave it out for packages found by their import name. Sorry to tell you this after you've implemented that; allow me to explain a bit more.

pkg_resources.get_distribution() expects a distribution name, not a module name - they're the same for many packages, but definitely not for all (e.g. pyzmq is imported as zmq). So trying to use the import name as a distribution name will lead to confusing, inconsistent behaviour. And even if we did implement correctly finding the distribution metadata, it may not accurately describe the current code (e.g. if you have an editable install).

Conceptually, this makes sense: a wheel is a distribution, containing one or more packages and some metadata. If you specify an importable name, you'll get just the importable package, not a distribution. Specifying packages with importable names is largely a legacy option (except for including the app's own code), and I encourage people to use pypi_wheels wherever possible.

takluyver commented 6 years ago

As so often, I thought of a more concise summary just after posting my comment:

The .dist-info metadata is a property of a distribution, such as a wheel, not an importable module. Most distributions consist of one top-level module named after the distribution, but enough don't that I would rather not assume it.

adferrand commented 6 years ago

In fact I tested the situation for an editable install, metedata are corrected resolved from the source code, and will create the correct folder.

But your are conceptually right, one can not expect to have a fully instantiated distribution from a unique module import, and using the plugins discovery mechanism when you explicitly import modules is overkill.

I will update the PR accordingly.

adferrand commented 6 years ago

By the way your project itself has its distribution named pynsist, but the module included in it is nsist ^^

takluyver commented 6 years ago

Thanks!

Yup, I was considering using Pynsist as an example, but it's not something you'd normally bundle into an application. :-)

I was a bit too brief about editable installs. The issue is that the metadata is generated when you make the editable install, and if you later update the code (e.g. git pull), the metadata is not regenerated. So you could have metadata saying it's version 4, but the code is actually version 5. Though that's probably overshadowed by the possibility of bundling up a development version which doesn't correspond to any release, which is a general problem of copying in packages found in your development environment.

adferrand commented 6 years ago

By the way, about including development modules.

I was trying to include the module that corresponds to the application itself, what you specify in the entry_point, as a wheel by compiling it first before invoking pynsist. I found this more elegant to have everything packaged the same way, the application and its dependencies, and do not use at all the import module mechanism in pynsist.

But in this case pynsist will fails, as it will try to import the module specified in entry_point from the python environment.

However at runtime it would work, as the module would be installed from the wheel, its path would be added to sys.path, and so the module would be found.

As a workaround I was thinking to use script instead of an entry_point, and make this script able to load from pkgs folder (basically, the same code that the generated script one when entry_point is used). Is it the best way to achieve this?

takluyver commented 6 years ago

Hmm, can you open a new issue for that (with whatever error message you get)?

The mechanism to automatically include the package for entry points was added before it was possible to specify wheels. I think it still makes sense in many cases, but I can see that it might make sense to get the application from a wheel instead.

adferrand commented 6 years ago

Nevermind, I was certainly doing something wrong: I tried with a simple project to test. In this case, pynsist was able to find the entrypoint from the packages exposed in the included wheels.

I updated my PR to integrate package metadata only from wheels.

takluyver commented 6 years ago

I'd love to have a simple check for this in the existing nsist.test_pypi.test_download. I know that doing so will produce a merge conflict, unfortunately, but we can resolve that.

takluyver commented 6 years ago

Thanks, that's great. And it doesn't even seem to have produced a merge conflict. :-)

takluyver commented 6 years ago

I've done a few bits with Pynsist today, with the aim of making a 2.2 release soon. So it shouldn't be too long before this is in a release.

adferrand commented 6 years ago

Great !