Root package version should be optional

[zanieb]

Example at https://github.com/zanieb/pubgrub/blob/zanie/examples/examples/unsat_root_messages.rs

In practice, the root package often does not have a real or known version number. To get around this, we just use a dummy minimum version for the root package but then the version appears in messaging e.g.

Because there is no version of foo in 1.0.0 and root 0.0.0 depends on foo 1.0.0, root 0.0.0 is forbidden.

To resolve this, a PubGrub user must:

• Define a Package type with "root" and "not root" variants
• Implement a custom Display trait for External which requires copying the entire Reporter implementation

Ideally the root package version would be optional entirely or it would be an option to omit it from the report.

[Eh2406]

Thank you for trying PubGrub! I appreciate the production feedback.

I am just giving first reactions here. I may disagree with the opinion stated upon further reflection or the arrival of evidence.

• We should definitely have an example, in the section in the guide, about having a enum<P> {Root, Else(P)}! (If nothing else that can show me how painful it really is in practice.)
• If getting the error messages to display correctly requires "copying the entire Reporter implementation" than something is very wrong. We definitely need to improve the API so that this is not required! Suggestions are welcome.
• Adding a way to take a derivation_tree and strip out mentions of the root seems like a very helpful thing for us to provide. Once we have that implemented, we should see if there are other generalizations worth making.
• The dart implementation has a micro optimization where all references to the root packages are stripped out of incompatibilities. This adds a lot of special cases that I don't think pulls its weight. In general, the more internal details that know what the root package is (even to strip it out) the less happy I am. That being said, alternative APIs can be made that mainly affect to code in solver I am open to exploring it.

Reading over your statement again and my response, I may be responding to "my repackage doesn't have a name" not "my repackage doesn't have a version". If I have misunderstood and the two topics are not deeply intertwined, my responses may be entirely irrelevant.

[zanieb]

Thanks for the response!

enum<P> {Root, Else(P)} is not a problem itself. We actually need to do Root(Option<String>) to allow it to have a name in some cases but that's fine.

The greater problem is that the version of root is meaningless and propagated through the reporting system. Here's an example of the code I wrote to strip versions from the display of the root package

https://github.com/zanieb/pubgrub/blob/zanie/examples/examples/unsat_root_message_no_version.rs

[mpizenberg]

The error reporting is probably the part of this lib that received the less love. It's not much optimized, performance-wise and there could be much better ways saying what the errors are while reading the derivation tree. It really is a minimal viable error reporting. The structure used for error reporting is a derivation tree, roughly presented here: https://pubgrub-rs-guide.netlify.app/pubgrub_crate/custom_report. We haven't tried implementing easy way to extend the default reporter as frankly, it was a minimum effort reporter and I didn't expect to live until now.

Regarding the "root" name, I agree it's very weird to have it presented like that in the reports. In your example :

Because there is no version of foo in 1.0.0 and root 0.0.0 depends on foo 1.0.0, root 0.0.0 is forbidden.

it would make much more sense if the report said instead something like this:

Because you depend on foo @ 1.0.0 and there is no such version, there is no solution

I don't know how much of a better minimal version reporter we could make by "just" updating it a bit. Or if it needs a full rewrite ...

Regarding the version associated with root package. It's because the solver can be used to solve both projects and package roots. And for a package root it's nice to have it's version displayed too. We didn't make a distinction in the reporter.

[zanieb]

Thanks for the additional context.

Because you depend on foo @ 1.0.0 and there is no such version, there is no solution

I agree this would be ideal. The existing error messages are unfortunately insufficient for our use-case and thus it is my highest priority to determine if we can reach a more user-friendly state.

It sounds like I should attempt to write a new reporter that is specifically for "projects".

[Eh2406]

The greater problem is that the version of root is meaningless and propagated through the reporting system. Here's an example of the code I wrote to strip versions from the display of the root package

While looking over the changes you had to make I got to wondering. Should you have an enum for version? Where the display of the Version::Root is the empty string?

I completely agree that we should provide a more flexible reporter, but in the meantime I wonder if that would help.

[zanieb]

Should you have an enum for version? Where the display of the Version::Root is the empty string?

Hm interesting I hadn't considered that. Then we'd end often up with an extra space since the formatting is "blah blah {package} {version} blah blah", right? It also seems slightly more complicated when dealing with version sets.