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

Prevent loss of deep dependencies of local requirements #1506

Closed

Conversation

AndydeCleyre
Copy link
Contributor

@AndydeCleyre AndydeCleyre commented Oct 13, 2021

Fixes #1505

Fixes #1054

I've just:

@richafrank Please have a look, as I don't know what any negative consequences or better alternatives might be.

That said, all existing tests continue to pass, and the behavior regarding those bugs is looking good.

TODO:

  • sanity check
  • add tests
Contributor checklist
  • Provided the tests for the changes.
  • Assure PR title is short, clear, and good to be included in the user-oriented changelog
Maintainer checklist
  • Assure one of these labels is present: backwards incompatible, feature, enhancement, deprecation, bug, dependency, docs or skip-changelog as they determine changelog listing.
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@AndydeCleyre AndydeCleyre changed the title Prevent loss of deep dependencies of local requirements [Fixes #1505] Prevent loss of deep dependencies of local requirements Oct 13, 2021
@AndydeCleyre AndydeCleyre force-pushed the restore-copy-ireq-deps branch from 61f351c to 81129a1 Compare October 13, 2021 00:59
@richafrank
Copy link
Contributor

General idea looks right to me @AndydeCleyre ! Looking at the differences between yours and mine:

  1. This is missing handling for the combined_ireq being a constraint.
  2. This didn't restore the test for FakeRepository.
  3. I added a new test for Second level dependencies missing on for local file packages on >= 6.3.1 #1505.

So basically the same!

As I mentioned in #1505 (comment), I think there may be another broken case that no one's complained about yet, so we might need to keep iterating on this. The various pieces of state here are very challenging...

@AndydeCleyre
Copy link
Contributor Author

Closing in favor of #1507, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants