GAP should display more information when a package doesn't load.

[ChrisJefferson]

Try loading a package (like profiling) which requires IO, with IO not compiled, just prints fail.

It would be useful if more useful information was returned. One obvious choice would be to set InfoPackageLoading to a higher default level (after checking it only printed messages by default when a package fails to load).

While there are methods of finding out why packages aren't loading, which can be found by reading the LoadPackage manual page, I think it would be much more useful if this information was just printed to users.

If users wanted no information, they could manually put InfoPackageLoading back to a lower level?

[olexandr-konovalov]

Many packages are loaded automatically e.g. if they are suggested packages for another package - having GAP printing a log of these details at start wouldn't be convenient. OTOH we have DisplayPackageLoadingLog which can be called at any time, for example

gap> DisplayPackageLoadingLog(4);                                     
#I  GAP: entering AutoloadPackages (no workspace)
#I  GAP: entering InitializePackagesInfoRecords
#I  GAP: return from InitializePackagesInfoRecords
#I  GAP: trying to load needed packages
#I       gapdoc (>= 1.2)
#I  GAPDoc: entering LoadPackage  (omitting suggested packages)
#I  GAPDoc: PackageAvailabilityInfo for version 1.5.1
#I          (required: >= 1.2)
#I  GAPDoc: PackageAvailabilityInfo: the AvailabilityTest function returned 'true'
#I  GAPDoc: PackageAvailabilityInfo: no needed packages
#I  GAPDoc: PackageAvailabilityInfo: version 1.5.1 is available
#I  GAPDoc: start loading needed/self packages
#I          gapdoc
#I  GAPDoc: start reading file 'init.g'
#I  GAPDoc: finish reading file 'init.g'
#I  GAPDoc: return from LoadPackage
#I  GAP: needed packages loaded
#I  GAP: read the impl. part of the GAP library
#I  GAPDoc: start reading file 'read.g'
#I  GAPDoc: finish reading file 'read.g'
#I  GAP: omitting packages suggested via "PackagesToLoad" (-A option)
#I  GAP: call LoadPackageDocumentation for not loaded packages
#I  cohomolo: book `cohomolo': no manual index file `doc/manual.six', ignored
#I  GAP: LoadPackageDocumentation for not loaded packages done
#I  GAP: return from AutoloadPackages
#I  profiling: entering LoadPackage 
#I  profiling: PackageAvailabilityInfo for version 0.3.0
#I  profiling: PackageAvailabilityInfo: the AvailabilityTest function returned false
#I  profiling: PackageAvailabilityInfo: no installed version fits
#I  profiling: return from LoadPackage, package is not available

• maybe we should just publicise that information more?

[stevelinton]

As @alex-konovalov pointed out while I was typing this, what we don't want are messages arising, directly or indirectly from loading suggested packages (whether suggested by GAP itself, or by other packages), but not related to needed packages. These used to come up a lot in the past and irritated people. Making the cut correctly is tricky -- during GAP initialization, or responding to an explicit LoadPackage, there may be several reasons why a specific package is looked at. We only want to bother the user if (a) it can't be loaded and (b) at least one of the reasons is a chain from GAP or a manually loaded package, consisting purely of "needed" relationships with no "suggested" relationships.

[ChrisJefferson]

How about the following as an heuristic:

(1) Only print when the banner option of LoadPackage is true, so if a banner would be printed when the package is loaded, then a message is printed if the package isn't loaded. This is OK (I think) because the user is already expecting to see output (the banner, when loading is successful).

(2) Don't do this for suggested packages (or recursively, anything required by suggested packages).

This would be done in practice by adding an extra option to LoadPackage, so it becomes LoadPackage( name[, version][, banner [, verbose] ] ), where verbose is by default set to the same value as banner (but can be set to false on purpose, so when LoadPackage is loading a suggested package).

[stevelinton]

Implementing (2) needs a bit more than this because a single LoadPackage() call can reach the same dependent package by multiple routes. You actually have to analyze the directed graph of dependencies, edge-coloured by whether it is needed or suggested.

[olexandr-konovalov]

@ChrisJefferson for packages from GAP 4.7.9 release, you may see this graph, built from information in PackageInfo.g files, at https://alexk.host.cs.st-andrews.ac.uk/gap/GAP_Packages.html

[fingolfin]

I absolutely think it would be worthwhile to do that. I have wished for this countless times, and I have also apologized to GAP users countless times for this un-intuitive behavior by GAP.

Another heuristic: Only print a message if LoadPackage is called directly from the GAP command prompt, i.e interactively, but not for any other LoadPackage invocations. A simple first stab at that would be to use a special "option", (as in ValueOption). LoadPackage would contain this pseudo code:

isTopLevel := ValueOption("isRecursiveLoadPackageCall") = fail;
if isTopLevel then
    PushOptions( rec( isRecursiveLoadPackageCall := true) );
fi;
...
if loadingFailed and isTopLevel then
   PrintReason(...)
fi;

or so... the reason could simply be "failed to load because depency XYZ failed to load". To find out more, the user could then try to LoadPackage("XYZ") which then might print "failed to load because executable ABC not found"

[ChrisJefferson]

I will admit I'd never looked at LoadPackage, and I assumed it just recursively called LoadPackage. After reading the source, I see that as @stevelinton says, it is more complicated than that :) (for good reasons). I think @fingolfin 's is a good start, just letting people know which requirement failed to load (or if the package failed it's own availabilitytest) would be a good start.

Another thing I'd like to look at, at the same time, would be making it easier for packages to provide a message to users. Some packages use InfoWarning (many are now commented out, as @stevelinton mentioned). Adding an explicit Info we can let packages use, and enable/disable as we choose would let packages give users more helpful info.

[ChrisJefferson]

Here is one option, which doesn't add too much code. In short, use the existing PackageAvailabilityInfo and TestPackageAvailability functionality, when LoadPackage is called interactively, with banner=true (so we know users are expecting some output), and LoadPackage fails.

To see the output I'm suggesting (obviously coded sensibly), run the following:

SetInfoLevel(InfoPackageLoading, PACKAGE_INFO); TestPackageAvailability("profiling");.

For example, if I do not compile io and try to loading profiling, I get:

gap> LoadPackage("profiling");
#I  profiling: PackageAvailabilityInfo for version 0.3.0
#I  IO: PackageAvailabilityInfo for version 4.4.4
#I      (required: >= 4.4.4)
#I  IO: PackageAvailabilityInfo: the AvailabilityTest function returned fail
#I  IO: PackageAvailabilityInfo: no installed version fits
#I  profiling: PackageAvailabilityInfo: dependency 'io' is not satisfied
#I  profiling: PackageAvailabilityInfo: no installed version fits
fail

whereas with io I get:

gap> TestPackageAvailability("io");
#I  IO: PackageAvailabilityInfo for version 4.4.4
#I  IO: PackageAvailabilityInfo: the AvailabilityTest function returned fail
#I  IO: PackageAvailabilityInfo: no installed version fits
fail

[ChrisJefferson]

Might want to edit the output under SetInfoLevel(InfoPackageLoading, PACKAGE_INFO); so it reads more naturally, but that could be done for other situations too. Perhaps something like:

gap> LoadPackage("profiling");
#I  PackageAvailabilityInfo: Check loading profiling version 0.3.0
#!  PackageAvailabilityInfo: Requires IO >= version 4.4.4
#I  PackageAvailabilityInfo: Check loading IO version 4.4.4
#I  PackageAvailabilityInfo: the AvailabilityTest function for IO version 4.4.4 returned fail
#I  PackageAvailabilityInfo: no installed version of IO can be loaded
#I  PackageAvailabilityInfo: dependency IO of profiling version 0.3.0 is not satisfied
#I  PackageAvailabilityInfo: no installed version of profiling can be loaded
fail

But as I look at this, while I think this is better than what we have, it might be better to come up with something much shorter, like:

gap> LoadPackage("profiling");
#! PackageLoading: Cannot load profiling, as the required package IO >= 4.4.4 could not be loaded

Which could also be easily done, by just calling TestPackageAvailability on each requirement, to see which one failed (or if it was availabilitytest on the top package).

[fingolfin]

Yeah, a single concise line would be better. In particular, if there is no error, then there should not be any output either.

[ChrisJefferson]

@fingolfin : I was planning that there will be no output if there is no error. My current way to do that is to basically double the work -- only run TestPackageAvailability with a changed Info level just before we return fail. While this slightly slows things down, it is still very quick, and only happens in the failure case.

I'll have a look at writing a function that returns a single failure line -- in particular for the easy cases (we can always switch to a longer dump in cases where the issue is more complicated -- some cyclic issue for example, or a malformed package).

[stevelinton]

There is already an interface for packages to supply messages LogPackageLoadingMessage (and an Info class InfoPackageLoading). See reference manual 76.2-4.

I think all we need to do is analyze the log at the end of a LoadPackage call and be a little more generous in what we print by default.

Another option would just be to print one line suggesting a call to DisplayPackageLoadingLog.

[frankluebeck]

@stevelinton wrote:

Another option would just be to print one line suggesting a call to DisplayPackageLoadingLog

This would be my advise as well. But, of course, if someone sees a way to automatically generate a useful one line information about the failure to load a package (only if banner = true) that would be nice as well.

@ChrisJefferson: Please do not change the layout of the info message as you showed above. It is very useful to see at the beginning of each line to which package that line belongs. Note that package loading can be nested in a complicated way, because first all declaration parts are read and then the implementation parts.

[olexandr-konovalov]

I've attempted to respond to this in #610 - any feedback?

[stevelinton]

The question I guess is what ought to be in info message and what ought to be in the log. At the moment the roles of the two seem confused. Once we know what we want it should be easy enough to implement it and come up with a one-line hint to point people at it.

Maybe the answer is that all these messages should be in both places -- printed immediately if the info level is high enough, and retained in the log for analysis after the fact.

[olexandr-konovalov]

The PR #610 has been merged and should improve this. Let's see how it goes. The discussion lead to some more suggestions, and in a comment to #610 @stevelinton suggests that any reliable solution should include these three measures:

1. make sure that all InfoPackageLoading messages get logged -- actually why don't we log ALL info messages globally? We can always start dumping old or low-priority message after the log gets to 64MB or something.
2. write a function which analyses the log after a failed package load and actually identifies the reason(s) taking proper account of all the weird possibilities.
3. either print the output of this after a failed package load, or print a short message explaining how to get it.

We should keep them in mind for possible refactoring of the package loading mechanism later.

[james-d-mitchell]

@fingolfin @olexandr-konovalov @stevelinton @ChrisJefferson this is resolved no? Loading a package:

gap> LoadPackage("alnuth");
#I  alnuth package is not available. Check that the name is correct
#I  and it is present in one of the GAP root directories (see '??RootPaths')
fail

[fingolfin]

Yes this is still an issue. Two stalled (failed?) attempts at addressing it are https://github.com/gap-system/gap/pull/4795 and https://github.com/gap-system/gap/pull/3322