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.

Sign up for GitHub

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

Make GAP.jl usable as a Julia module with precompilation #249

Merged
merged 1 commit into from
Jun 19, 2019
Merged

Make GAP.jl usable as a Julia module with precompilation #249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Project.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ authors = ["Thomas Breuer <[email protected]>", "Sebastian Gutsche <gutsch
version = "0.1.0"

[deps]
GAPTypes = "cd4130c4-8d21-11e9-2a29-61e008680c2d"
Libdl = "8f399da3-3557-5675-b5ff-fb832c97cbdb"
Pkg = "44cfe95a-1eb2-52ea-b672-e2afdf69b78f"
REPL = "3fa0cd96-eef1-5676-8a61-b3b8758bbffb"
Expand Down
14 changes: 13 additions & 1 deletion deps/build.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,23 @@ if install_gap
## TODO: We currently use the GAP master branch.
## Once all issues of using GAP with the julia
## GC are resolved, we switch to a stable version.
run(`git clone https://github.com/gap-system/gap`)
run(`git clone --depth=1 https://github.com/gap-system/gap`)
cd("gap")
run(`./autogen.sh`)
run(`./configure --with-gc=julia --with-julia=$(julia_binary)`)
run(`make -j$(Sys.CPU_THREADS)`)
## FIXME: Hack to make GAPTypes available in the global package environment:
## For a moment, we set the Julia load path to the actual depot, i.e.,
## the env GAP.jl is installed to, to install GAPTypes.jl globally.
## Since the package manager usually sets the load paths,
## JULIA_LOAD_PATH should always be set when this script is
## executed. However, just setting this environment variable
## just for the configure currently does not work.
julia_load_path_save = ENV["JULIA_LOAD_PATH"]
Copy link
Member

Choose a reason for hiding this comment

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

Is ENV["JULIA_LOAD_PATH"] guaranteed (by documentation / specification) to be set at this point? If it is not, then this line could throw an exception.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this build script is (or better should, nobody is keeping you from shooting yourself in the foot) only executed by the package manager, yes, I am sure this is always set.

pop!( ENV, "JULIA_LOAD_PATH" )
julia_executable = joinpath(julia_binary, "julia")
run(`$(julia_executable) -e 'import Pkg; Pkg.add("GAPTypes")'`)
Copy link
Member

Choose a reason for hiding this comment

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

Wow, that is a gross hack. I am also not quite sure why it is needed? Surely, GAP's configure also takes care of it? OK, perhaps the problem is that it inherits the JULIA_LOAD_PATH? But then at the very least, the comment above should explain so.

But I also wonder (very naively, so there is probably an easy answer why this won't work): couldn't we make the code in GAP more flexible, and ask it to first check if GAP.GAPTypes is available, then use that; if not, try to use GAPTypes directly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I am painfully aware that this is gross, and should not be done this way. On the other hand, the Julia package manager sets this environment variable, too, which is also not very nice of it.

Actually, patching GAP this way would be possible, although not for the 4.10.2 release I guess. But then we can at least remove that hack afterwards.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will leave one more comment. The problem is that for the configure script, this somehow failed to give me the right responses when we asked for flags. I have no real idea why yet. This solution is merely the result of a lot of try-and-error.

ENV["JULIA_LOAD_PATH"] = julia_load_path_save
gap_install_packages = get(ENV, "GAP_INSTALL_PACKAGES", "yes")
if gap_install_packages == "yes"
run(`make bootstrap-pkg-full`)
Expand Down
21 changes: 0 additions & 21 deletions pkg/GAPJulia/JuliaInterface/julia/libgap.jl
View file
Open in desktop

This file was deleted.

2 changes: 0 additions & 2 deletions pkg/GAPJulia/JuliaInterface/read.g
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ dirs_julia := DirectoriesPackageLibrary( "JuliaInterface", "julia" );

JuliaIncludeFile( Filename( dirs_julia, "gaptypes.jl" ) );

JuliaEvalString( Concatenation( "__JULIAGAPMODULE.include( \"", Filename( dirs_julia, "libgap.jl" ), "\")" ) );

_JULIAINTERFACE_INTERNAL_INIT();

## The GAP module is also bound to the variable __JULIAGAPMODULE,
Expand Down
20 changes: 20 additions & 0 deletions src/GAP.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ import Base: length, convert, finalize

import Libdl

import GAPTypes: GapObj

export GapObj

const Obj = Union{GapObj,FFE,Int64,Bool,Nothing}

sysinfo = missing

function read_sysinfo_gap(dir::String)
Expand Down Expand Up @@ -108,6 +114,20 @@ function __init__()
if ! isdefined(Main, :__GAPINTERNAL_LOADED_FROM_GAP) || Main.__GAPINTERNAL_LOADED_FROM_GAP != true
run_it(GAPROOT, error_handler_func)
end

## FIXME: Hack because abstract types cannot be endowed with methods in Julia 1.1.
## With Julia 1.2, this will be possible and this hack could then be replaced with the
## corresponding function call in in ccalls.jl (func::GapObj)(...)
MPtr = Base.MainInclude.eval(:(ForeignGAP.MPtr))
Base.MainInclude.eval(:(
(func::$MPtr)(args...; kwargs...) = GAP.call_gap_func(func, args...; kwargs...)
))
end

include( "ccalls.jl" )
include( "gap2.jl" )
fingolfin marked this conversation as resolved.
Show resolved Hide resolved
include( "macros.jl" )
include( "gap_to_julia.jl" )
include( "julia_to_gap.jl" )

end
21 changes: 12 additions & 9 deletions pkg/GAPJulia/JuliaInterface/julia/ccalls.jl → src/ccalls.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,21 @@ function AssignGlobalVariable(name::String, value::Any)
(Ptr{UInt8}, Ptr{Cvoid}), name, tmp)
end

function MakeString( val::String )::MPtr
function MakeString( val::String )::GapObj
string = ccall( :GAP_MakeString, Any,
( Ptr{UInt8}, ),
val )
return string
end

function CSTR_STRING( val::MPtr )::String
function CSTR_STRING( val::GapObj )::String
char_ptr = ccall( :GAP_CSTR_STRING, Ptr{UInt8},
( Any, ),
val )
return deepcopy(unsafe_string(char_ptr))
end

function UNSAFE_CSTR_STRING( val::MPtr )::Array{UInt8,1}
function UNSAFE_CSTR_STRING( val::GapObj )::Array{UInt8,1}
string_len = Int64( ccall( :GAP_LenString, Cuint,
( Any, ),
val ) )
Expand Down Expand Up @@ -91,7 +91,7 @@ function NEW_MACFLOAT(x::Float64)
return o
end

function ValueMacFloat(x::MPtr)
function ValueMacFloat(x::GapObj)
o = ccall( :GAP_ValueMacFloat,
Cdouble,
(Any,),
Expand All @@ -107,7 +107,7 @@ function CharWithValue(x::Cuchar)
return o
end

function ElmList(x::MPtr,position)
function ElmList(x::GapObj,position)
o = ccall( :GAP_ElmList,
Ptr{Cvoid},
(Any,Culong),
Expand All @@ -124,15 +124,18 @@ function NewJuliaFunc(x::Function)
end

"""
(func::MPtr)(args...)
(func::GapObj)(args...)

> This function makes it possible to call MPtr objects as functions.
> This function makes it possible to call GapObjs as functions.
> There is no argument number checking here, all checks on the arguments
> are done by GAP itself.
"""
(func::MPtr)(args...; kwargs...) = call_gap_func(func, args...; kwargs...)
# # The (func::GapObj) function (commented out below) needs to be instantiated for `MPtr`
# # and is therefore moved to the init function
# (func::GapObj)(args...; kwargs...) where T <: GapObj = call_gap_func(func, args...; kwargs...)

function call_gap_func(func::MPtr, args...; kwargs...)

function call_gap_func(func::GapObj, args...; kwargs...)
global Globals
options = false
if length(kwargs) > 0
Expand Down
48 changes: 24 additions & 24 deletions pkg/GAPJulia/JuliaInterface/julia/gap.jl → src/gap2.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,53 +1,53 @@
import Base: convert, getindex, setindex!, length, show

function Base.show( io::IO, obj::Union{MPtr,FFE} )
function Base.show( io::IO, obj::Union{GapObj,FFE} )
str = Globals.String( obj )
stri = CSTR_STRING( str )
print(io,"GAP: $stri")
end

function Base.string( obj::Union{MPtr,FFE} )
function Base.string( obj::Union{GapObj,FFE} )
str = Globals.String( obj )
return CSTR_STRING( str )
end

## implement indexing interface
Base.getindex(x::MPtr, i::Int64) = Globals.ELM_LIST(x, i)
Base.setindex!(x::MPtr, v::Any, i::Int64 ) = Globals.ASS_LIST( x, i, v )
Base.length(x::MPtr) = Globals.Length(x)
Base.firstindex(x::MPtr) = 1
Base.lastindex(x::MPtr) = Globals.Length(x)
Base.getindex(x::GapObj, i::Int64) = Globals.ELM_LIST(x, i)
Base.setindex!(x::GapObj, v::Any, i::Int64 ) = Globals.ASS_LIST( x, i, v )
Base.length(x::GapObj) = Globals.Length(x)
Base.firstindex(x::GapObj) = 1
Base.lastindex(x::GapObj) = Globals.Length(x)

# matrix
Base.getindex(x::MPtr, i::Int64, j::Int64) = Globals.ELM_LIST(x, i, j)
Base.setindex!(x::MPtr, v::Any, i::Int64, j::Int64) = Globals.ASS_LIST(x, i, j, v)
Base.getindex(x::GapObj, i::Int64, j::Int64) = Globals.ELM_LIST(x, i, j)
Base.setindex!(x::GapObj, v::Any, i::Int64, j::Int64) = Globals.ASS_LIST(x, i, j, v)

# records
RNamObj(f::Union{Symbol,Int64,AbstractString}) = Globals.RNamObj(MakeString(string(f)))
# note: we don't use Union{Symbol,Int64,AbstractString} below to avoid
# ambiguity between these methods and method `getproperty(x, f::Symbol)`
# from Julia's Base module
Base.getproperty(x::MPtr, f::Symbol) = Globals.ELM_REC(x, RNamObj(f))
Base.getproperty(x::MPtr, f::Union{AbstractString,Int64}) = Globals.ELM_REC(x, RNamObj(f))
Base.setproperty!(x::MPtr, f::Symbol, v) = Globals.ASS_REC(x, RNamObj(f), v)
Base.setproperty!(x::MPtr, f::Union{AbstractString,Int64}, v) = Globals.ASS_REC(x, RNamObj(f), v)
Base.getproperty(x::GapObj, f::Symbol) = Globals.ELM_REC(x, RNamObj(f))
Base.getproperty(x::GapObj, f::Union{AbstractString,Int64}) = Globals.ELM_REC(x, RNamObj(f))
Base.setproperty!(x::GapObj, f::Symbol, v) = Globals.ASS_REC(x, RNamObj(f), v)
Base.setproperty!(x::GapObj, f::Union{AbstractString,Int64}, v) = Globals.ASS_REC(x, RNamObj(f), v)

#
Base.zero(x::Union{MPtr,FFE}) = Globals.ZERO(x)
Base.one(x::Union{MPtr,FFE}) = Globals.ONE(x)
Base.:-(x::Union{MPtr,FFE}) = Globals.AINV(x)
Base.zero(x::Union{GapObj,FFE}) = Globals.ZERO(x)
Base.one(x::Union{GapObj,FFE}) = Globals.ONE(x)
Base.:-(x::Union{GapObj,FFE}) = Globals.AINV(x)

#
typecombinations = ((:MPtr,:MPtr),
typecombinations = ((:GapObj,:GapObj),
(:FFE,:FFE),
(:MPtr,:FFE),
(:FFE,:MPtr),
(:MPtr,:Int64),
(:Int64,:MPtr),
(:GapObj,:FFE),
(:FFE,:GapObj),
(:GapObj,:Int64),
(:Int64,:GapObj),
(:FFE,:Int64),
(:Int64,:FFE),
(:MPtr,:Bool),
(:Bool,:MPtr),
(:GapObj,:Bool),
(:Bool,:GapObj),
(:FFE,:Bool),
(:Bool,:FFE))
function_combinations = ((:+,:SUM),
Expand Down Expand Up @@ -131,7 +131,7 @@ end

export LoadPackageAndExposeGlobals

function Display(x::MPtr)
function Display(x::GapObj)
## FIXME: Get rid of this horrible hack
## once GAP offers a consistent
## DisplayString function
Expand Down
Loading