search

conda-forge / gap-feedstock

A conda-smithy repository for gap.
BSD 3-Clause "New" or "Revised" License
2 stars 9 forks source link

Update to GAP 4.10.1 #16

Closed james-d-mitchell closed 5 years ago

james-d-mitchell commented 5 years ago

Checklist

TODO

conda-forge-linter commented 5 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

james-d-mitchell commented 5 years ago

@isuruf @saraedum I think I've taken this as far as I can for the time being, do you have any insights into why the CI is failing?

saraedum commented 5 years ago

The problem could be that src/ is a symlink but we do

mkdir -p "$INSTALL_DIR"/src

and then

cp -R * "$INSTALL_DIR"

which tries to replace the directory "$INSTALL_DIR"/src with a symlink which is not supported.

So, maybe just drop that mkdir line?

james-d-mitchell commented 5 years ago

Thanks for the suggestion, let's see if it works.

olexandr-konovalov commented 5 years ago

Apparently it doesn't :(

isuruf commented 5 years ago

Does gap need its source at runtime? gap/gap-4.9.3/bin/x86_64-pc-linux-gnu-default64/src is a symlink to the source that conda is not pacakging. That's the issue here.

markuspf commented 5 years ago

None of the contents of bin/ are needed to run GAP, neither is src/. If you want to compile kernel modules you need src/.

isuruf commented 5 years ago

Okay. then they should be installed.

Which of the following should be installed?

markuspf commented 5 years ago

I am not sure why you want to make any selection.

Currently GAP comes as a fairly self-contained directory structure, and before make install is implemented I don't see a point in making any selection, really (and if you make one, then it'd be better to implement make install).

isuruf commented 5 years ago

True. I guess we can only delete the obj folder, which I don't want in the final distribution.

olexandr-konovalov commented 5 years ago

There should be further changes made, for example at: https://github.com/conda-forge/gap-feedstock/blob/1dad07e2599730d436bb51add340abf1c0bdaf8c/recipe/build.sh#L31-L34

First off, in GAP 4.4 there are no small prim trans directories which became PrimGrp, SmallGrp and TransGrp packages. Second, these packages are now needed to run GAP for backwards compatibility. So one should not remove them from the distribution.

Second, would it be possible to have a full installation of GAP provided this way, together with all packages? This was the way like it worked in e.g. Homebrew, and there is an intention to resubmit it to homebrew-core in the same way.

olexandr-konovalov commented 5 years ago

Also, SmallGrp now is licensed under Artistic License 2.0: https://github.com/gap-packages/smallgrp/blob/master/LICENSE and one could consider restoring these tests: https://github.com/conda-forge/gap-feedstock/blob/1dad07e2599730d436bb51add340abf1c0bdaf8c/recipe/build.sh#L77-L80

isuruf commented 5 years ago

Second, would it be possible to have a full installation of GAP provided this way, together with all packages?

Sure. Since they are different packages, I'd like to have separate conda packages gap-primgrp, gap-smallgrp, gap-transgrp and rename this package to gap-core and have a gap package that depends on gap-core, gap-primgrp, gap-smallgrp, gap-transgrp. What do you think?

isuruf commented 5 years ago

Second, these packages are now needed to run GAP for backwards compatibility. So one should not remove them from the distribution.

Ah, didn't see that.

olexandr-konovalov commented 5 years ago

First of all, in this case there should be gap-gapdoc as well for the sake of consistency. All these four packages are needed to run GAP, and setup for them should be the same.

But my suggestion was actually different - I suggest to now to remove packages at all. This will ensure that the user experience is the same whenever they install GAP from source or from conda (and my hope is that eventually to run GAP using Jupyter interface on Windows with https://github.com/gap-packages/JupyterKernel we will recommend to install Python with anaconda and then add GAP with conda).

isuruf commented 5 years ago

What about other packages? What do you recommend?

isuruf commented 5 years ago

Here are some issues with not separating packages.

  1. There are optional gap packages that takes up space. These are optional and for eg we don't want the sage download to be bigger than necessary.

  2. Gap packages are maintained independently and therefore separating them into packages will make it possible to install a newer version of a package than what came with the gap distribution. (Maybe this is not desirable?)

  3. Some packages like float have to be compiled and they have extra dependencies. So, they are better off as separate packages.

olexandr-konovalov commented 5 years ago

I actually recommend all of them. The reason is that we run GAP regression tests in three modes:

Some of the packages extend GAP e.g. by adding new methods for existing operations, or by improving significantly its performance for some calculations, so not having them may lead to errors or slow performance. Furthermore, even if packages are not usable (e.g. Float compilation failed) one can search in its documentation using ? and ?? commands and learn about available functionality.

While I was writing this, a new comment came, thanks for explaining! So,

There are optional gap packages that takes up space. These are optional and for eg we don't want the sage download to be bigger than necessary.

I was not following this repository from the start - so was the idea to provide it as a dependency of SageMath (i.e. with packages needed by SageMath) or as an alternative installation for GAP? I think it would be nice to have the latter opportunity.

Gap packages are maintained independently and therefore separating them into packages will make it possible to install a newer version of a package than what came with the gap distribution. (Maybe this is not desirable?)

If package authors publish a release, it will be picked up by the tools that I run, and will be checked by a set of regression tests. If they are approved and will made into the next GAP release, their pages will be updated on the GAP website. I am not sure that you want to dive into all dependencies between packages, and pick up for conda packages that may not pass our regression tests - I suggest that a more robust approach would be to use approved versions of packages. Moreover, package authors have a tendency to not to update dependencies carefully - sometimes there are packages stating GAP 4.5 as a minimum, but requiring features from a later GAP release...

Some packages like float have to be compiled and they have extra dependencies. So, they are better off as separate packages.

I may think of a handful of packages like that (mainly those mentioned in https://github.com/gap-system/gap-docker-base/blob/master/Dockerfile), but not more.

isuruf commented 5 years ago

I was not following this repository from the start - so was the idea to provide it as a dependency of SageMath (i.e. with packages needed by SageMath) or as an alternative installation for GAP? I think it would be nice to have the latter opportunity.

I'd like it to be both. That's why I mentioned splitting up the distribution, so that sagemath can use gap-core and the other required packages while a user can install gap and have all the packages.

olexandr-konovalov commented 5 years ago

P.S. Actually, we provide these having in mind providers of alternative GAP installers:

Will this make things easier for conda GAP installer?

isuruf commented 5 years ago

Thanks. Do you have a summary of which packages need external libraries and what external libraries are needed per package?

olexandr-konovalov commented 5 years ago

@isuruf yes - will try to give more details by the end of the week.

isuruf commented 5 years ago

Are all the packages GPLv2+ compatible?

conda-forge-linter commented 5 years ago

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

isuruf commented 5 years ago

@conda-forge-admin, rerender

conda-forge-linter commented 5 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

isuruf commented 5 years ago

There are 2 packages now

  1. gap-core package has gap and smallgrp, transgrp, primgrp, gapdoc
  2. gap package has all the 145 gap-packages except kbmag, cohomolo, guava, example, ace, fplsa, xgap, anupq, polymakeinterface. All these gap-packages are built (including the ones with external packages), so the user doesn't have to build them.

@alex-konovalov, @markuspf, @sebasguts when the PRs I've mentioned above are merged and makes it into a new release, I'll package them as well.

isuruf commented 5 years ago

Thanks @james-d-mitchell for the PR and the helpful discussions

olexandr-konovalov commented 5 years ago

@isuruf thanks, great to see this finally merged! GAP 4.10.2 is coming shortly by the way.

isuruf commented 5 years ago

@alex-konovalov, I've just merged the gap 4.10.2 PR. Would be good to get reviews on gap-packages/kbmag#24, gap-packages/cohomolo#21, gap-packages/example#26, gap-packages/ace#18, gap-packages/FPLSA#8 before the next release.

dimpase commented 3 years ago

IMHO disabling of most of the packages in https://github.com/conda-forge/gap-feedstock/blob/19ff5a3e3365bd1ef9ed9bd9315f0596d9e7de21/recipe/build.sh#L44-L48 can end. Things like kbmag, cohomolo build fine on recent clangs and gcc (after updates carried out since 2019).