Improving capture

[mahrud]

This is a continuation of #1469.

Currently many examples in the documentation of "Macaulay2Doc" are being generated using capture, however, there are still a number of remaining issues when using capture for generating examples, each one is being avoided by one of the lines below:

M2/M2/Macaulay2/m2/examples.m2

Lines 159 to 171 in 2d48ff6

	-- try capturing in the same process
	if not match("no-capture-flag", inputs) -- this flag is really necessary, but only sometimes
	-- TODO: remove this when the effects of capture on other packages is reviewed
	and match({"Macaulay2Doc"}, pkg#"pkgname")
	-- FIXME: these are workarounds to prevent bugs, in order of priority for being fixed:
	and not match("(gbTrace|read|run|stderr|stdio|print|<<)", inputs) -- stderr and prints are not handled correctly
	and not match("(notify|stopIfError|debuggingMode)", inputs) -- stopIfError and debuggingMode may be fixable
	and not match("([Cc]ommand|schedule|thread|Task)", inputs) -- remove when threads work more predictably
	and not match("(temporaryFileName)", inputs) -- this is sometimes bug prone
	and not match("(installMethod|load|export|newPackage)", inputs) -- exports may land in the package User
	and not match("(GlobalAssignHook|GlobalReleaseHook)", inputs) -- same as above
	and not match({"ThreadedGB", "RunExternalM2"}, pkg#"pkgname") -- TODO: eventually remove
	then (

I think it's fairly safe to use capture for other packages as well, but I think it would require a closer review before doing that for every package.

1. The string -* no-capture-flag *- is used to single out specific examples that aren't excluded by any other keyword. Currently, only the following examples have this tag:

• (Grassmannian, ZZ, ZZ) and "finite fields", because they use the option Variable => T in the following lines:

GF (ZZ/2[T]/(T^9+T+1), Variable => T) -* no-capture-flag *-",
J = Grassmannian(2,5, CoefficientRing => ZZ/31, Variable => T) -* no-capture-flag *-

. As it turns out, the variable "T" (and only this one!) is particularly annoying. See #1627 (comment)

• "new", because it includes this line which has a permanent effect by adding a new method:

new Type of BasicList from Function := (A,B,f) -> hashTable { net => f }; -* no-capture-flag *-

2. capture still doesn't capture direct printing to either stderr or stdout, hence why any keyword that affects those is currently excluded, like gbTrace, stderr, stdio, print, <<.

3. examples that use notify may be changed if a file is already loaded, so those are excluded, examples that use stopIfError and debuggingMode are also likely not going to work yet, unless there's some sort of time limit to interrupt capture.

4. Certain actions have a permanent effect: GlobalAssignHook, GlobalReleaseHook, installMethod

5. export, newPackage, read, run aren't well tested yet, the same with schedule, thread, *Task.

6. load in effect prevents any of the checks above from working, so it has to be excluded

7. temporaryFileName is buggy when the same process uses it repeatedly, so it is excluded, but can probably be fixed.

8. ThreadedGB and RunExternalM2 packages probably shouldn't use capture anyway.

TODO:

A few things listed above can be fixed, but to reiterate, I think the following are the important ones:

• fix capturing toplevel stderr (see e37f272 and 8ad88cd)
• fix capturing printing of nets (see 8f9aff6)
• fix capturing output directly from engine (printf, cerr, cout, etc.)
• let capture work in a re-initialized thread, with re-initialized thread local variables, etc.
• when that is done, run multiple examples in parallel
• use capture for check (see Speeding up running the tests #1708)
• let capture enforce time limits, memory limits, etc.
  • use capture for memory leak detection and memory benchmarking! (see Too many heap sections capturing check(8, "DiffAlg") on armhf #1728)
• make sure capture can be interrupted without exiting M2 (should debuggingMode and stopIfError be thread local?)

I already have made changes in capture to save and restore the values of all exported mutable symbols from all packages, but I'm not confident that this entirely eliminates the risk of capturing one example from affecting the result of future examples.

[pzinn]

unrelated, but can we make this line slightly more mode-agnostic:

new Type of BasicList from Function := (A,B,f) -> hashTable { net => f, html => f }; -* no-capture-flag *-

otherwise the example in the "new" help makes no sense when run in WebApp mode.

[mahrud]

@DanGrayson thanks for the help, this now works:

i8 : format last capture ///stderr << "hi" << endl///

o8 = "
     i1 : stderr << \"hi\" << endl
     hi

     o1 = stdio

     o1 : File

     i2 : 
     "

However, I noticed that in a lot of places in the interpreter, stderr is used rather than stdError, here is an example:

M2/M2/Macaulay2/d/stdiop.d

Lines 127 to 137 in 0205d20

	printMessage(position:Position,message:string):void := (
	if !SuppressErrors then (
	cleanscreen();
	stderr << position;
	if recursionDepth > 0 then stderr << "[" << recursionDepth << "]:";
	-- gettid() is not there in Solaris
	-- tid := gettid();
	-- if tid != -1 && tid != getpid() then stderr << "<" << gettid() << ">:";
	stderr << " " << message << endl;
	);
	);

Changing stderr -> stdError fixes capturing of errors:

i2 : format last capture "error 4"

o2 = "
     i1 : error 4
     currentString:1:1:(3):[4]: error: 4
     "

But I'm not sure if there's a more important reason why stderr is used here or not. In particular, here is where stderr is defined in the interpreter:

M2/M2/Macaulay2/d/errio.d

Lines 2 to 7 in 0205d20

	-- io routines for stderr that do not depend on global variables having been initialized,
	-- so they can be used early in the execution of a thread, e.g., by finalizers
	-- we queue up the strings so the entire message can be written with one write() system call
	use varstrin;
	export BasicFile := varstring or null; -- there can be only one of these, but that's enough for now
	export threadLocal stderr := BasicFile(null()); -- this is safe for other threads, because it is initially 0

Is it safe to replace them? I did so only for printMessage in 8ad88cd.

[DanGrayson]

I think the point of stderr is that it can be used while debugging the more complicated stdError, so your replacement of it seems fine.