Skip to content

"files" is wrong when installed without "wheel" #115

Description

@jaraco

In GitLab by @blueyed on Mar 17, 2020, 12:32

When "wheel" is not installed/used, installing a package will not contain "RECORD", and it falls back to reading "SOURCES.txt" (https://gitlab.com/python-devs/importlib_metadata/blob/3150ed4da9e1267d0787c6f4c1f8258a26a1dd93/importlib_metadata/__init__.py#L270), which then might result in paths not being locate()able, when a src-based setup is used:

setup(
    ...
    packages=find_packages('src'),
    package_dir={'': 'src'},
)

This is similar to https://gitlab.com/python-devs/importlib_metadata/-/issues/112, but in this case here it could use installed-files.txt from the egg-info.

Activity

  1. jaraco commented on Oct 22, 2020

    @jaraco
    MemberAuthor

    In GitLab by @blueyed on Mar 17, 2020, 12:34

    mentioned in commit blueyed/importlib_metadata@5d73789b0619d5ac53dfb2966902c13b24d31e2b

  2. jaraco commented on Oct 22, 2020

    @jaraco
    MemberAuthor

    In GitLab by @blueyed on Mar 17, 2020, 12:34

    mentioned in merge request !114

  3. jaraco commented on Oct 22, 2020

    @jaraco
    MemberAuthor

    In GitLab by @jaraco on Mar 25, 2020, 15:55

    Before adding support for installed-files.txt, I'd like to know what command generates that file. It looks like it's generated in the legacy install.

    I'm leaning toward saying that importlib_metadata shouldn't support this use-case, that if someone wants to have metadata support, they should use the non-legacy features of the packaging ecosystem.

    What is the impact of this issue? Are there real-world use-cases that experience issues as a result of this non-support?

  4. jaraco commented on Oct 22, 2020

    @jaraco
    MemberAuthor

    In GitLab by @jaraco on Jun 5, 2020, 15:37

    Bump.

  5. jaraco commented on Oct 22, 2020

    @jaraco
    MemberAuthor

    In GitLab by @jaraco on Sep 23, 2020, 13:41

    Happy to revisit when there's additional information available, but for now, I'm declaring "won't fix" without prejudice.

  6. jherland commented on Mar 9, 2023

    @jherland
    Contributor

    I've recently run into this issue with the Qiskit (meta)-package:

    • The package does not actually contain any (runtime) Python code itself, but serves as a mechanism to install its transitive dependencies (which populate the qiskit package namespace).
    • The package is distributed as a source archive.
    • When installed as an egg, it provides a SOURCES.txt which is incorrect from a runtime POV: it references 3 .py files, a setup.py and two files under test/, none of which are actually installed.
    • The packages includes a top_level.txt which is empty (contains a single newline). This is arguably correct given that it does not directly install any importable packages/modules.

    IMHO, there are two issues with how importlib_metadata handles this package:

    1. Because SOURCES.txt does not reflect the installed files, importlib_metadata.files("qiskit") ends up returning paths that don't actually exist on disk. According to setuptools docs, SOURCES.txt should not be relied upon at runtime (although I understand that it is likely used as a last resort in this case).
    2. packages_distributions() returns nonsense for this package: _top_level_declared() does not differentiate between an empty top_level.txt and a missing top_level.txt, which triggers a fallback to _top_level_inferred(). This fallback ultimately ends up looking for *.py paths in SOURCES.txt, which finds the three non-existing paths mentioned above. The end result is that packages_distributions() associates the import names setup and test with this package, neither of which exist, or make any sense.

    Some possible ways to fix this:

    • For (1), try installed-files.txt before falling back to SOURCES.txt. This file is written by pip on installation, and is a more accurate source than SOURCES.txt. (If installed-files.txt does not exist for any reason, then fall back to SOURCES.txt as last resort.)
    • For (1), if/when we need to use SOURCES.txt, then at least make sure that only the paths in SOURCES.txt that actually remain after installation are returned.
    • For (2), consider whether it's worthwhile to treat an empty top_level.txt different from a missing top_level.txt. In the above case the empty top_level.txt correctly communicates that this packages does not expose any importable names. Having said that, there might be good reasons for the current behavior.
  7. reopened this on Mar 9, 2023
  8. jaraco commented on Mar 9, 2023

    @jaraco
    MemberAuthor

    When installed as an egg,

    I believe the issue may be rooted in how qiskit is installed.

    The preferred way to install Python packages is as a wheel using dist-info metadata. As eggs are deprecated and superseded by wheels, support for egg-info is incomplete and best-effort.

    Consider this example where I've installed qiskit from the source archive as a wheel and am able to avoid the undesirable behavior:

     ~ $ pip-run --no-deps --no-binary qiskit --no-cache-dir --use-feature no-binary-enable-wheel-cache --use-pep517 qiskit
    Python 3.11.1 (main, Dec 23 2022, 09:28:24) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import importlib.metadata
    >>> 'test' in importlib.metadata.packages_distributions()
    False
    >>> importlib.metadata.files('qiskit')
    [PackagePath('qiskit-0.41.1.dist-info/AUTHORS'), PackagePath('qiskit-0.41.1.dist-info/INSTALLER'), PackagePath('qiskit-0.41.1.dist-info/LICENSE.txt'), PackagePath('qiskit-0.41.1.dist-info/METADATA'), PackagePath('qiskit-0.41.1.dist-info/RECORD'), PackagePath('qiskit-0.41.1.dist-info/REQUESTED'), PackagePath('qiskit-0.41.1.dist-info/WHEEL'), PackagePath('qiskit-0.41.1.dist-info/top_level.txt')]
    

    The key here was to pass --use-pep517, which caused the source archive to build as a wheel and produce dist-info metadata. Other parameters to pip install (via pip run) are there to force the source dist install and bypass other warnings.

    This approach utilizes the preferred code path for metadata and avoids the undesirable behavior.

    Honestly, if it weren't for the fact that editable installs still rely heavily on egg-info metadata, I'd deprecate that format outright.

    When I wrote files() to resolve from SOURCES.txt, I did that knowing it was an imperfect approximation for editable installs. I was unaware that installed-files was also a thing :(.

    I'm not confident that installed-files.txt is properly supported either. I see now there was some discussion some years ago in pypa/setuptools#371 about installed-files.txt. Ideally, I'd like to rally around supported standards and not add support for inferred interfaces.

    Does the --use-pep517 method of installation provide a suitable approach for your use-case?

  9. jherland commented on Mar 9, 2023

    @jherland
    Contributor

    Yes, installing it with dist-info metadata (either using --use-pep517 or by installing with modern pip in a venv that already has wheel available) does solve the issue (*.dist-info/RECORD is obviously more accurate than *.egg-info/SOURCES.txt).

    I 100% agree on using modern packaging practices. However, in this case, I'm not in the position to control the package installation process: I'm working on a tool to find undeclared and unused dependencies in the user's project, and as such I'm using importlib_metadata inside the user's existing venv, and I do not control which packages are installed, or how they were installed.

    installed-files.txt seems to have been written by pip since version 0.3 (Jan 2009), and even if it has not been adopted by the rest of the packaging ecosystem, I think the current prevalence of pip makes installed-files.txt useful as a better source than SOURCES.txt when it exists. When it does not exist (e.g. it seems to not be created in the case of editable installs), falling back to SOURCES.txt is still available as a final - albeit imperfect - resort).

    Do you foresee problems with any of the three fixes I outlined above? In any case, I'm happy to whip up a PR.

  10. jaraco commented on Mar 9, 2023

    @jaraco
    MemberAuthor

    All good points. Yes, I think I agree with the recommendation, and I'm delighted to hear you'd like to work on an implementation. Crucial will be to have tests that capture the relevant factors and expectations (including which scenarios apply to which logic branches). I'm slightly tempted to deprecate the use of SOURCES.txt altogether, but that probably should happen separately. Feel free to take a stab at it and let me know if you have any questions. I can be reached in Gitter (except I haven't enabled this project) and Discord.

  11. FFY00 commented on Mar 10, 2023

    @FFY00
    Member

    I recently touched this code and also think looking at installed-files.txt when available is a good approach 👍

  12. added
    enhancementNew feature or request
    and removed on Apr 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions