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

Try telling who is requesting missing package #971

Conversation

davidlatwe
Copy link
Contributor

@davidlatwe davidlatwe commented Nov 5, 2020

Problem

When requested package reqires a packge that is missing, error message only tells what was missing, but not telling which package has that requirement. Unless you crawl the resolving log with verbose enabled.

For example:

>rez-env maya-2020 nice_pipeline
00:37:26 ERROR    PackageFamilyNotFoundError: package family not found: foo (searched: /path/to/packages)

Proposal

When PackageFamilyNotFoundError occurred, catch it internally and look up extracted requirements to find the dependent and re-raise another a bit more detailed PackageFamilyNotFoundError.

Result:

>rez-env maya-2020 nice_pipeline
00:37:26 ERROR    PackageFamilyNotFoundError: package family not found: foo, was required by: maya (searched: /path/to/packages)

@maxnbk
Copy link
Contributor

maxnbk commented Nov 5, 2020

love this idea. I think allan is back from vacay maybe next week, I bet this is a quick merge and I would find very useful.

@j0yu
Copy link
Contributor

j0yu commented Nov 5, 2020

I really like this too, simple and saves time trawling through rez env --verbose

@davidlatwe
Copy link
Contributor Author

davidlatwe commented Nov 6, 2020

Since I find #972 is also a helpful idea and much related with original implementation, I refactored the code and the logic can be shared in both cases.

Here is my test cases, and the output would be as follows:

Conflict on merge
====================
The context failed to resolve:
The following package conflicts occurred: (dep==1 <--!--> dep==2)
	dep==1: foo==1
	dep==2: bar==1

Conflict on intersect
====================
The context failed to resolve:
The following package conflicts occurred: (kai==1 <--!--> kai==2)
	kai==1: tom==1
	kai==2: -

Conflict on intersect (deeper)
====================
The context failed to resolve:
The following package conflicts occurred: (python==2 <--!--> python==3)
	python==2: pymel==2 <-- maya==2020 <-- pipeline==1
	python==3: houdini==18

Required package not found
====================
package family not found: oops, was required by: dummy==1|==2 (searched: memory@any)

Required package not found (deeper)
====================
package family not found: oops, was required by: dummy==2 (searched: memory@any)

Update
Test case updated, and this output from it proves that cyclic (RecursionError) will be avoided when finding dependents.
Phew 💦

Cycle
====================
The context failed to resolve:
A cyclic dependency was detected: nuke-11 --> cycle-00 --> nuke-11

Conflict on intersect (with cycle hidden)
====================
The context failed to resolve:
The following package conflicts occurred: (python==2 <--!--> python==3)
	python==2: pymel==2 <-- maya==2020 <-- pipeline==1
	python==3: nuke==11 <-- cycle==00

@nerdvegas
Copy link
Contributor

There are a few issues here that make this not quite so straightforward. I like the idea though, but I think it has to be refined. The following are in order of severity:

1: It costs something to run _dep_info, and that's bad, because DependencyConflict objects can be created many times in any given solve. This will slow down the solver, adding info to a potentially large number of DependencyConflict objects that never get seen - because it's quite normal for a solve to go through many phases before the final, accepted phase is found. Basically this is adding an extra performance cost to every attempted solve phase, when you only want to see that info in the final phase, and if the solve failed. Any solution that adds extra contextual info to a conflict, needs to do so after the solve has finished, so it doesn't add cost to successful solves. In other words, it needs to be derived from the solve graph.

2: Dependency info is shown as a chain, but this doesn't reflect what's actually going on, since dependencies in the solve graph are parts of the DAG. For example, consider:

The following package conflicts occurred: (python==2 <--!--> python==3)
	python==2: pymel==2 <-- maya==2020 <-- pipeline==1

This may be true, but may also be disguising the fact that this is also true:

The following package conflicts occurred: (python==2 <--!--> python==3)
	python==2: pymel==2 <-- maya-2020+<maya-2020.1 <-- floob==1.2.3

3: Similar to (2), the PackageFamilyNotFound error is not taking into account that multiple packages may have requested the missing package.

I think we need to target this PR specifically at the PackageFamilyNotFound case for the moment. Performance is not an issue here because the exception is raised and the solve ends there. We only need to show the immediate packages that requested the missing package, and that doesn't require the generic _dep_info function.

In terms of what to do about (1) and (2), we can talk about that in a separate PR specifically made for that case. My 2c on that is that we should be able to generate a list of chains for each conflicting requirement, because this shows the right info (it's basically showing multiple flattened paths in a DAG). We can also do this post-solve. I don't think it's as bad as it seems though, and I think there may be an elegant solution.

What you really need to do is get the solve graph (see Solver.get_graph), and extract all the conflict paths from that. There are three benefits to doing it this way:

  • There remains just one function that generates graph info from the solve;
  • This will also work for "total reduction" type failures, which this PR does not take into account;
  • This will show info that this PR is missing - the actual package requests in the chain. Ie whereas a chain shown in this PR might be foo-1 --> python==2.7.3, the actual chain is more likely to be something like foo-1 --> python-2 --> python==2.7.3. Because you're deriving the chain from extractions only, the requires info (ie that foo requires python-2 in this eg) can be lost (eg, in cases where multiple extractions overlap and are merged).

I'm going to close this PR, as I want to get into the habit of doing that rather than leaving loads of PRs hanging. But as I said I think this is headed in the right direction, and a PR targeted at the PackageFamilyNotFound case should be straightforward.

@nerdvegas nerdvegas closed this Nov 17, 2020
@davidlatwe
Copy link
Contributor Author

Thanks Alan, great hints ! ❤️

3: Similar to (2), the PackageFamilyNotFound error is not taking into account that multiple packages may have requested the missing package.

I'll submit another PR that only focusing on PackageFamilyNotFound, which will be based on commit 4c51335 plus above point fixed.

And yes, I'll go check out the graph and try that again. 👍🏼

@davidlatwe davidlatwe deleted the improve-family-not-found-err branch November 18, 2020 16:02
@j0yu
Copy link
Contributor

j0yu commented Nov 20, 2020

Thanks @nerdvegas for the information and @davidlatwe for trying to do an initial implementation.

Originally I opened #972 so I can have a go at it someday. It's more clear now, with the feedback from #971 (comment), on things to look out for when implementing it

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

Successfully merging this pull request may close these issues.

4 participants