Print dependency cycles in error message

[martijnbastiaan]

While Tasty detects cycles and prints an error message, it does not print the cycles it found. This PR makes sure it does. I've tested it on another code base with cycles in it, the output looks like:

clash-testsuite: Test dependencies form a loop:

..tests.shouldwork.Issues.T2046.VHDL.Vivado => ..tests.shouldwork.Issues.T2046.VHDL.Vivado

..tests.shouldwork.Issues.T2046.SystemVerilog.Vivado => ..tests.shouldwork.Issues.T2046.SystemVerilog.Vivado

..tests.shouldwork.BlackBox.T2117.VHDL.Vivado => ..tests.shouldwork.BlackBox.T2117.VHDL.Vivado

..tests.shouldfail.Verification.NonTemporalPSL.VHDL.Vivado => ..tests.shouldfail.Verification.NonTemporalPSL.VHDL.Vivado

Note the double dot is an artifact of this particular test tree.

[martijnbastiaan]

Sorry, goofed up. The PR should now pass CI as demonstrated here.

[Bodigrim]

LGTM. Indeed, debugging dependency cycles is quite tricky currently.

Since DependencyLoop has been exposed to users, this is a breaking change, but I doubt that there are many clients affected.

[martijnbastiaan]

I've added a changelog entry. @Bodigrim Given that this PR has been open for two weeks and hasn't attracted any comments, I doubt it is controversial. Is there anything I can do to help this get merged?

[Bodigrim]

I think this is a valuable addition, which justifies a minor breakage. Indeed I needed such feature in the past. @andreasabel @ocharles @VictorCMiraldo @adamgundry opinions?

[VictorCMiraldo]

I never needed this and I didn't know that tasty does detect dependency cycles. Makes sense and I can see how this can useful. The only think I'd suggest is shortening the output, if we have a dependency loop [a,b,c,d], why not print names only once? For instance:

Cyclic test dependencies:
  - a
  - b
  - c
  - d

[martijnbastiaan]

I did that to make self-loops non-ambiguous / more obvious to read. In my example I've got four separate dependency cycles, but they're all forming a self-loop. If we'd go with the list as you posted, I think we should do something like:

Cyclic test dependencies:

Cycle 1:
  - a
  - b
  - c

Cycle 2:
  - d

[martijnbastiaan]

@VictorCMiraldo I've implemented your suggestion. Could you re-review and/or start CI?

[VictorCMiraldo]

Thank you @martijnbastiaan, I triggered CI.
@Bodigrim, as you mentioned, this is a breaking change. That being said, I agree that it is very unlikely to break for many users and @martijnbastiaan already prepped the changelog with a relevant entry for a future release of tasty. Do you think this still needs anything or should we merge once CI goes green?

[VictorCMiraldo]

LGTM, thanks!

[martijnbastiaan]

Yes, you're right. And showCycle should do the - stuff. Currently it looks like:

mod-bench: Test dependencies have cycles:All.Power.Data.Mod, All.Power.Data.Mod
- Foo, Bar, Foo

[Bodigrim]

We need "\n- " before the first line as well, otherwise the output looks like

mod-bench: Test dependencies have cycles:All.Power.Data.Mod, All.Power.Data.Mod

[VictorCMiraldo]

Good catch, thanks!

[martijnbastiaan]

Thanks @Bodigrim, I've added a test for it too. Should be good now!

[Bodigrim]

@VictorCMiraldo feel free to merge.

[VictorCMiraldo]

Let's merge then! :)