Hi Robin,

That seems reasonable, I accept.

On Mon, Feb 20, 2023 at 18:59:01 +0100, Robin Jarry wrote:
> Could you please respond to this message stating whether you accept or
> refuse the change of license from MIT to GPL[2] for the aerc project.
> This will only affect future versions of aerc. Existing releases will
> remain with their original license (MIT).

First, use wineboot instead of winecfg to set up the WINEPREFIX. It's
the right tool for it.

Second, when initialising WINEPREFIX, use output pipes instead of
CombinedOutput. The latter will wait for *all* output to be copied to a
buffer, including the output from grandchildren processes. Since wine
starts wineserver automatically, and wineserver lingers for three
seconds by default, this is bad. We would waste three seconds waiting
for wineserver doing nothing, and then the next wine call (to start the
app) would need to start wineserver all over.

Instead, with pipes we can get cmd.Run to only wait for the parent
process to finish. wineserver stays running but we don't care. And, when
we start the gio app, we very likely reuse the same wineserver process.
[message trimmed]

Fix a long-standing TODO: instead of each sub-test handling its own
output separately, just make each expose its output via an io.Reader.
Then, the shared driverBase code can tell if any of the lines contain
the magic "gio frame ready" string.

Reduces the amount of code a bit, but most importantly, it keeps the "is
a frame ready?" logic in a single place.

In the future, this also enables us to do more with all the e2e test app
output consistently. For example, we might want to add a -debug flag to
always log output lines as they happen.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
---
[message trimmed]

A simple 'go test -modfile=<path>' won't work properly for the
end-to-end tests, since they run 'go build' under the hood as well.

To properly propagate the flag, we need to use GOFLAGS. Since -modfile
is always relative to the current directory, we can't use it to test
many packages at once, nor can we use it via 'go test ./gogio'.

While at it, document this distinction in go.local.mod to prevent others
getting confused like I did.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
---
 .builds/linux.yml | 3 ++-
 cmd/go.local.mod  | 6 ++++++
[message trimmed]

This way, if the user has a custom winecfg, it can't possibly affect the
tests. I was encountering this as DXVK does not work on virtual Xorg
servers (which we use), and Gio thus failed to render on such a
combination.

From the numbers below, it can be seen that setting up a new WINEPREFIX
takes roughly five seconds:

	$ rm -rf ~/.cache/gio-e2e-wine
	$ go test -run EndToEnd/Windows
	PASS
	ok  	gioui.org/cmd/gogio	16.369s
	$ go test -run EndToEnd/Windows
	PASS
[message trimmed]

This way, a Gio app's logs can be filtered uniquely, which wasn't
possible before since the tag "gio" would be the same between gio apps.

Since the app ID is supplied at build time, inject it via a variable
with the linker's help. The variable is only used on Android for now,
but that's OK. It might be useful for other platforms or other internal
packages in the future.

Fixes #84.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
---
 app/internal/log/log.go         | 2 ++
 app/internal/log/log_android.go | 2 +-
[message trimmed]

It's essentially a copy of go.mod, but with the gioui.org module replaed
with the parent directory. Useful when wanting to try a change in the
root module as part of a gogio invocation or end-to-end test, such as
either of the following:

	$ cd cmd; go test -modfile=go.local.mod ./...
	$ cd cmd/gogio; go build -modfile=../go.local.mod

Since go.sum can essentially be shared, and since it seems to work with
the go tool, use a symlink. The way the -modfile flag works, if given
foo.mod, it will use the sum file at foo.sum. The only caveat is that
'go mod tidy -modfile=go.mod.local' will try to remove gioui.org lines,
since that module is replaced. So tidy shouldn't be used with -modfile.
[message trimmed]

Installing it on Debian was enough, with the only wrinkle that
propagating -race won't work when we're cross-compiling, since
cross-compilation disables CGo by default.

For now, just skip the test in that edge case. If we want to use the
race detector on Windows in the future, we need to get a Windows CI
builder somehow.

Tested on my fork; see https://builds.sr.ht/~mvdan/job/164899.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
---
 .builds/linux.yml         | 1 +
 cmd/gogio/windows_test.go | 5 +++++
[message trimmed]

Since Wine is heavily tied to X11, we build its end-to-end test driver
on top of X11's. We use the same mechanism to start an X server, take
screenshots, and issue clicks.

Its only quirk is that it was difficult to get the screenshots to line
up with Gio's window. The comments cover what we ended up with. The
display dimensions are now part of driverBase, so that methods other
than Start can also use them - this is necessary for the wine driver to
crop screenshots.

We also use a sleep for now; a comment explains why, and a TODO is left
for future Dan to deal with. What we have now works, and I've spent
enough hours on this patch as it is.
[message trimmed]

We were using 'go run . <args>' before, which works fine, but does mean
re-linking a new binary and throwing it away at each invocation. Given
that the end-to-end tests don't do all that much work besides building
the tiny red.go app, this amount of extra work was noticeable.

We can obtain statistics for the JS sub-test, which used 'go run', via
the perflock and benchcmd tools:

	$ go test -c
	$ perflock -governor=70% benchcmd EndToEnd/JS ./gogio.test -test.run=EndToEnd/JS

After capturing those numbers before and after the change, we can then
compare them with benchstat. The CPU cost of the subtest is halved:
[message trimmed]