go1.15: go build -v fails unless -tags PSIPHON_DISABLE_QUIC is passed

[bassosimone]

(We have now completely mitigated this issue by building atop the documented workaround.)

This is the output I obtain when compiling using Go 1.15:

% ./miniooni -h
panic: qtls.ConnectionState not compatible with tls.ConnectionState

goroutine 1 [running]:
github.com/Psiphon-Labs/quic-go/internal/handshake.init.1()
	/Users/sbs/go/pkg/mod/github.com/!psiphon-!labs/quic-go@v0.14.1-0.20200306193310-474e74c89fab/internal/handshake/unsafe.go:17 +0x12e

The reason seems to be that Go 1.15 changed its tls.ConnectionState structure. There is a fix applied upstream that handles this issue. It seems Psiphon is using its own fork of quic-go: https://github.com/Psiphon-Labs/quic-go.

This means that, for fixing the issue, the Psiphon devs need to backport the patch. For now, it's important to use Go 1.14 only for building OONI.

[bassosimone]

Documented workaround

Psiphon devs just recommended this workaround (which also works for me): go build -a -v -tags "DISABLE_QUIC" which has been confirmed to be working for them when using Psiphon and Go 1.15. Many thanks!

[cyBerta]

I adapted the build script accordingly. Is it worth a PR?

[bassosimone]

Who's building using go build -v ./... is gonna get a broken binary anyway, unless the proper tag is passed from command line. Because of this reasoning, I believe fixing the script to conditionally disable QUIC when developing using Go 1.15 is not fully solving the problem, because we're producing a broken binary for who's not using the build script.

How about: we create one/two files in the top-level package that only conditionally build if either we're in Go 1.14 or the user has provided the DISABLE_QUIC tag? With this solution, the build breaks. This is arguably better than breaking the binary, which is something that is definitely going to be much more surprising for the user?

What do you think?

[cyBerta]

When I started to figure out how to build the project, the first thing I did was to have a look at the build script to see the necessary flags etc.. But I'm fine with adding a file that crashes the build on compile time if preconditions are not met. I'll propose something in a PR, together with the changes for the build script.

EDIT: This compile time hack should be undone as soon as soon as the psyphon folks have updated their fork.

[bassosimone]

EDIT: This compile time hack should be undone as soon as soon as the psyphon folks have updated their fork.

Yes, indeed. Ideally, the tag should only be added to the compilation when using Go 1.15, while Go 1.14 binaries should continue to be compiled without it. So the only functional change is better user experiences for folks on Go 1.15.

[bassosimone]

So, this is where we are at now. When using Go 1.15 and compiling, you'll get:

# go build -v ./cmd/miniooni/
github.com/ooni/probe-engine
# github.com/ooni/probe-engine
./readme_compiletimecheck.go:6:1: syntax error: non-declaration statement outside function body

and ./readme_compiletimecheck.go contains:

# cat readme_compiletimecheck.go 
//+build !DISABLE_QUIC
//+build go1.15

package engine

ATTENTION: If you are compiling probe-engine with go1.15 please make sure
to pass -tags DISABLE_QUIC. Alternatively use ./build-cli.sh.

Also building with -tags DISABLE_QUIC works (as it used to work before). Also, when you are using ./build-cli.sh the code will also emit a warning pointing you to this issue:

# ./build-cli.sh linux
Warning: disabling QUIC when using Go 1.15.
See https://github.com/ooni/probe-engine/issues/866 for more info.
+ export 'GOOS=linux' 'GOARCH=amd64'
+ go build -o ./CLI/linux/amd64 -tags DISABLE_QUIC,netgo '-ldflags=-s -w -extldflags "-static"' ./cmd/miniooni

We will still keep it open and pinned. People compiling with Go 1.15 will still see an error unless they do not apply the workaround when building. Also, the name of the issue can now be changed.

[bassosimone]

Changed issue title since now both the latest stable release (v0.19.0) and master use PSIPHON_DISABLE_QUIC

[bassosimone]

So, after #747, it seems we always need to add -tags PSIPHON_DISABLE_QUIC to use ooni/probe-engine. I am now starting to realise why this is painful for users. So, I tried to come up with another strategy that allows us to build without any flag at #1022. Currently, the PR is still in progress and quite rough around the edges; we have a TODO list to finalise it #1022 (comment). Also, of course, this issue is now high prio because we cannot release w/o fixing this issue.

[bassosimone]

This issue has been solved by #1022.

[bassosimone]

We can close now

[bassosimone]

Reopening until we have a stable version for which this is fixed!

[bassosimone]

Moving to next Sprint where we'll publish a release fixing this issue.

[bassosimone]

No need to keep this in Sprint 26, since it's mentioned at #1004. Removing from this milestone.

[bassosimone]

This is fixed in v0.20.0