Add a Cygwin job to the GitHub Actions CI

[wilfwilson]

See gap-packages/io#97 and #90.

This still uses the non-canonical versions of the gap-actions actions, so it's not what we want to use long term. But for now it could help to get #90 resolved.

[codecov]

Codecov Report

Merging #91 (5df6143) into master (7cfe928) will not change coverage.
The diff coverage is n/a.

❗ Current head 5df6143 differs from pull request most recent head 08c900f. Consider uploading reports for the commit 08c900f to get more accurate results

@@           Coverage Diff           @@
##           master      #91   +/-   ##
=======================================
  Coverage   68.42%   68.42%           
=======================================
  Files          15       15           
  Lines         966      966           
=======================================
  Hits          661      661           
  Misses        305      305           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7369f1d...08c900f. Read the comment docs.

[fingolfin]

Thanks! But here it actually builds fine under cygwin, huh. I wonder if the build steps differ (in the GH Action, we are currently not using BuildPackages.sh, so ...)

[wilfwilson]

Yeah, maybe. Perhaps also relevant, as noted on #90, we're building the release archive of v2.4.1 in the GAP CI, whereas here we're building the development version. Not that there's any significant differences in the source, but maybe wrapping the archive introduced a problem.

[wilfwilson]

As far as I am concerned, this PR is now complete and ready to be merged - except for the fact that one of the test files is failing in the Cygwin job - perhaps @ChrisJefferson has some insight?

Architecture: x86_64-pc-cygwin-default64-kv8

testing: /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling/tst/tstall/pr\
ofilefile.tst
########> Diff in /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling\
/tst/tstall/profilefile.tst:12
# Input is:
str := ProfileFile(quicktest, rec(showOutput := false));;
# Expected output:
# But found:
Error, Unable to open file /tmp/gaptempdirBj9vAt/raw.json
########
########> Diff in /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling\
/tst/tstall/profilefile.tst:13
# Input is:
EndsWith(str, ".html");
# Expected output:
true
# But found:
Error, Variable: 'str' must have a value
########
########> Diff in /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling\
/tst/tstall/profilefile.tst:15
# Input is:
str := ProfileFile(runquicktest);;
# Expected output:
# But found:
Error, Unable to open file /tmp/gaptempdirufig9d/raw.json
########
########> Diff in /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling\
/tst/tstall/profilefile.tst:16
# Input is:
EndsWith(str, ".html");
# Expected output:
true
# But found:
Error, Variable: 'str' must have a value
########
########> Diff in /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling\
/tst/tstall/profilefile.tst:20
# Input is:
str := ProfilePackage("transgrp", rec(indir := "", showOutput := false));;
# Expected output:
# But found:
Error, Unable to open file /tmp/gaptempdirW63OIM/raw.json
########
########> Diff in /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling\
/tst/tstall/profilefile.tst:21
# Input is:
EndsWith(str, ".html");
# Expected output:
true
# But found:
Error, Variable: 'str' must have a value
########
      15 ms (0 ms GC) and 94.9KB allocated for /cygdrive/d/a/profiling/profili\
ng/gaproot/pkg/profiling/tst/tstall/profilefile.tst
-----------------------------------
total        15 ms (0 ms GC) and 94.9KB allocated
              6 failures in 1 of 1 files

Test failed: /cygdrive/d/a/profiling/profiling/gaproot/pkg/profiling/tst/tstal\
l/profilefile.tst

[wilfwilson]

In the first instance, this is the error that is being triggering:

profiling/src/profiling.cc

Lines 294 to 302 in f55808f

	if(!(IS_STRING(filename))) {
	ErrorMayQuit("Filename must be a string", 0, 0);
	}
	Obj filenamestr = CopyToStringRep(filename);
	Stream infile(CSTR_STRING(filenamestr));
	if(infile.fail()) {
	ErrorMayQuit("Unable to open file %s", (Int)CSTR_STRING(filenamestr), 0);
	return Fail;
	}

called from:

profiling/gap/profiling.gi

Lines 8 to 16 in f55808f

	InstallGlobalFunction( "ReadLineByLineProfile",
	function(filename)
	local res, stacks;
	if IsLineByLineProfileActive() then
	Info(InfoWarning, 1, "Reading Profile while still generating it!");
	fi;
	res := READ_PROFILE_FROM_STREAM(UserHomeExpand(filename), 0);
	return res;
	end );

called from:

profiling/gap/profiling.gi

Lines 1001 to 1027 in f55808f

	# Gather data
	rawfile := Filename(opts.outdir, "raw.json");
	if opts.showOutput = true then
	redirect := "";
	else
	redirect := "> /dev/null 2>&1";
	fi;
	len := Length(testfile);
	gap_cmd := GAPInfo.KernelInfo.COMMAND_LINE[1];
	if testfile{[len-3 .. len]} = ".tst" then
	cmd := StringFormatted("""
	gapinput="Test(\"{}\"); quit;"
	{} --quitonbreak -a 500M -m 500M -A -q --cover {} {} <<EOF
	$gapinput
	EOF
	""", testfile, gap_cmd, rawfile, redirect);
	else
	cmd := StringFormatted("""
	{} --quitonbreak -a 500M -m 500M -A -q --cover {} {} {} <<EOF
	quit; quit;
	EOF
	""", gap_cmd, rawfile, testfile, redirect);
	fi;
	Exec(cmd);
	# Process profile
	x := ReadLineByLineProfile(rawfile);;

[fingolfin]

I've added a HACK commit to inspect some values, and see this:

DEBUG: rawfile = /tmp/gaptempdirMa13Rc/raw.json
DEBUG: UserHomeExpand(rawfile) = /tmp/gaptempdirMa13Rc/raw.json
DEBUG: IsExistingFile(rawfile) = false
DEBUG: IsExistingFile(UserHomeExpand(rawfile)) = false
Error, Unable to open file /tmp/gaptempdirMa13Rc/raw.json

I think the Exec(cmd); call is problematic. It works by invoking a shell, and has windows specific code. If we rewrote that code to use Process, I think it would actually become easier to read and debug, would be less fragile and allow us to do fancy things like check the error code of the subprocess and report suitable error messages... :-)

[wilfwilson]

Do different GAP sessions in Cygwin have access to different temporary directories? Perhaps there is some disagreement here because the different copies of GAP, and an ability to access each other's files.

[wilfwilson]

All seems to be alright now, and I was talking to @ChrisJefferson earlier and he said if skipping the test file worked, then he's happy. So I'll merge.