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

Link is not defined if not TYPE_CHECKING but it's used #2151

Closed
alkuzad opened this issue Jul 31, 2023 · 5 comments · Fixed by j178/pdm#1
Closed

Link is not defined if not TYPE_CHECKING but it's used #2151

alkuzad opened this issue Jul 31, 2023 · 5 comments · Fixed by j178/pdm#1
Labels
🐛 bug Something isn't working

Comments

@alkuzad
Copy link

alkuzad commented Jul 31, 2023

  • [*] I have searched the issue tracker and believe that this is not a duplicate.

Make sure you run commands with -v flag before pasting the output.

Steps to reproduce

  1. install 2.8.2
  2. Init project
  3. pdm add --dev black
  4. Error happens:

File "/nix/store/5a457g4zr7q11b4mz0ifc9z88kwcp8g7-python3-3.10.12/lib/python3.10/concurrent/futures/thread.py", line 58, in run
result = self.fn(*self.args, **self.kwargs)
File "/home/dgoslawski/.local/pipx/venvs/pdm/lib/python3.10/site-packages/pdm/installers/synchronizers.py", line 277, in install_candidate
self.manager.install(can)
File "/home/dgoslawski/.local/pipx/venvs/pdm/lib/python3.10/site-packages/pdm/installers/manager.py", line 33, in install
installer(str(prepared.build()), self.environment, prepared.direct_url())
File "/home/dgoslawski/.local/pipx/venvs/pdm/lib/python3.10/site-packages/pdm/models/candidates.py", line 379, in build
self.obtain(allow_all=False)
File "/home/dgoslawski/.local/pipx/venvs/pdm/lib/python3.10/site-packages/pdm/models/candidates.py", line 413, in obtain
self.link = _find_best_match_link(
File "/home/dgoslawski/.local/pipx/venvs/pdm/lib/python3.10/site-packages/pdm/models/candidates.py", line 88, in _find_best_match_link
links = [Link(f["url"]) for f in files if "url" in f]
File "/home/dgoslawski/.local/pipx/venvs/pdm/lib/python3.10/site-packages/pdm/models/candidates.py", line 88, in
links = [Link(f["url"]) for f in files if "url" in f]
NameError: name 'Link' is not defined

Actual behavior

pdm refuses to install. Installs when candidates.py is modified to move from unearth import Link, Package, PackageFinder outside the TYPE_CHECKING condition

Expected behavior

pdm installs

Environment Information

# Paste the output of `pdm info && pdm info --env` below:
λ pdm info && pdm info --env
PDM version:
  2.8.2
Python Interpreter:
  /home/dgoslawski/projects/pre-commit-semver/.venv/bin/python (3.10)
Project Root:
  /home/dgoslawski/projects/pre-commit-semver
Local Packages:

{
  "implementation_name": "cpython",
  "implementation_version": "3.10.7",
  "os_name": "posix",
  "platform_machine": "x86_64",
  "platform_release": "5.15.90.1-microsoft-standard-WSL2",
  "platform_system": "Linux",
  "platform_version": "#1 SMP Fri Jan 27 02:56:13 UTC 2023",
  "python_full_version": "3.10.7",
  "platform_python_implementation": "CPython",
  "python_version": "3.10",
  "sys_platform": "linux"
}
@alkuzad alkuzad added the 🐛 bug Something isn't working label Jul 31, 2023
@frostming
Copy link
Collaborator

Can you submit a fix?

@alkuzad
Copy link
Author

alkuzad commented Aug 1, 2023

Can you submit a fix?

Technically yes but I do not understand the details of the problem :) Why it is inside condition in the first place? This code seems preety common, am I only one getting this error, maybe due to some nix/pdx shenaningans?

@frostming
Copy link
Collaborator

frostming commented Aug 1, 2023

Can you submit a fix?

Technically yes but I do not understand the details of the problem :) Why it is inside condition in the first place? This code seems preety common, am I only one getting this error, maybe due to some nix/pdx shenaningans?

You already found the cause, didn't you?

What you need to do is add a line in this function to import Link from unearth:

def _find_best_match_link(
finder: PackageFinder,
req: Requirement,
files: list[FileHash],
ignore_compatibility: bool = False,
) -> Link | None:
"""Get the best matching link for a requirement"""
# This function is called when a lock file candidate is given or incompatible wheel
# In this case, the requirement must be pinned, so no need to pass allow_prereleases
# If links are not empty, find the best match from the links, otherwise find from
# the package sources.
links = [Link(f["url"]) for f in files if "url" in f]
hashes = convert_hashes(files)

Others don't encounter this because they never enter the loop, because url field is missing, which is the default behavior. Your lockfile is generated with --static-urls option, right?

@alkuzad
Copy link
Author

alkuzad commented Aug 1, 2023

yes it is using static_links because that is the easiest method to enforce the internal packages to be resolved from internal PyPi and others from public one without using the --extra-index-url shenanigans that are a vulnerability.

I wouldn't add import inside a function, it is a correct syntax-wise but imports inside functions seems hacky. Can we move it just 2 lines above to skip the type checking condition?

@frostming
Copy link
Collaborator

I wouldn't add import inside a function, it is a correct syntax-wise but imports inside functions seems hacky. Can we move it just 2 lines above to skip the type checking condition?

No, that is an intended change, because unearth is a heavy module and importing it will significantly affect the startup time of CLI.

tgolsson added a commit to EmbarkStudios/pdm-plugin-torch that referenced this issue Nov 14, 2023
A bit frustrating with the level of hacking we do to piggyback on top of
PDM internals. Ending up also removing PDM 2.8 due to bug,
pdm-project/pdm#2151, which was only fixed in
2.9. Apart from that all versions seem to work now.

Will put up a PR with new CI image elsewhere.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants