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

Fix GAP.Packages.install with a HACK #413

Merged
merged 3 commits into from
May 20, 2020

Conversation

fingolfin
Copy link
Member

@fingolfin fingolfin commented Apr 14, 2020

Fixes #452
Closes #467

@codecov
Copy link

codecov bot commented Apr 14, 2020

Codecov Report

Merging #413 into master will increase coverage by 0.22%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master     #413      +/-   ##
==========================================
+ Coverage   73.29%   73.52%   +0.22%     
==========================================
  Files          66       67       +1     
  Lines        5101     5099       -2     
==========================================
+ Hits         3739     3749      +10     
+ Misses       1362     1350      -12     
Impacted Files Coverage Δ
deps/build.jl 0.00% <ø> (ø)
test/convenience.jl 100.00% <ø> (ø)
src/packages.jl 79.41% <60.00%> (+27.89%) ⬆️
src/GAP.jl 62.79% <100.00%> (ø)
pkg/JuliaInterface/src/calls.h 0.00% <0.00%> (ø)

@fingolfin fingolfin force-pushed the mh/fix-Packages.install branch 3 times, most recently from 4049e2d to 31d0c4d Compare April 14, 2020 13:40
@fingolfin fingolfin changed the title WIP fixing GAP.Packages.install Fixing GAP.Packages.install Apr 15, 2020
@fingolfin fingolfin changed the title Fixing GAP.Packages.install Fix GAP.Packages.install with a HACK Apr 15, 2020
deps/build.jl Outdated Show resolved Hide resolved
@mohamed-barakat
Copy link
Contributor

The following failed:

julia> GAP.Packages.install("io")
#I  Getting PackageInfo URLs...
#I  Created directory /Users/mo/.julia/dev/GAP/deps/build/gap-4.11.0-julia-1.4.0/pkg/
#I  Retrieving PackageInfo.g from https://gap-packages.github.io/io/PackageInfo.g ...
#I  Downloading archive from URL https://github.com/gap-packages/io/releases/download/v4.7.0/io-4.7.0.tar.gz ...
#I  Saved archive to /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmJRWQuN/io-4.7.0.tar.gz
#I  Extracting to /Users/mo/.julia/dev/GAP/deps/build/gap-4.11.0-julia-1.4.0/pkg/io-4.7.0 ...
#I  Running compilation script on /Users/mo/.julia/dev/GAP/deps/build/gap-4.11.0-julia-1.4.0/pkg/io-4.7.0 ...
#I  Possible error detected: see log at /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmpS3y8c/exec-log.txt
#I  Compilation failed (package may still be usable)
#I  Package availability test failed
#I  (for IO 4.7.0)
#I  Removed directory /Users/mo/.julia/dev/GAP/deps/build/gap-4.11.0-julia-1.4.0/pkg/io-4.7.0
false
$ cat /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmpS3y8c/exec-log.txt
/bin/sh: /Users/mo/.julia/dev/GAP/deps/build/gap-4.11.0-julia-1.4.0/bin/BuildPackages.sh: No such file or directory

Copy link
Contributor

@mohamed-barakat mohamed-barakat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like very much the idea of being able to specify a path for compilation. I think when this works it will be safe to remove the nonessential packages from the artifact.

@fingolfin fingolfin force-pushed the mh/fix-Packages.install branch 4 times, most recently from b58e182 to b2d7a43 Compare May 7, 2020 00:59
@mohamed-barakat
Copy link
Contributor

Thank you for supporting the optional argument pkgdir :)

We manipulate some internal state of the GAP PackageManager to make it
use our custom copy of BuildPackage.sh. This should be dropped once
there is a newer PackageManager version.
@fingolfin
Copy link
Member Author

@mohamed-barakat tests for this finally pass. Perhaps you can to try it again?

For CI, instead invoke GAP.Packages.install
@mohamed-barakat
Copy link
Contributor

Compiling io succeeded but compiling dgraphs failed.

julia> GAP.Packages.install("io")
#I  Getting PackageInfo URLs...
#I  Retrieving PackageInfo.g from https://gap-packages.github.io/io/PackageInfo.g ...
#I  Downloading archive from URL https://github.com/gap-packages/io/releases/download/v4.7.0/io-4.7.0.tar.gz ...
#I  Saved archive to /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmuuTeUm/io-4.7.0.tar.gz
#I  Extracting to /Users/mo/.gap/pkg/io-4.7.0 ...
true

julia> GAP.Packages.install("io")
#I  Getting PackageInfo URLs...
#I  Retrieving PackageInfo.g from https://gap-packages.github.io/io/PackageInfo.g ...
#I  The newest version of package "io" is already installed
true

julia> GAP.Packages.install("digraphs")
#I  Getting PackageInfo URLs...
#I  Retrieving PackageInfo.g from https://gap-packages.github.io/Digraphs/PackageInfo.g ...
#I  Downloading archive from URL https://github.com/gap-packages/Digraphs/releases/download/v1.2.0/digraphs-1.2.0.tar.gz ...
#I  Saved archive to /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmZNitPP/digraphs-1.2.0.tar.gz
#I  Extracting to /Users/mo/.gap/pkg/digraphs-1.2.0 ...
#I  Checking dependencies for Digraphs...
#I    io >=4.5.1: true
#I    orb >=4.8.2: true
#I    datastructures >=0.2.5: false
#I  Retrieving PackageInfo.g from https://gap-packages.github.io/datastructures/PackageInfo.g ...
#I  Installing dependency datastructures 0.2.5 ...
#I  Downloading archive from URL https://github.com/gap-packages/datastructures/releases/download/v0.2.5/datastructures-0.2.5.tar.gz ...
#I  Saved archive to /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmBbHLyj/datastructures-0.2.5.tar.gz
#I  Extracting to /Users/mo/.gap/pkg/datastructures-0.2.5 ...
#I  Checking dependencies for datastructures...
#I    GAPDoc 1.5: true
#I  Running compilation script on /Users/mo/.gap/pkg/datastructures-0.2.5 ...
#I  Possible error detected: see log at /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmZMOECA/exec-log.txt
#I  Compilation failed (package may still be usable)
#I  Checking dependencies for datastructures...
#I    GAPDoc 1.5: true
#I  Package availability test failed
#I  (for datastructures 0.2.5)
#I  Removed directory /Users/mo/.gap/pkg/datastructures-0.2.5
#I  Dependencies not satisfied for digraphs-1.2.0
#I  Removed directory /Users/mo/.gap/pkg/digraphs-1.2.0
false

@fingolfin
Copy link
Member Author

So what does the log at /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmZMOECA/exec-log.txt say?

@mohamed-barakat
Copy link
Contributor

Dump of /var/folders/z7/br6dwhjj5xn8691ygcnbzxc40000gs/T//tmZMOECA/exec-log.txt:

Using GAP root /Users/mo/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0
ERROR: /Users/mo/.julia/artifacts/848f0f318405266896d24d11e4baf5cf7bf3f203/gap-4.11.0 is not the root of a gap installation (no sysinfo.gap)
ERROR: Please provide the absolute path of your GAP root directory as
ERROR: first argument with '--with-gaproot=' to this script.

@fingolfin
Copy link
Member Author

That's odd: the given path is into the artifact and indeed not the correct GAPROOT. But the main point of this PR is to solve this kind of problem, by (a) using a newer version of BuildPackages.sh.

I tried replicating this, but failed. In particular, it works fine on my Mac, on plesken and in a ubuntu docker container (which I just had running for your issue #465 anyway).

@mohamed-barakat Could you please provide some details about the system where the issue appears?

@fingolfin
Copy link
Member Author

@mohamed-barakat I am going to merge this anyway, as it seems to improve lots of things and doesn't make anything worse. If you can still reproduce the digraphs problems afterwards, please open an issue for it (with some details on the system where it occurs), then I can work on fixing it in a new PR.

@fingolfin fingolfin merged commit f8e0477 into oscar-system:master May 20, 2020
@fingolfin fingolfin deleted the mh/fix-Packages.install branch May 20, 2020 09:41
@mohamed-barakat
Copy link
Contributor

I was probably doing something wrong. It is now working.

@mohamed-barakat
Copy link
Contributor

Now that GAP.Packages.install is working I would like to start using it in HomalgProject.jl. Is it possible to make a new minor release?

mohamed-barakat added a commit to mohamed-barakat/HomalgProject.jl that referenced this pull request May 20, 2020
Now that oscar-system/GAP.jl#413 has been merged
start using `GAP.Packages.install`.

Closes homalg-project#22
mohamed-barakat added a commit to mohamed-barakat/HomalgProject.jl that referenced this pull request May 20, 2020
Now that oscar-system/GAP.jl#413 has been merged
start using `GAP.Packages.install`.

Closes homalg-project#22
@fingolfin
Copy link
Member Author

@mohamed-barakat: done, GAP.jl 0.4.1 is tagged and should be available soon

@mohamed-barakat
Copy link
Contributor

Wonderful, thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Travis: compiling profiling package fails due to missing autoconf
2 participants