Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cli, engine: use go1.21 #2548

Closed
bassosimone opened this issue Oct 4, 2023 · 4 comments · Fixed by ooni/probe-cli#1424
Closed

cli, engine: use go1.21 #2548

bassosimone opened this issue Oct 4, 2023 · 4 comments · Fixed by ooni/probe-cli#1424
Assignees
@bassosimone
Labels
ooni/probe-engine priority/medium technical task technical tasks e.g. deployment user feedback requests that have been added to the backlog as a direct result of user feedback or testing

Comments

@bassosimone
Copy link
Member

bassosimone commented Oct 4, 2023 •
edited

This issue captures #2524 (comment) feature request by @jefferyto.
@bassosimone bassosimone added the releaseBlocker This issue blocks releasing label Oct 4, 2023
@bassosimone bassosimone self-assigned this Oct 4, 2023
@bassosimone bassosimone changed the title cli, engine: try to use go1.21 cli, engine: use go1.21 Oct 4, 2023
@bassosimone
Copy link
Member Author

bassosimone commented Oct 4, 2023 •
edited

It seems it's not yet possible:

% ~/go/bin/go1.21.1 build -v ./...
github.com/Psiphon-Labs/quic-go/internal/qtls
# github.com/Psiphon-Labs/quic-go/internal/qtls
vendor/github.com/Psiphon-Labs/quic-go/internal/qtls/go121.go:5:13: cannot use "The version of quic-go you're using can't be built on Go 1.21 yet. For more details, please see https://github.com/Psiphon-Labs/quic-go/wiki/quic-go-and-Go-versions." (untyped string constant "The version of quic-go you're using can't be built on Go 1.21 yet. F...) as int value in variable declaration

We'll cut 3.19 and 3.20 using go1.20 and 3.21 will hopefully use go1.21.

Luckily, after go1.21 the quic-go library does not need this kind of strict binding with a go version anymore. We still need to bind to a specific Go version, but that would definitely be easier to do.

@bassosimone bassosimone added priority/medium technical task technical tasks e.g. deployment ooni/probe-engine user feedback requests that have been added to the backlog as a direct result of user feedback or testing and removed releaseBlocker This issue blocks releasing labels Oct 4, 2023
@bassosimone
Copy link
Member Author

bassosimone commented Oct 6, 2023 •
edited

I understand from Psiphon developers that their migration to a Psiphon-Labs/quic-go version supporting go1.21 would take time, because changes to make quic-go use the stdlib for crypto from now on conflict with Psiphon changes to support circumvention they applied in their own fork.

I also investigated what it would take to support go1.21 in our tree by disabling Psiphon and creating a compatibility layer that disables our own stdlib forks of net/http and crypto/tls when using go1.21. I don't want to go down this path, because it would create several subtle behavior differences with the version of ooniprobe using our net/http and crypto/tls forks, which would possibly cause headaches when processing and interpreting measurements. (Not to mention my headache in making sure the version w/o our forks is WAI and even defining what WAI means here is a bit of work.)

Because Psiphon is eventually going to migrate to a version of their quic-go fork supporting go1.21, and because after that migration we would not have build issues, and because we tend to stick using the latest version of Go (chiefly because I am a Go enthusiast and always want to use the latest version), I think it's fair to wait in this case, knowing this is probably the last time in which a version of ooniprobe ships after a version of Go and does not support it (well, hopefully, and I don't want to self-jynx it here 😅 🤞).

@jefferyto jefferyto mentioned this issue Oct 13, 2023
Merged
@bassosimone
Copy link
Member Author

See also #2585.

@bassosimone bassosimone mentioned this issue Oct 31, 2023
Closed
@chenrui333 chenrui333 mentioned this issue Dec 4, 2023
Merged
@bassosimone bassosimone mentioned this issue Dec 13, 2023
Merged
@bassosimone
Copy link
Member Author

bassosimone commented Dec 13, 2023 •
edited

I will soon merge ooni/probe-cli#1424 and enable building with go1.21. The price to pay is that psiphon will be disabled when building in this configuration. I don't think there's much else I can do here, TBH. 🤷

bassosimone added a commit to ooni/probe-cli that referenced this issue Dec 13, 2023
@bassosimone
feat: allow go1.21 builds (#1424)
f8c6189 
We can ~safely compile our net/http and crypto/tls forks with a later
version of Go, because they use public packages in the standard library,
which should be covered by the Go 1.x stability guarantee. Whether that
is 100% safe, I don't know, but I suppose we should be fine unless
there's really something subtle. (We will continue building our packages
with the correct version of Go, but I also understand how distribution
maintainers have other needs.)

The real price to pay for compiling with go1.21 is disabling Psiphon.
There isn't much we can do here. It's not possible to have a build
configuration with fixed flags that disable GQUIC for Psiphon, so I have
two choices (a) either ooniprobe does not build for go1.21 because of
Psiphon and I need to tell people to apply a flag; or (b) when you
compile with go1.21, everything is WAI but for Psiphon. I chose the
latter in this pull request.

Note that I originally tried to make oohttp and oocrypto optional.
However, it turns out making oohttp optional and using net/http instead
cripples ooniprobe entirely. This is why I concluded that maybe
compiling go1.20 code using the go1.21 compiler and stdlib is not that
bad, considering that our stdlib forks do not (obviously) depend on
internals.

Part of ooni/probe#2556; closes
ooni/probe#2548.
Murphy-OrangeMud pushed a commit to Murphy-OrangeMud/probe-cli that referenced this issue Feb 13, 2024
@bassosimone @Murphy-OrangeMud
feat: allow go1.21 builds (ooni#1424)
bf7a9df 
We can ~safely compile our net/http and crypto/tls forks with a later
version of Go, because they use public packages in the standard library,
which should be covered by the Go 1.x stability guarantee. Whether that
is 100% safe, I don't know, but I suppose we should be fine unless
there's really something subtle. (We will continue building our packages
with the correct version of Go, but I also understand how distribution
maintainers have other needs.)

The real price to pay for compiling with go1.21 is disabling Psiphon.
There isn't much we can do here. It's not possible to have a build
configuration with fixed flags that disable GQUIC for Psiphon, so I have
two choices (a) either ooniprobe does not build for go1.21 because of
Psiphon and I need to tell people to apply a flag; or (b) when you
compile with go1.21, everything is WAI but for Psiphon. I chose the
latter in this pull request.

Note that I originally tried to make oohttp and oocrypto optional.
However, it turns out making oohttp optional and using net/http instead
cripples ooniprobe entirely. This is why I concluded that maybe
compiling go1.20 code using the go1.21 compiler and stdlib is not that
bad, considering that our stdlib forks do not (obviously) depend on
internals.

Part of ooni/probe#2556; closes
ooni/probe#2548.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
ooni/probe-engine priority/medium technical task technical tasks e.g. deployment user feedback requests that have been added to the backlog as a direct result of user feedback or testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: allow go1.21 builds ooni/probe-cli
1 participant
@bassosimone