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

Orb package not compiled after "InstallPackage("orb");" #49

Open
james-d-mitchell opened this issue Nov 8, 2019 · 5 comments
Open

Orb package not compiled after "InstallPackage("orb");" #49

james-d-mitchell opened this issue Nov 8, 2019 · 5 comments
Labels
discussion Not exactly an issue to be resolved, but a discussion about the future of the package

Comments

@james-d-mitchell
Copy link
Contributor

The title of the issues says it all I think, orb is not compiled by the package manager. It'd be great if it was.

@mtorpey
Copy link
Collaborator

mtorpey commented Nov 8, 2019

Thanks for the report! I've looked at it, and this is due to optional compilation.

It's really hard to tell whether a given package has been compiled yet, so at the moment we only run the compilation script on packages that can't currently be loaded. Since orb can be loaded straight away without compiling, we don't do it. However, it would be good if it did get compiled.

I wonder what a good condition would be for running the compilation script. We don't want to run it every time we go near a package (messy!) but maybe we can find some happy medium...

@james-d-mitchell
Copy link
Contributor Author

The thing that really gets me confused with this, is that Orb sets a global variable ORBC if it's compiled, and for some reason when I'm using the PackageManager, this is always bound whether the package is compiled or not. Maybe this is more of an issue for the Orb package than the package manager.

@mtorpey mtorpey added the discussion Not exactly an issue to be resolved, but a discussion about the future of the package label Jan 22, 2020
@fingolfin
Copy link
Member

Perhaps adding a BuildPackage or CompilePackage function would be helpful, so that user's at least can force this bit if they need to?

Also, we could discuss extending the PackageInfo.g record by (optional) functions to test whether a package was compiled. Or perhaps GAP should enforce something there? This whole issue is also vaguely related to the question of installing GAP and install packages (system wide, I mean); and to having multiple GAP builds share the same copy of a package (so you'd ideally want to compile the package multiple times, once for each GAP build).

One neat way to pull this of might be the following (for packages that support it -- on the long run, I'd hope that'd be all): each GAP build creates its own directory where it stores compile output (and most importantly: binaries / kernel extensions) of a given package. The name and location of that directory might be hard coded (e.g. for system wide installs by a Linux distro, it could be /usr/lib/gap/pkg/PKGNAME), or relative to the GAP executable, or even in a subdirectory of the package directory (then to ensure uniqueness, the dir name could be e.g. the SHA1 of the path to the GAP executable). The build system of the package would have to support building out of tree, and also support the use of a given directory for that. And of course GAP would have to be able to query the package for all that -- probably via suitable PackageInfo.g entries.

So we might have something like this in the PackageInfo record:

rec(
...
BuildPackage := function( gapdir, pkgdir, builddir )
  local res;
  # initiate out-of-tree build
  res := Process( builddir,
      Concatenation(pkgdir, "./configure"),
      InputTextUser(), OutputTextUser(),
      [Concatenation("--with-gaproot=", gapdir)]);
  if res <> 0 then return false; fi;
  res := Process( builddir, "make", InputTextUser(), OutputTextUser(), []);  
  return res <> 0;
end
...
)

Then GAP could test for the presence of such an entry; if available, happily use it; if not, then fall back to some kind of legacy support.

Now, in order to load the packages, the package also needs to use the right builddir. We can achieve this by modifying DirectoriesPackagePrograms to respect that builddir in addition to the actual package dir.

Finally, we could keep track of whether the package was compiled by inspecting its builddir; and/or by recording the result of its BuildPackage command in e.g. that builddir, resp. in a separate file whose name is derived from the builddir name.

As I mentioned, all this would also allow us to safely share a single copy of a GAP package with kernel extension among multiple GAP versions and configs.

CC @ThomasBreuer @alex-konovalov @ChrisJefferson

@ChrisJefferson
Copy link
Member

So, I feel there is three(ish) parts here.

  1. Unconditionally trying to build every package when it is first installed seems like a reasonable idea? I don't think there is any package where this is a bad thing to do. That would solve the immediate problem?

  2. Having an explicit way to check if a package is "loadable but not built" seems like a nice idea, and could just be another (optional) function in PackageInfo.g.

  3. Having a general way of having lots of versions of a gap package out-of-tree sounds fine, but a seperate and much bigger issue.

@mtorpey
Copy link
Collaborator

mtorpey commented Oct 14, 2020

We now have CompilePackage, a manual way of forcing a package to compile. So this is a partial solution to the problem.

The only remaining question in this issue that's in the scope of this package is possibly implementing Chris's suggestion above:

Unconditionally trying to build every package when it is first installed seems like a reasonable idea? I don't think there is any package where this is a bad thing to do.

This might be something to try in the future, so I'll leave this issue open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Not exactly an issue to be resolved, but a discussion about the future of the package
Projects
None yet
Development

No branches or pull requests

4 participants