Use make install #17
Use make install #17
Conversation
This is a first step towards getting the formula approved for use in homebrew proper
Co-authored-by: Max Horn <[email protected]>
|
I've tried this PR, and while it builds, I get Some other packages, e.g. Also, shouldn't |
|
This PR seems like a great improvement, so it would be a shame to let this go to waste. |
|
@gvol if you are still interested in this, I could just give you write access, so you don't have to wait for me to even notice you have done work or submitted a PR. Also @pedritomelenas has offered to help out, I'd be happy to give him as well |
|
|
||
|
|
||
| no_compilation_packages = [ | ||
| "atlasrep", "aclib", "agt", "alnuth", "automata", "automgrp", |
There was a problem hiding this comment.
Such a long list seems error prone and annoying because it needs to be updated for each GAP release.
Can't you instead just compute a list of the names of all subdirs of the pkg directory on the fly? Resp. not even use that list, just iterate over them all
|
|
||
| # The makefiles appear to only be used for docs... | ||
| # The BuildPackages.sh script didn't call them | ||
| all_packages.each do |pkg| |
There was a problem hiding this comment.
So here one could instead iterate over all subdirs (I am sure there is a ruby way to do that)
| if File.exist?("./prerequisites.sh") | ||
| system "./prerequisites.sh", "#{libexec}/gap/lib/gap" | ||
| end | ||
| end |
There was a problem hiding this comment.
Instead of having the configure_packages and configure_packages lists, here you could have a similar check to see if there is a ./configure file. If there is, you can distinguish "old" and "new" style by checking of configure contains the string with-gaproot...
Though actually, with GAP 4.13.0, all but one configure script support --with-gaproot -- the one exception is that of simpcomp, which however happily ignores it. So you could just ditch the whole old_configure_packages part.
|
I am leaving this in here a little bit more, to give @gvol time to react, if he likes to. But otherwise we can perhaps merge this next week, and then perhaps 4.13.1 is out and someone can update this here. The suggestions I just made of course don't have to be addressed in this PR right away, though I hope they are actually not that hard to realize. |
|
Thanks! |
|
@pedritomelenas I gave you access @gvol I'll give you access if you let me know Anyone else interested in contributing: reach out to me and we'll figure something out I guess an immediate job would be to merge this PR; then perhaps perform the changes I suggest above (or to record those change suggestions in an issue or two). And also to update this to GAP 4.13.1 which is about to be released in a few hours at most. |
|
@pedritomelenas @gvol ping |
This switches from building in place to using
make installto install to the Cellar. This should fix #14. Please let me know if I installed the packages to the wrong location or something.I took the approach of listing every package explicitly, and whether it used the new or old style configure. It seemed like that was consensus on how it should work, but I'm happy to change it to work more like
BuildPackages.sh.It's built on top of #16 because I didn't want to have to rebase everything. If that's not desired, then I can get rid of the
--HEADoption, I don't have too strong of feelings.I'm a little worried that I didn't explicitly require all the dependencies I should have, but I just didn't notice because I already had them installed on my machine.
Two packages just don't work yet, namely
xgapandcddinterface.I have lightly tested the installation and it seems sane, though some of the singular packages fail their tests due to some minor differences (perhaps my singular is older or new than the one used when writing the tests).
Closes #16
Resolves #15
Resolves #14
Resolves #1