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

Improve context resolve failure info #1083

Merged

Conversation

davidlatwe
Copy link
Contributor

@davidlatwe davidlatwe commented May 22, 2021

Continue from #971, and based on the suggestion in this comment, here is another take that work no top of context resolve graph.

Say we have these packages and try requesting "pipeline", "maya", "houdini-18":

# test_conflict_pipeline
repo = Repo()
repo.add("pipeline", version="1", requires=["maya-2018+<=2020"])
repo.add("maya", version="2020", requires=["pymel"])
repo.add("pymel", version="2", requires=["python-2"])
repo.add("houdini", version="18", requires=["python-3"])
repo.add("python", version="2")
repo.add("python", version="3")

context = ResolvedContext(["pipeline", "maya", "houdini-18"])
context.print_info()

The output from current master branch:

The context failed to resolve:
The following package conflicts occurred: (python-2 <--!--> python==3)

And here's the output from this PR branch:

resolve failed, by david@DESKTOP-EABK8SP, on Sat May 22 22:58:58 2021, using Rez v2.88.2

requested packages:
pipeline                     
maya                         
houdini-18                   
~platform==windows           (implicit)
~arch==AMD64                 (implicit)
~os==windows-10.0.19041.SP0  (implicit)

The context failed to resolve:
The following package conflicts occurred: (python-2 <--!--> python==3)

Resolve paths starting from initial requests to conflict:
  pipeline ? --> pipeline-1 --> maya-2020 --> pymel-2 >>> python-2
  maya ? --> maya-2020 --> pymel-2 >>> python-2
  houdini-18 ? --> houdini-18 --> python-3

And here's an example on cyclic failure:

# test_big_cycle
repo = Repo()
repo.add("pipeline", version="0", requires=["nuke-10"])
repo.add("nuke", version="10", requires=["cycle", "python==3"])
repo.add("python", version="3")
repo.add("cycle", version="1", requires=["bigger"])
repo.add("bigger", version="5", requires=["nuke"])

context = ResolvedContext(["pipeline", "cycle"])
context.print_info()

The output:

resolve failed, by david@DESKTOP-EABK8SP, on Sat May 22 23:01:24 2021, using Rez v2.88.2

requested packages:
pipeline                     
cycle                        
~platform==windows           (implicit)
~arch==AMD64                 (implicit)
~os==windows-10.0.19041.SP0  (implicit)

The context failed to resolve:
A cyclic dependency was detected: bigger-5 --> nuke-10 --> cycle-1 --> bigger-5

Resolve paths starting from initial requests to cycle:
  pipeline ? --> pipeline-0 --> nuke-10 --> cycle-1 --> bigger-5 --> nuke-10
  cycle ? --> cycle-1 --> bigger-5 --> nuke-10 --> cycle-1

Here is the full code of above testing examples.

@nerdvegas
Copy link
Contributor

Hey David this looks really promising.

Just one minor thing, I think the formatting should be a little different. Eg you have:

Resolve paths starting from initial requests to cycle:
  pipeline ? --> pipeline-0 --> nuke-10 --> cycle-1 --> bigger-5 --> nuke-10
  cycle ? --> cycle-1 --> bigger-5 --> nuke-10 --> cycle-1

I would change to:

Resolve paths starting from initial requests to cycle:
  pipeline --> pipeline-0 --> nuke-10 --> cycle-1
  cycle --> cycle-1 --> bigger-5 --> nuke-10 --> cycle-1

And similarly:

Resolve paths starting from initial requests to conflict:
  pipeline ? --> pipeline-1 --> maya-2020 --> pymel-2 >>> python-2
  maya ? --> maya-2020 --> pymel-2 >>> python-2
  houdini-18 ? --> houdini-18 --> python-3

To:

Resolve paths starting from initial requests to conflict:
  pipeline --> pipeline-1 --> maya-2020 --> pymel-2 --> python-2
  maya --> maya-2020 --> pymel-2 --> python-2
  houdini-18 --> houdini-18 --> python-3

We don't need the extra syntaxes ? and >>> since the description header already gives enough context.

Also though, I'm not clear as to why the cycle output actually shows requirements that go past the cycle package? Here:

pipeline ? --> pipeline-0 --> nuke-10 --> cycle-1 --> bigger-5 --> nuke-10

Finally, I thnk it'd be good to add a line to the output like so:

To see a graph of the failed resolution, write the context to file (using rez-env -o), then view the graph (using rez-context -g)

@nerdvegas
Copy link
Contributor

Ok so on further thought wrt cycle, how about this:

The context failed to resolve:
A cyclic dependency was detected: bigger-5 --> nuke-10 --> cycle-1 --> bigger-5

Resolve paths starting from initial requests to cycle:
  pipeline ? --> pipeline-0 --> nuke-10
  cycle ? --> cycle-1

In other words, just show the dep chain from each request, to where it first hits a pkg in the cycle. Any more info than this is redundant, since the cycle is already identified on that first line.

@davidlatwe
Copy link
Contributor Author

We don't need the extra syntaxes ? and >>> since the description header already gives enough context.

Yes, I was thinking if it's redundant as well.

Finally, I thnk it'd be good to add a line to the output like so:

Fine touch !

About cyclic path, I was thinking that maybe would be better to show the full cycle for better knowing what's going on in one full path ?

If shorter path is desired, we could stop visiting nodes when hitting the edge that marked as CYCLE.

@davidlatwe
Copy link
Contributor Author

davidlatwe commented May 25, 2021

About the additional hint message for viewing graph, what about rez-env --fail-graph or rez-build --fail-graph ?

@davidlatwe
Copy link
Contributor Author

Okay, now the message looks like this

resolve failed, by davidlatwe.lai@MoonShineExp27, on Tue May 25 18:41:58 2021, using Rez v2.88.2

requested packages:
pipeline                     
maya                         
houdini-18                   
~platform==windows           (implicit)
~arch==AMD64                 (implicit)
~os==windows-10.0.18362.SP0  (implicit)

The context failed to resolve:
The following package conflicts occurred: (python-2 <--!--> python==3)

Resolve paths starting from initial requests to conflict:
  pipeline --> pipeline-1 --> maya-2020 --> pymel-2 --> python-2
  maya --> maya-2020 --> pymel-2 --> python-2
  houdini-18 --> houdini-18 --> python-3

To see a graph of the failed resolution, add --fail-graph in your rez-env or rez-build command.

And this

resolve failed, by davidlatwe.lai@MoonShineExp27, on Tue May 25 18:42:46 2021, using Rez v2.88.2

requested packages:
pipeline                     
cycle                        
~platform==windows           (implicit)
~arch==AMD64                 (implicit)
~os==windows-10.0.18362.SP0  (implicit)

The context failed to resolve:
A cyclic dependency was detected: cycle-1 --> bigger-5 --> nuke-10 --> cycle-1

Resolve paths starting from initial requests to cycle:
  pipeline --> pipeline-0 --> nuke-10 --> cycle-1
  cycle --> cycle-1

To see a graph of the failed resolution, add --fail-graph in your rez-env or rez-build command.

@nerdvegas
Copy link
Contributor

nerdvegas commented May 25, 2021 via email

@nerdvegas nerdvegas merged commit fc2da48 into AcademySoftwareFoundation:master Jun 1, 2021
@davidlatwe davidlatwe deleted the resolve-fail-msg branch June 1, 2021 05:02
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.

2 participants