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

Private repositories broken after bdfdc62 #5476

Closed
3 tasks done
lovesegfault opened this issue Apr 21, 2022 · 13 comments · Fixed by #5484
Closed
3 tasks done

Private repositories broken after bdfdc62 #5476

lovesegfault opened this issue Apr 21, 2022 · 13 comments · Fixed by #5484
Labels
kind/bug Something isn't working as expected

Comments

@lovesegfault
Copy link
Contributor

  • I am on the latest Poetry version.
  • I have searched the issues of this repo and believe that this is not a duplicate.
  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).
  • OS version and name: NixOS
  • Poetry version: dca0c56

Issue

I was trying to bump our internal Poetry from 5900f37 to dca0c56 when I noticed our private repositories stopped working.

Upon attempting poetry lock --no-update, I got the following:

  ValueError

  Package('redacted', '21.11.4+ad7d6ed') is not in list

  at ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/legacy_repository.py:111 in package
      107│         Note that this will be cached so the subsequent operations
      108│         should be much faster.
      109│         """
      110│         try:
    → 111│             index = self._packages.index(Package(name, version, version))
      112│ 
      113│             return self._packages[index]
      114│         except ValueError:
      115│             package = super().package(name, version, extras)

The following error occurred when trying to handle this error:


  Stack trace:

  29  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cleo/application.py:330 in run
       328│ 
       329│             try:
     → 330│                 exit_code = self._run(io)
       331│             except Exception as e:
       332│                 if not self._catch_exceptions:

  28  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/console/application.py:174 in _run
       172│         self._load_plugins(io)
       173│ 
     → 174│         return super()._run(io)
       175│ 
       176│     def _configure_io(self, io: IO) -> None:

  27  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cleo/application.py:425 in _run
       423│                 io.set_input(ArgvInput(argv))
       424│ 
     → 425│         exit_code = self._run_command(command, io)
       426│         self._running_command = None
       427│ 

  26  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cleo/application.py:467 in _run_command
       465│ 
       466│         if error is not None:
     → 467│             raise error
       468│ 
       469│         return event.exit_code

  25  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cleo/application.py:451 in _run_command
       449│ 
       450│             if event.command_should_run():
     → 451│                 exit_code = command.run(io)
       452│             else:
       453│                 exit_code = ConsoleCommandEvent.RETURN_CODE_DISABLED

  24  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cleo/commands/base_command.py:118 in run
       116│         io.input.validate()
       117│ 
     → 118│         status_code = self.execute(io)
       119│ 
       120│         if status_code is None:

  23  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cleo/commands/command.py:85 in execute
        83│ 
        84│         try:
     →  85│             return self.handle()
        86│         except KeyboardInterrupt:
        87│             return 1

  22  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/console/commands/lock.py:55 in handle
        53│         self._installer.lock(update=not self.option("no-update"))
        54│ 
     →  55│         return self._installer.run()
        56│ 

  21  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/installation/installer.py:99 in run
        97│         # Check if refresh
        98│         if not self._update and self._lock and self._locker.is_locked():
     →  99│             return self._do_refresh()
       100│ 
       101│         # Force update if there is no lock file present

  20  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/installation/installer.py:202 in _do_refresh
       200│         )
       201│ 
     → 202│         ops = solver.solve(use_latest=[]).calculate_operations()
       203│ 
       204│         local_repo = Repository()

  19  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/puzzle/solver.py:73 in solve
        71│         with self._provider.progress():
        72│             start = time.time()
     →  73│             packages, depths = self._solve(use_latest=use_latest)
        74│             end = time.time()
        75│ 

  18  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/puzzle/solver.py:132 in _solve
       130│ 
       131│         try:
     → 132│             result = resolve_version(
       133│                 self._package, self._provider, locked=locked, use_latest=use_latest
       134│             )

  17  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/mixology/__init__.py:24 in resolve_version
        22│     solver = VersionSolver(root, provider, locked=locked, use_latest=use_latest)
        23│ 
     →  24│     return solver.solve()
        25│ 

  16  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/mixology/version_solver.py:81 in solve
        79│             while next is not None:
        80│                 self._propagate(next)
     →  81│                 next = self._choose_package_version()
        82│ 
        83│             return self._result()

  15  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/mixology/version_solver.py:394 in _choose_package_version
       392│             package = locked
       393│ 
     → 394│         package = self._provider.complete_package(package)
       395│ 
       396│         conflict = False

  14  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/puzzle/provider.py:448 in complete_package
       446│             package = DependencyPackage(
       447│                 package.dependency,
     → 448│                 self._pool.package(
       449│                     package.name,
       450│                     package.version.text,

  13  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/pool.py:144 in package
       142│             for repo in self._repositories:
       143│                 try:
     → 144│                     package = repo.package(name, version, extras=extras)
       145│                 except PackageNotFound:
       146│                     continue

  12  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/legacy_repository.py:115 in package
       113│             return self._packages[index]
       114│         except ValueError:
     → 115│             package = super().package(name, version, extras)
       116│             package._source_type = "legacy"
       117│             package._source_url = self._url

  11  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/cached.py:80 in package
        78│         extras: (list | None) = None,
        79│     ) -> Package:
     →  80│         return self.get_release_info(name, version).to_package(name=name, extras=extras)
        81│ 

  10  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/cached.py:57 in get_release_info
        55│             return PackageInfo.load(self._get_release_info(name, version))
        56│ 
     →  57│         cached = self._cache.remember_forever(
        58│             f"{name}:{version}", lambda: self._get_release_info(name, version)
        59│         )

   9  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cachy/repository.py:174 in remember_forever
       172│             return val
       173│ 
     → 174│         val = value(callback)
       175│ 
       176│         self.forever(key, val)

   8  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/cachy/helpers.py:6 in value
         4│ def value(val):
         5│     if callable(val):
     →   6│         return val()
         7│ 
         8│     return val

   7  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/cached.py:58 in <lambda>
        56│ 
        57│         cached = self._cache.remember_forever(
     →  58│             f"{name}:{version}", lambda: self._get_release_info(name, version)
        59│         )
        60│ 

   6  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/legacy_repository.py:134 in _get_release_info
       132│             raise PackageNotFound(f'No package named "{name}"')
       133│ 
     → 134│         links = list(page.links_for_version(name, Version.parse(version)))
       135│ 
       136│         return self._links_to_data(

   5  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/link_sources/base.py:88 in links_for_version
        86│ 
        87│         for link in self.links:
     →  88│             pkg = self.link_package_data(link)
        89│ 
        90│             if pkg.name == name and pkg.version and pkg.version == version:

   4  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/repositories/link_sources/base.py:82 in link_package_data
        80│             version = Version.parse(version)
        81│ 
     →  82│         return Package(name, version, source_url=link.url)
        83│ 
        84│     def links_for_version(self, name: str, version: Version) -> Iterator[Link]:

   3  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/core/packages/package.py:75 in __init__
        73│ 
        74│         if not isinstance(version, Version):
     →  75│             self._version = Version.parse(version)
        76│             self._pretty_version = pretty_version or version
        77│         else:

   2  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/core/version/pep440/version.py:143 in parse
       141│         from poetry.core.version.pep440.parser import parse_pep440
       142│ 
     → 143│         return parse_pep440(value, cls)
       144│ 
       145│     def is_prerelease(self) -> bool:

   1  ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/core/version/pep440/parser.py:90 in parse_pep440
        88│     value: str, version_class: Optional[Type["PEP440Version"]] = None
        89│ ) -> "PEP440Version":
     →  90│     return PEP440Parser.parse(value, version_class)
        91│ 

  InvalidVersion

  Invalid PEP 440 version: '21.07.28.5ffb65e2ff8067c732e2b178d03b707c7fb27855'

  at ~/.local/share/pypoetry/venv/lib/python3.8/site-packages/poetry/core/version/pep440/parser.py:69 in parse
       65│         cls, value: str, version_class: Optional[Type["PEP440Version"]] = None
       66│     ) -> "PEP440Version":
       67│         match = cls._regex.search(value) if value else None
       68│         if not match:
    →  69│             raise InvalidVersion(f"Invalid PEP 440 version: '{value}'")
       70│ 
       71│         if version_class is None:
       72│             from poetry.core.version.pep440.version import PEP440Version
       73│ 

I bisected this to commit bdfdc62 by @abn.

I still see this message at the beginning of the lock process:

❯ poetry lock -vvv --no-update
Loading configuration file /home/beme/.config/pypoetry/config.toml
Adding repository qh-py (https://redacted-python.pkg.dev/redacted/redacted/simple)

But it doesn't seem to look in that repository for any packages at all.

@lovesegfault lovesegfault added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels Apr 21, 2022
@lovesegfault
Copy link
Contributor Author

cc. @radoering Who reviewed the relevant PR (#3868)

@radoering
Copy link
Member

I'd suspect the change in links_for_version.

Before:

def links_for_version(self, version: Version) -> Iterator[Link]:
for link in self.links:
if self.link_version(link) == version:
yield link
def link_version(self, link: Link) -> Version | None:
m = wheel_file_re.match(link.filename)
if m:
version = m.group("ver")
else:
info, ext = link.splitext()
match = self.VERSION_REGEX.match(info)
if not match:
return None
version = match.group(2)
try:
version = Version.parse(version)
except ValueError:
return None
return version

After:

def link_package_data(self, link: Link) -> Package:
name, version = None, None
m = wheel_file_re.match(link.filename) or sdist_file_re.match(link.filename)
if m:
name = canonicalize_name(m.group("name"))
version = m.group("ver")
else:
info, ext = link.splitext()
match = self.VERSION_REGEX.match(info)
if match:
version = match.group(2)
with contextlib.suppress(ValueError):
version = Version.parse(version)
return Package(name, version, source_url=link.url)
def links_for_version(self, name: str, version: Version) -> Iterator[Link]:
name = canonicalize_name(name)
for link in self.links:
pkg = self.link_package_data(link)
if pkg.name == name and pkg.version and pkg.version == version:
yield link

@lovesegfault Can you debug and compare what's happening in these lines for your use case?

@lovesegfault
Copy link
Contributor Author

Working on it :)

@lovesegfault
Copy link
Contributor Author

It's interesting, if I print the value of version before this try:

version = match.group(2)
try:
version = Version.parse(version)
except ValueError:
return None
return version

version=1.0.0_prea0c74a61
version=1.0.0_prea0c74a61
version=1.1.0+26d841b7
version=1.1.0+26d841b7
version=1.1.0+89038aff
version=1.1.0+89038aff028b2e9da22978063045ba734265d0bd
version=1.1.0+8cf348fd
version=1.1.0_89038aff
version=1.2.0+f82ec42b
version=1.2.0+f82ec42b

But if I print in the new code before this with:

with contextlib.suppress(ValueError):
version = Version.parse(version)
return Package(name, version, source_url=link.url)

I only see

version=1.0.0_prea0c74a61                                                     
   1: fact: gqec doesn't exist
   1: conflict: gqec doesn't exist                                            
   1: ! gqec (==1.2.0+f82ec42b) is satisfied by gqec (==1.2.0+f82ec42b)
   1: ! which is caused by "demo depends on gqec (1.2.0+f82ec42b)"
   1: ! thus: version solving failed                                                                                                                         
   1: Version solving took 0.318 seconds.                       
   1: Tried 1 solutions.         

It seems like if the version it needs isn't the first one it encounters, then it bails. If I edit pyproject to require that version, it still doesn't work:

Resolving dependencies...                                                     
   1: fact: demo is 0.1.0                                                     
   1: derived: demo                                                           
   1: fact: demo depends on gqec (1.0.0_prea0c74a61)
   1: selecting demo (0.1.0)                                                  
   1: derived: gqec                                                                                                                                          
version=1.0.0_prea0c74a61                                                     
   1: fact: gqec doesn't exist
   1: conflict: gqec doesn't exist                                            
   1: ! gqec is satisfied by gqec                                             
   1: ! which is caused by "demo depends on gqec (1.0.0_prea0c74a61)"
   1: ! thus: version solving failed                                                                                                                         
   1: Version solving took 0.272 seconds.                       
   1: Tried 1 solutions.   

@lovesegfault
Copy link
Contributor Author

I got it!

@lovesegfault
Copy link
Contributor Author

The issue is the return Package line can fail if the version is invalid, and we're not wrapping calls to link_package_data in a try.

@lovesegfault
Copy link
Contributor Author

It seems like the changes in that commit also made it hard to log from this code, which is a real bummer.

@dimbleby
Copy link
Contributor

def link_package_data(self, link: Link) -> Package:
name, version = None, None
m = wheel_file_re.match(link.filename) or sdist_file_re.match(link.filename)
if m:
name = canonicalize_name(m.group("name"))
version = m.group("ver")
else:
info, ext = link.splitext()
match = self.VERSION_REGEX.match(info)
if match:
version = match.group(2)
with contextlib.suppress(ValueError):
version = Version.parse(version)
return Package(name, version, source_url=link.url)

if we go into the else branch then we will try to construct a Package without a name. This surely doesn't make any sense, this code path has gone wrong somehow.

@radoering
Copy link
Member

Yeah, apart from None being not allowed for name according to the type hints, it seems that such packages are always discarded later. Thus, the else branch seems to be completely useless at the moment. Probably, match.group(1) is the name.

@lovesegfault Can you include this fix in your PR and write unit tests at least for link_package_data, but probably better for the callers of link_package_data, too?

@abn
Copy link
Member

abn commented Apr 24, 2022

It seems like the changes in that commit also made it hard to log from this code, which is a real bummer.

Hey @lovesegfault in theory the changes should not have changed anything for logging. Can you clarify what the difficult bits were? Improvements to those kinds of things are important.

@lovesegfault
Copy link
Contributor Author

It seems like the changes in that commit also made it hard to log from this code, which is a real bummer.

Hey @lovesegfault in theory the changes should not have changed anything for logging. Can you clarify what the difficult bits were? Improvements to those kinds of things are important.

I just meant that the class no longer has a _log method. When I tried to add my own, I couldn't get it to work.

I'm not at all familiar with the Poetry codebase though, so perhaps I'm missing something?

@abn
Copy link
Member

abn commented Apr 27, 2022

Fwiw, the _log method is inherited and should be available. Alternatively, simply doing logging.getLogger(__name__) should work too. I also made some recent changes to logging config (on master now) to make debugging better.

Copy link

github-actions bot commented Mar 1, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants