Make it possible to build GAP as a dynamic library #1205
Conversation
markuspf
added
do not merge
| .PHONY: all | ||
|
|
||
| libgap: libgap.la | ||
| .PHONY: libgap |
There was a problem hiding this comment.
This duplicates what is already there in the lines below.
| @@ -12,7 +12,9 @@ | |||
| # Default rule: build gap | |||
| ######################################################################## | |||
| all: gap$(EXEEXT) gac | |||
| .PHONY: all | |||
| @@ -115,6 +116,9 @@ ifeq ($(HPCGAP),yes) | |||
| SOURCES += src/hpc/traverse.c | |||
| endif | |||
|
|
|||
| ifeq ($(BUILD_LIBGAP),yes) | |||
| SOURCES += src/sage_interface.c | |||
| endif | |||
There was a problem hiding this comment.
Hmm, perhaps distinguish between BUILD_LIBGAP and BUILD_SAGEINTERFACE ?
| @@ -1002,3 +1006,13 @@ doc/make_doc: $(srcdir)/doc/make_doc.in config.status | |||
|
|
|||
| gac: $(srcdir)/cnf/gac.in config.status | |||
| $(SHELL) ./config.status $@ | |||
|
|
|||
| # TODO: This needs to be done more elegantly | |||
There was a problem hiding this comment.
You probably plan to clean this up before the PR is merged. But if not, then this comment needs to be expanded, to explain what "this" is and what the TODO is.
| typedef void (* TJumpToCatchFunc)(); | ||
| extern TJumpToCatchFunc JumpToCatchFunc; | ||
| static inline void SetJumpToCatchFunc(TJumpToCatchFunc func) | ||
| { JumpToCatchFunc = func; }; |
There was a problem hiding this comment.
That an odd code formatting style...
Also, why does this setter even exist, if JumpToCatchFunc is a global anyway?
| extern TExtraMarkFuncBags ExtraMarkFuncBags; | ||
|
|
||
| static void SetExtraMarkFuncBags(TExtraMarkFuncBags func) | ||
| { ExtraMarkFuncBags = func; } |
There was a problem hiding this comment.
Same as above: Odd non-GAPish code formatting; and what's the point of a setter if wee don't hide the global variable anyway?
src/iostream.c
Outdated
| /* Set up the trap to detect future dying children */ | ||
| signal( SIGCHLD, ChildStatusChanged ); | ||
| #endif |
There was a problem hiding this comment.
As mentioned before (on the old PR): This #ifdef should be replaced by an if (!disableSigChildHandler) { ... or something like that, so that the same libgap.so can be used by GAP, and by Sage.
src/sage_interface.c
Outdated
|
|
||
| void libgap_finalize() | ||
| { | ||
| FinishBags(); |
There was a problem hiding this comment.
Please consider running clang-format on this PR. :-)
| */ | ||
|
|
||
| #ifndef LIBGAP__H | ||
| #define LIBGAP__H |
There was a problem hiding this comment.
Why the double underscore? Also, this does not match the current filename anymore.
Codecov Report
@@ Coverage Diff @@
## master #1205 +/- ##
==========================================
- Coverage 64.35% 62.57% -1.78%
==========================================
Files 1002 885 -117
Lines 326699 254474 -72225
Branches 13218 12031 -1187
==========================================
- Hits 210244 159236 -51008
+ Misses 113585 92369 -21216
+ Partials 2870 2869 -1
|
This is mainly done to avoid confusion over what "libgap" is or should be. The plan is to have an API to use GAP as a library in other programs, and to achieve this we start out from libgap as provided by Volker Braun and work towards an abstract interface that will be useful to Sage as well as other projects.
This allows the installation of a callback function when JUMP_TO_CATCH is called. This was added in an effort to make GAP suitable for being used as a shared library.
There were some hardcoded paths that are now subsituted appropriately.
markuspf
force-pushed
the
gap-library
branch
from
3036a22
to
65fbb55
Compare
|
Do you know whether this will be merged in GAP 4.9.0? |
|
This PR will definitely not be part of GAP 4.9.x, for any x .(note that GAP 4.9.0 is the GAP beta releas that was released a few months ago; the first stable release will be 4.9.1, due to be released soon.) This might go into GAP 4.10 (which I hope will be out this fall), but only if someone invests work into it, e.g. @markuspf. But for now, it seems to have been stale for several more months now. |
|
Closing this: parts of this have already been merged; for the rest, @markuspf and @sebasguts have been working on a different branch, and submitted incremental PRs (and hopefully will keep doing so) |
DO NOT MERGE YET.
This is a prototype of building GAP as a dynamically loadable shared library, a rebased copy of fingolfin#64, which I decided to pull over here for re-centralisation after the merge of #1193.
This code is based on what SageMath calls "libgap". In the interest of avoiding misunderstandings, I have renamed the files that were called "libgap" to "sage_interface", since currently this is what it is: An API that is tailored to SageMath's needs.
I am working on doing some refactoring and cleanup that will make it possible to compile GAP as a shared library and link it to any binary. Ideally, this proof of concept will implement GAP's REPL as a separate program around this shared library.
@nthiery has made a prototype integration into SageMath based around this work which works, and we are working on being able to use GAP kernel modules together with GAP as a shared library.
Be aware that I will probably be rebasing and rewriting history on this branch.