Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow skipping validation when installing packages #129

Closed
fingolfin opened this issue Jul 26, 2024 · 2 comments
Closed

Allow skipping validation when installing packages #129

fingolfin opened this issue Jul 26, 2024 · 2 comments
Labels
feature request New feature

Comments

@fingolfin
Copy link
Member

I was just asked how to install Frank Lübeck's FUtil package.

Unfortunately my first idea, installing it via PackageManager, failed:

gap> LoadPackage("PackageManager");
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Loading PackageManager 1.4.3 (Easily download and install GAP packages)
by Michael Young (https://mct25.host.cs.st-andrews.ac.uk/).
maintained by:
   Michael Young (https://mct25.host.cs.st-andrews.ac.uk/) and
   The GAP Team (support@gap-system.org).
Homepage: https://gap-packages.github.io/PackageManager/
Report issues at https://github.com/gap-packages/PackageManager/issues
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────
true
gap> InstallPackage("http://www.math.rwth-aachen.de/~Frank.Luebeck/gap/FUtil/futil.0.1.5.tar.gz");
#I  Downloading archive from URL http://www.math.rwth-aachen.de/~Frank.Luebeck/gap/FUtil/futil.0.1.5.tar.gz ...
#I  Saved archive to /var/folders/d_/1zss2fnd6xdgclqnj8jj_27m0000gp/T//gaptempdirsZLWwx/futil.0.1.5.tar.gz.pkgman
#I  Extracting to /Users/mhorn/Library/Preferences/GAP/pkg/futil ...
#I  Checking dependencies for FUtil...
#I    GAPDoc >= 1.5: true
#E  component `License' must be bound to a nonempty string containing an SPDX ID
#I  WARNING: could not build doc (no makedoc.g or doc/make_doc)
#E  component `License' must be bound to a nonempty string containing an SPDX ID
#I  PackageInfo.g validation failed
#I  (in /Users/mhorn/Library/Preferences/GAP/pkg/futil)
#I  Removed directory /Users/mhorn/Library/Preferences/GAP/pkg/futil
false

Oops -- the package predates the introduction of the License field, so of course it doesn't validate -- but that's fine, the package works, so it should be installable!

So I think there should be a way to ignore the validation. Some additional thoughts:

  • there should also be a way to prevent it from deleting the packages; besides helping here, it would also help with debugging in general, I think we have at least one issue requesting that
  • ValidatePackageInfo perhaps needs to be changed, it has too many roles: validating whether a package satisfies the minimals to be usable is different from validating that a package is satisfactory for distribution in the GAP package distro, and then there are certainly levels in between. Perhaps we need options to ValidatePackageInfo to control how strict it is; or let it return a value more complex than just "true" or "false"
@mtorpey mtorpey added the feature request New feature label Aug 23, 2024
@fingolfin
Copy link
Member Author

Actually I just rediscovered that in PR #119 (commit d69e567) I added a debug option that should stop it from deleting directories!

But this is not documented in the InstallPackage documentation. And perhaps it should be called keepDirectory or preserveDirectory instead?

One could still also have a ignoreValidation (or so) option. But I think it's better if we reconsider how we use ValidatePackageInfo here, and distinguish between "issues that will cause serious problems" vs. "policy violation that's a problem for the package distribution but not for a user"

@mtorpey
Copy link
Collaborator

mtorpey commented Aug 28, 2024

I've addressed this in e9922c8 by relaxing the requirement to pass validation when being installed.

The documentation for ValidatePackageInfo explicitly states:

This function is intended to support package authors who create or modify PackageInfo.g files. (It is not called when these files are read during the startup of GAP or when packages are actually loaded.)

It therefore doesn't seem reasonable to require a passing validation in order to call an installation successful. This was a bit of an abuse of ValidatePackageInfo by me. We now run ValidatePackageInfo, but just print an info warning if it returns false.

We do need at least some stuff in PackageInfo.g for a successful install. I've therefore introduced PKGMAN_RequiredPackageInfoFields, which defines four fields that must be there, and we really do reject if they're not there. This list could be modified in future.

The separate discussion about not deleting directories can continue in #66.

@mtorpey mtorpey closed this as completed Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature
Projects
None yet
Development

No branches or pull requests

2 participants