Convert buildsystem to use Makefile.gappkg instead of automake

[fingolfin]

Not done yet, but I wanted to show what I have here, in case someone wants to help, or in case I get sidetracked and forget about it, so it won't be lost forever

[james-d-mitchell]

@fingolfin is there an analogue of AM_DEFAULT_VERBOSITY=1 when using this set up, to display the exact call to make that's being made by the build sys?

[james-d-mitchell]

Seems like at the moment this isn't picking up required information from libsemigroups itself. In particular, it isn't picking up the include paths -Ilibsemigroups/extern -Ilibsemigroups/extern/fmt-8.1.1/include -Ilibsemigroups/extern/eigen-3.3.9/ and even if I add these manually in line 41 of Makefile.in, then compilation is successful but linking fails because the sources of libsemigroups are not compiled. This was previously handled by SUBDIRS = libsemigroups in Makefile.am, I'm not sure if there's an equivalent to this here?

Also previously, libsemigroups was make installed into the bin directory, and I noticed that if bin/include/libsemigroups/* are present with the new setup, then everything compiles just fine (i.e. the remnants of a previous build in bin are used).

[fingolfin]

@fingolfin is there an analogue of AM_DEFAULT_VERBOSITY=1 when using this set up, to display the exact call to make that's being made by the build sys?

Yes, you can do make V=1 to do that (same as with the GAP build system, and also automake supports that)

[fingolfin]

This seems to be working now, though of course the CI tests fail until the recent fixes on the stable-5.2 branch get backported to main.

Also, I did not try all that much beyond a standard build on macOS and Linux. No OpenBSD, no attempt to use a preinstalled libsemigroups, etc.

[james-d-mitchell]

Thanks @fingolfin any chance you could rebase this onto main? I just tried to do this myself, but I couldn't push to your fork for some reason.

[fingolfin]

Rebased

(I don't know why you can't push, "Allow edits and access to secrets by maintainers" is enabled... huh)

[fingolfin]

Hmm, the cygwin test still fails.

Moreover, I'd feel more comfortable if there was a CI test that verified building against an installed version of libsemigroups. I can try to whip one up (but not tonight :-)).

[james-d-mitchell]

Thanks @fingolfin, I'll try to add a job that tests with an external libsemigroups this morning

[james-d-mitchell]

@fingolfin there are now checks for external libsemigroups in the CI in the main branch, if you rebase, these will run. Thanks again for all of your effort here, much appreciated!

[fingolfin]

Seems with-external-libsemigroups worked.

The cygwin error is a bit weird:

Error, LOAD_DYN: failed to load kernel module /tmp/gaproot/pkg/Semigroups//bin/x86_64\
-pc-cygwin-default64-kv8/semigroups.so, No such file or directory in
  LOAD_DYN( filename ) at /home/runneradmin/gap/lib/files.gd:594 called from

But it definitely built that file... I wonder if the error is just misleading and the real problem is that it can't load libsemigroups. I'll investigate later.

[fingolfin]

I tried to debug this using https://github.com/mxschmitt/action-tmate but no luck: in that shell, I can access the files from the current step, but I was not able to access (or even find) GAP itself, so I could not perform any tests.

[fingolfin]

The problem was that the libsemigroups DLL must be copied to the directory in which gap.exe resides. The old build system did just that. I added code to replicate this in the new system, and it worked in a test commit! I've now tweaked this a bit and squashed it all down in one commit. If it still works, this is good to go from my perspective.

[fingolfin]

This is a bit of a hack, but the old code didn't do it any better, I think

[fingolfin]

This is the old code

[fingolfin]

It's really weird: this now builds fine on Cygwin, as far as I can tell, and indeed a bunch of CI test pass just fine on Windows... Until this error, which seems to hint at a failure to fork:

testing: /tmp/gaproot/pkg/Semigroups/tst/standard/tools/display.tst
    1000 ms (532 ms GC) and 25.4MB allocated for tools/display.tst
testing: /tmp/gaproot/pkg/Semigroups/tst/standard/tools/io.tst
      0 [main] gap (6628) C:\cygwin64\home\runneradmin\gap\.libs\gap.exe: *** fatal error in forked process - MEM_COMMIT failed, Win32 error 1455
  18811 [main] gap (6628) cygwin_exception::open_stackdumpfile: Dumping stack trace to gap.exe.stackdump
      0 [main] gap 915 dofork: child -1 - forked process 6628 died unexpectedly, retry 0, exit code 0x100, errno 11
######## > Diff in:
/tmp/gaproot/pkg/Semigroups/tst/standard/tools/io.tst:186
# Input is:
ReadMultiplicationTable(name, 3);
# Expected output:
[ [ 1, 1, 3, 4, 5, 6, 7, 8, 9, 1 ], [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 ], 
  [ 3, 3, 4, 5, 6, 7, 8, 9, 1, 3 ], [ 4, 4, 5, 6, 7, 8, 9, 1, 3, 4 ], 
  [ 5, 5, 6, 7, 8, 9, 1, 3, 4, 5 ], [ 6, 6, 7, 8, 9, 1, 3, 4, 5, 6 ], 
  [ 7, 7, 8, 9, 1, 3, 4, 5, 6, 7 ], [ 8, 8, 9, 1, 3, 4, 5, 6, 7, 8 ], 
  [ 9, 9, 1, 3, 4, 5, 6, 7, 8, 9 ], [ 1, 10, 3, 4, 5, 6, 7, 8, 9, 2 ] ]
# But found:
Error, could not open the file "/tmp/gaproot/pkg/Semigroups//data/tst/tables.gz"
########
      0 [main] gap (5920) C:\cygwin64\home\runneradmin\gap\.libs\gap.exe: *** fatal error in forked process - MEM_COMMIT failed, Win32 error 1455
   3549 [main] gap (5920) cygwin_exception::open_stackdumpfile: Dumping stack trace to gap.exe.stackdump
8761052 [main] gap 915 dofork: child -1 - forked process 5920 died unexpectedly, retry 0, exit code 0x100, errno 11

[fingolfin]

This might actually suggest a problem with gac on cygwin :-(. I am now comparing the linker flags used by the old automake based build system with those used by cygwin, and I see quite some difference. Automake passes these additional flags:

• -nostdlib /usr/lib/gcc/x86_64-pc-cygwin/11/crtbeginS.o
  • is this really necessary? basic examples work after all, just fork does. Perhaps it is only for compatibility non-Cygwin gcc?
• -lgcc_s -lgcc -lcygwin -ladvapi32 -lshell32 -luser32 -lkernel32 -lgcc_s -lgcc
  • I guess some of these may be needed due to -nostdlib? But this is still puzzling
• /usr/lib/gcc/x86_64-pc-cygwin/11/crtend.o
  • also related to -nostdlib
• /cygdrive/d/a/Semigroups/Semigroups/libsemigroups/../bin/lib/libsemigroups.dll.a
  • so it seems it seems to statically link libsemigroups?! Interesting
• -Wl,--enable-auto-image-base -Xlinker --out-implib -Xlinker .libs/semigroups.dll.a
  • these look interesting

I'll try and see if adding some combination of these resolves the issues (and if so, we can use them here, but also upstream them to GAP resp. gac).
Of course testing is slow because each Cygwin build takes > 30 minutes sigh

[james-d-mitchell]

I'm not sure the failures in the Cygwin ci job are related to this pr, there have been similar failures before, and I added some @if statements to the test file to just not run those tests on windows.

[fingolfin]

Adding --enable-auto-image-base seems to have helped (which makes some vague sense, considering the issue was fork related)

[james-d-mitchell]

Of course testing is slow because each Cygwin build takes > 30 minutes sigh

continued feelings of regret...

[fingolfin]

This seems to be working now.

[james-d-mitchell]

I'm going to take a closer look tomorrow and then merge, thanks @fingolfin

[james-d-mitchell]

Thanks very much @fingolfin, I very much appreciate all the effort that went into this PR.