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.

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

chore: upgrade dependencies (1/N) #1530

Merged
merged 4 commits into from
Mar 25, 2024
Merged

chore: upgrade dependencies (1/N) #1530

merged 4 commits into from
Mar 25, 2024

Conversation

bassosimone
Copy link
Contributor

@bassosimone bassosimone commented Mar 25, 2024

This diff starts upgrading dependencies using gomajor.

We still have other dependencies to upgrade, including, at least:

  • quic-go/quic-go;

  • snowflake.

We already know that quic-go>=0.41.0 requires at least Go 1.21. We also know that snowflake most likely requires us to pull some tricks to make it live together with Psiphon. For this reason, I am not upgrading either of them now.

Part of ooni/probe#2690.

This diff starts upgrading dependencies using gomajor.

We still have other dependencies to upgrade, including, at least:

- quic-go/quic-go, which should be relatively easy;

- snowflake, which is hard and we already know.

Part of ooni/probe#2690.
@bassosimone
Copy link
Contributor Author

Okay, it seems we have a bit of a test-failure fest. It seems the reason why tests fail with go1.20 is different from the way in which tests fail for go1.21 and go1.22. The latter failures seem more fundamental. The failure for go1.20, instead, looks like either a fluke or something that changed in miekg/dns causing some DNS results to be different. Time to investigate!

@bassosimone
Copy link
Contributor Author

Alright, it seems go1.21 and go1.22 now want one to upgrade the go.mod and have a toolchain directive. When trying to run go1.21.8 with the current status of the tree, we get an error that is like "please, run go mod tidy". If we do that, the code adds the toolchain directive and then using go1.20 fails saying the toolchain directive is not recognised. At this point, I am left wondering why it was working before. Maybe I should try to revert changes in this repository in some selective way and understand why the code was working as intended before. Let's try and understand this!

@bassosimone
Copy link
Contributor Author

Okay. It is confirmed that upgrading dependencies causes inconsistencies. We need to handpick the correct dependencies that do not require go >= 1.21 in their go.mod file, otherwise this would cause the aforementioned issues.

These are the candidates causing us upgrading issues:

  • github.com/rubenv/sql-migrate
  • github.com/prometheus/common

@bassosimone
Copy link
Contributor Author

I need to further understand and investigate, but I suspect the toolchain directive may be a problem in going forward, because it would clash with our policy of using forked Go libraries. So, I am not even 100% sure that we can guarantee we are using exactly the correct version of Go we would like to use when building ooniprobe. 😓

@bassosimone bassosimone merged commit 2f8ff17 into master Mar 25, 2024
82 of 84 checks passed
@bassosimone bassosimone deleted the fullbuild branch March 25, 2024 11:36
@bassosimone bassosimone mentioned this pull request Mar 25, 2024
36 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant