Print reason for requiring resolution in verbose mode

[neonichu]

When running SwiftPM while requiring a resolved file, print some more verbose info on why SwiftPM decided the existing file was out-of-date or why a resolved file was required.

Motivation:

We have been seeing some unexplained issues in CI related to resolved files where this would help us to debug them. It can also generally be helpful for folks.

Modifications:

If resolution is required and we are in verbose mode, append a reason to the diagnostic about the resolved file being out-of-date.

Result:

This is a pretty basic translation from WorkspaceResolveReason into prose which may need some improvements in the future, but it's a start at least.

[neonichu]

@swift-ci please smoke test

[neonichu]

This actually needs an update, I just found that there's an existing mechanism for this willResolveDependencies with already better output.

[abertelrud]

Should this only be for verbose mode? It seems that with this error, it's useful to always know the reason.

[neonichu]

@abertelrud I was thinking we only do it in verbose mode initially in case we still want to work on the formatting of the output, but considering this goes to main anyway, maybe that's not necessary.

[abertelrud]

@abertelrud I was thinking we only do it in verbose mode initially in case we still want to work on the formatting of the output, but considering this goes to main anyway, maybe that's not necessary.

I do prefer providing minimal regular information, but in the case of this error, it seems useful to me to have the extra output. Since it's in main we can iterate on it. The alternative is that people have to re-run with verbose mode, and that's suboptimal when looking at CI output (less easy to rerun the command with extra flags).

[neonichu]

Changed this now by moving the existing code for pretty printing WorkspaceResolveReason and always appending it to the diagnostic here, not just in verbose mode.

[neonichu]

@swift-ci please smoke test

[neonichu]

Ah the existing tests for the diagnostics need an update.

[neonichu]

Looks like the values of WorkspaceResolveReason aren't actually stable :/ In testForceResolveWithNoResolvedFile, I would expect both dependencies to show up, but instead just one of them shows up and which one is not deterministic.

[neonichu]

So I guess we have two options:

• change the test so that it checks the prefix of the diagnostic or something like that and live with the non-determinism for now
• hold off on improving the diagnostic until we can make it deterministic

I'm kind of tending towards option 1, since this seems like a strict improvement even with the non-determinism. What do others think?

[abertelrud]

It depends on how much effort making it deterministic would be. I usually prefer incremental improvements, since deeper dives sometimes get shelved when more urgent things come along and then nothing has improved. So in that spirit I would go for 1, but it would be great to later try to make it deterministic.

[neonichu]

It looks like the problem is that despite the WorkspaceResolveReason carrying a list of dependencies which are new, it'll actually stop resolving after encountering the first missing one. This, together with the fact that this happens during package loading which is done concurrently leads to the non-determinism.

[neonichu]

@swift-ci please smoke test

[neonichu]

Updated this now to go with option 1 and filed a follow-up radar for the non-determinism.

[tomerd]

@swift-ci smoke test

[neonichu]

@swift-ci smoke test