Fix --fail-above with --exclude.

[marcandre]

This PR deals with exclusions once, which simplifies the code.

[marcandre]

New test was failing, as the count for fail-above was 6 instead of 1.

[josevalim]

Hi @marcandre!

It is worth noticing there is a difference in behaviour here. You are filtering now before we analyze the graph, which means that transitive dependencies through that node are no longer caught, while before it would simply remove those nodes from the results (with the exception of :compile_connected but perhaps that shouldn't consider excluded either).

We should discuss if we really want to make this change.

[marcandre]

Indeed. I do not know which is most useful/intuitive.
I'm using --exclude currently to remove dependencies that disappear in Phoenix 1.6. I think either ways give me the same result.
I'll make a different PR to fix the count without changing the behavior

[josevalim]

Hi @marcandre, just a heads up we plan to have a release candidate next week, in case you want to see this included. :)

[marcandre]

Thanks! Yes, I definitely want to fix that bug, so I'll have a separate fix by then.
As I'm using excluded strictly with compile_connected, I really can't think of a use case where there would be a difference in behavior (and thus argue one way or the other).

[marcandre]

I'll first start to write a test, because I'm not sure how this PR differs. At least for format "dot/plain/pretty", we go through the graph using callback and excluded references stop the processing.

Or did you mean that the change was for "stats" and "cycles"? Are those meant to be compatible with excluded? I'd tend to say that doing the actual filtering as proposed here might be a good option.

[josevalim]

@marcandre imagine a.ex depends on b.ex which depends on c.ex. My understanding is that, by filtering early, we will now no longer show c.ex at all. By filtering earlier, I think c.ex still appears in the "end result", just not linked to anything.

I may be misremembering the behaviour. But that's what I think is the difference. It should appear on any kind of graph.

[marcandre]

I rebased this branch on #11330, to show that the particular example of 3 "linear" dependencies (as I understood it) does not change with this PR (with or without a --source).

Is there a different scenario that I should be testing?

[josevalim]

💚 💙 💜 💛 ❤️

[marcandre]

Great, thanks!

[marcandre]

@josevalim Looks like this was not backported correctly to the 1.13 branch?

06594f5 only includes a test, while as 7efd70b28 includes the code fix and the test. Looks partly my fault, as my test appears not to be sufficient to reveal the issue...

Nevermind, I had an out of date master branch which has been renamed main. All is well, sorry for the noise.