Improve info if needed dep for a package does not exist

[fingolfin]

Print the "dependency XYZ is not satisfied" info message with
warning level PACKAGE_WARNING instead of PACKAGE_INFO. This seems
to be more in line with what the manual promises, namely:

PACKAGE_WARNING should be used whenever GAP has detected a reason why a
package cannot be loaded, and where the message describes how to solve
this problem, for example if a package binary is missing.

So if a package cannot be loaded because one of its needed dependencies
is not the name of any known package, that certainly is something the
user would like to know.

Before this change:

gap> SetInfoLevel( InfoPackageLoading, PACKAGE_WARNING );
gap> LoadPackage("pkg_with_missing_dep");
fail

After this change:

gap> SetInfoLevel( InfoPackageLoading, PACKAGE_WARNING );
gap> LoadPackage("pkg_with_missing_dep");
#I  pkg_with_missing_dep: PackageAvailabilityInfo: dependency 'missing_pkg' is not satisfied
fail

Motivated by issue #3306

[coveralls]

Coverage remained the same at 85.074% when pulling 393a26f on fingolfin:mh/PACKAGE_WARNING-dep-not-satisfied into 6ba858a on gap-system:master.

[olexandr-konovalov]

So if A has B as suggested and B has C as needed, then loading A will trigger a warning saying that B can not be loaded, since C is missing, isn't it?

[fingolfin]

Ah, indeed. But I think that can be fixed with some moderate effort.

[fingolfin]

OK, I had a closer look at this whole package dependency resolution code, and it is a bit ... complex. Not sure how much of that is necessary complexity, and how much not; I'd rather reserve judgement on that for now, as I am pretty sure there are more pitfalls and corner cases I am not actively aware of which might necessitate that complexity.

For the error / warning message at hand, here is what I think when a user should or shouldn't see it by "default". Meaning: at least on on info level PACKAGE_WARNING; but actually, I'd rather see more of these at the default info level; I know this was a controversial topic so far, but I think that's mostly because the current system makes it hard to show those messages only "when appropriate". So I am hoping that by improving this, we can simultaneously resolve the issue at hand, but also make it more palatable to lower some of these warning levels...

So, when should the "X could not be loaded dependency 'Y' is not satisfied" message be shown?

• when using PACKAGE_DEBUG, of course always
• when using PACKAGE_WARNING (or even on the default level):
  • always if the user tried to load X directly (via LoadPackage("X"))
  • NOT if the user only queried whether the package is available
  • always if the user tried to load package Z directly, where Z needs X
  • NOT if the user tried to load package Z' directly, where Z only suggests X
  • always if the user loads package W, which needs both Z and Z'
  • always but only once if the user loads package W', which needs two or more packages which in turn need X

Some ideas on how to tackle that:

• we should keep track if we already printed a given warning/error for a package, in case we revisit it
  • for this we could employ a "cache" (e.g. implemented as a record, mapping package names to a "status") which we init to empty at the start of LoadPackage (resp. other entry points for loading), and in which we keep track of these things
• during dependency resolution, we should keep track of whether we are really trying to load something (then show more warnings), or just trying to find out if we could load something (then perhaps show fewer warnings, or only on a higher level)
  • rationale for this: if the user does LoadPackage, they'll expect helpful error messages if that fails;
  • while if a package X checks if it can load a package Y, in order to safely and properly deal with Y not being available, then this should not trigger such a warning or error messages
    • of course another way to tackle this could be to introduce a mechanism for detecting whether LoadPackage was entered interactively by the user, or from a package. That requires some hacking, but overall shouldn't be too hard (famous last words). Not sure it'd be worth it, though?
• we should keep track if a package is scheduled for loading because it is "needed", or just because it was "suggested";
  • this then needs to be tracked recursively: if package A suggests B, and B needs C, then loading A with C missing should not warn about C missing
  • but note that as we proceed through the dependency graph, this status may change: if we are trying to load A which suggests B; but A also needs D; and D needs B; then of course overall, B is needed. This status change then must be handled appropriately

[wilfwilson]

I think this 1-line change is already an improvement, so I think it should be merged.

I agree with what you wrote just above in #3322 (comment) @fingolfin, including agreeing that it could get fiddly and some of it might not be worth doing. Perhaps we could have a short discussion about this at the GAP Days?

[wilfwilson]

@fingolfin Do you want this PR to be merged, or do you want to wait for a more thorough solution, along the lines you indicated above? I'm happy to merge it if you are.

[fingolfin]

@wilfwilson problem is that while this PR improves one situations, it (arguably) makes another worse, namely the one @alex-konovalov described...