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

Expose interface to locate a physical .dist-info / .egg-info directory by distribution name #111

Closed
jaraco opened this issue Oct 22, 2020 · 9 comments

Comments

@jaraco
Copy link
Member

jaraco commented Oct 22, 2020

In GitLab by @uranusjr on Feb 17, 2020, 20:29

#23 discussed whether it’d be possible to make the distribution’s _path attribute public. While I understand the rational behind not exposing it, IMO there are cases when a tool wants to find a distribution on disk. One example would be project tools wanting to perform a “clean” operation that cleans up previously-built package metadata.

Under those circumstances, the need is not to find a Distribution, and then locate it on disk (this does not make sense), but to find a dist/egg-info directory on disk (and then turn it into a Distributon; this is already possible with #90). One way to do that would be to add an interface to do something like

def locate_dist_dir(name):
    for path in MetadataPathFinder._search_paths(sys.path):
        if Distribution.at(path).metadata['Name'] == name:
            yield path
@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jaraco on Mar 9, 2020, 22:34

Yes, I'd like to expose this information. The real trick is describing for other loaders what to present (if anything) for this property.

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

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

Note that using locate() on files is public API already, and might be usable for @uranusjr ?

I also wanted to mention that with https://gitlab.com/python-devs/importlib_metadata/-/merge_requests/114 I've added a new _locate_path, which gets adjusted for src-based egg-info distributions.
I.e. I've thought that _path is good to point to the egg-info still, while it has extra information then about how to locate files (used by locate() then).

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @uranusjr on Mar 28, 2020, 14:24

@blueyed Not sure I get what you mean, the distribution itself is not included in the list returned by file, which is the only API I am aware of that returns PackagePath.

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jaraco on Jun 14, 2020, 16:25

@uranusjr What would you expect locate_dist_dir to return for a dist that's persisted in an abstract store not on the file system (think SQL database)?

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @uranusjr on Jun 14, 2020, 21:27

Either return None or raise something like FileNotFoundError, since that is not a .dist-info directory.

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @jaraco on Sep 23, 2020, 15:53

the need is not to find a Distribution, and then locate it on disk (this does not make sense), but to find a dist/egg-info directory on disk (and then turn it into a Distributon

I'm afraid I still don't understand the distinction between these two options.

I think what you want here is something like:

def locate_dist_dir(name):
    dist, = metadata.Distribution.discover(name=name)
    return dist._path if isinstance(getattr(dist, '_path', None), metadata.pathlib.Path) else None

Perhaps Distribution could get a get_local_dir() implementing that last line, such that one could:

path = one(metadata.Distribution.discover(name=name)).get_local_dir()

(uses one)

WDYT?

@jaraco
Copy link
Member Author

jaraco commented Oct 22, 2020

In GitLab by @uranusjr on Sep 23, 2020, 16:34

The impression I got from from !23 was that exposing _path was considered not a good idea since not all Distribution has a path, so my proposal was to avoid involving Distribution classes at all when locating the metadata directory, and provide a way turn that path into a Distribution object afterwards. The isinstance() and getattr() introspection just feels wrong.

A get_local_dir() method would make sense as well, though. I would be perfectly happy if that’s the interface we go with.

@jaraco
Copy link
Member Author

jaraco commented Jan 10, 2021

Under those circumstances, the need is not to find a Distribution, and then locate it on disk (this does not make sense), but to find a dist/egg-info directory on disk (and then turn it into a Distributon; this is already possible with #90). One way to do that would be to add an interface to do something like

def locate_dist_dir(name):
    for path in MetadataPathFinder._search_paths(sys.path):
        if Distribution.at(path).metadata['Name'] == name:
            yield path

Looking at this again, I'm pretty sure _search_paths already does what you need:

locate_dist_dir = functools.partial(MetadataPathFinder._search_paths(paths=sys.path)

I think we have two options.

  • add a test to protect the interface of the private _search_paths for select uses.
  • expose a new, supported interface.

I think I'll go for option 1 for now, for pip's purposes, until something more is needed.

@jaraco
Copy link
Member Author

jaraco commented Jun 8, 2023

I can see that pip never used the _search_paths method.

 pip main $ git log -p | grep _search_paths
 pip main $

I'm removing the protection I added for it.

clrpackages pushed a commit to clearlinux-pkgs/pypi-importlib_metadata that referenced this issue Jun 20, 2023
…6.0 to version 6.7.0

Barry Warsaw (1):
      gh-98040: Backport python/cpython#98059

Jason R. Coombs (21):
      Remove unnecessary and incorrect copyright notice. Fixes jaraco/skeleton#78.
      Replace flake8 with ruff. Fixes jaraco/skeleton#79 and sheds debt.
      👹 Feed the hobgoblins (delint).
      Make substitution fields more prominent and distinct from true 'skeleton' references. (#71)
      Suppress EncodingWarning in build.env. Ref pypa/build#615.
      Remove reference to EncodingWarning as it doesn't exist on some Pythons.
      Inline the NullFinder behavior.
      Add docstring to test_integration to give some context.
      Remove Python 2 compatibility in _compat.NullFinder.
      Move test_interleaved_discovery from test_integration to test_main.
      Remove test_search_dist_dirs as it was never used. Ref python/importlib_metadata#111.
      Extract _topmost and _get_toplevel_name functions.
      Extract _topmost and _get_toplevel_name functions.
      Capture that _get_toplevel_name can return None.
      Streamline the test to check one expectation (the standard dist-info expectation is handled by other tests above.
      Inline the symlink setup.
      Consolidate PackageDistributions tests.
      Update _path to jaraco.path 3.6 with symlink support.
      Utilize the new Symlink in preparing the test case.
      Update changelog.
      Remove '__init__.py', not needed.

Johan Herland (2):
      Add test to demonstrate issue with symlinked packages
      Attempt to fix issue with symlinked packages
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

1 participant