webrtc icon indicating copy to clipboard operation
webrtc copied to clipboard

Which Go compiler versions should Pion support?

Open adriancable opened this issue 1 year ago • 12 comments

This issue was 'inspired' by https://github.com/pion/transport/pull/200 created by @jech which has the effect of broadening the range of Go compiler versions that Pion can be built with. I felt the question of Go compiler support is best answered with substantial community input as it impacts past and future contributors to Pion, as well as third-party software that uses Pion as a dependency.

The core question is: which versions of the Go toolchain should Pion support? Right now there is no clear answer (i.e. nothing is stated explicitly in the docs), and the Pion codebase itself offers a number of contradictory answers:

  1. Pion's CI system tests builds only against 1.17 and 1.18. This is consistent with Go's own support for the current major version, plus the last version, of the toolchain. This means that, in practice, committed changes are only guaranteed to work on 1.17 and 1.18.
  2. go.mod in the main repo targets go 1.13. Some of the other packages e.g. pion/transport target go 1.12 in the go.mod.
  3. Recent changes e.g. inclusion of optimised assembly for crypto functions currently use syntax only available in 1.16 onwards.

The point was raised that the Go toolchain advances faster than some Linux distros are updated, so it is possible/likely that people are developing primary with Go toolchain releases that are now unsupported/EOL.

Some possible options:

  1. We synchronize Pion's support for the Go toolchain with Go's own support schedule, i.e. current version + last major version only. This is effectively what's done by Pion's current CI system, and so this 'change' is simply to state that 1.17 and 1.18 are the only supported Go toolchain releases. We can choose to enforce this in go.mod or not do so, and state that Pion may compile under earlier versions of Go but we do not test or support such usage.
  2. We make a decision to broaden support to e.g. current + last + last-but-one + last-but-two. If we do this we should change Pion's CI process to also build and test under 1.16 and 1.15. This might reveal breakages either due to Go features in use in the codebase that were not supported in 1.15/1.16, or due to compiler bugs whose fixes were not marked for backport. This might be fine, or might open a 'can of worms' - it is hard to tell.

What are people's views on this?

adriancable avatar Aug 05 '22 20:08 adriancable

Until recently, Pion has been using a system that apparently works for everyone:

  • people would submit improvements to Pion, without going out of their way to ensure it works with older copilers;
  • occasionally, somebody would notice that Pion no longer works on a version of Go that they still care about, and submit a fix;
  • occasionally, we'd remove obsolete code when we're reasonably sure that nobody cares any longer.

Here's an example of a perfectly friendly exchange fixing breakage on an older version: https://github.com/pion/ice/pull/361.

This system has been working well, and has allowed people who care about older compilers to use a recent version of Pion, which is good for the project since it gets timely testing. I don't understand why you have suddenly decided to replace it with something different, that is obviously causing pointless friction and that would cause me to stick to an older version of Pion.

jech avatar Aug 06 '22 12:08 jech

@jech - I'm not sure I understand.

I don't understand why you have suddenly decided to replace it with something different

This is exactly what I am not doing. Per my post in pion/transport I do not think that any single individual (including myself) should be making unilateral decisions on versioning policy, because these decisions potentially have wide community impact. That's why I am asking for broader views (beyond me and you), so that whatever the consensus is, we can codify it so there is no ambiguity, and both future contributors and Pion users know what compilers we support. I am not 'vetoing' your PR on broadening compiler support for a particular package (I don't have the ability to 'veto' anything), rather, I am using this as an opportunity to establish what our version support policy is, and write it down. If as a community we agree that our policy should be best-effort support for older compilers, for example, then I will merge the PR in a flash.

My point is not that anything done to date is wrong, rather, because we don't have anything written down on what this project's policy is on compiler support, there is the ongoing risk that two people disagree on what the policy actually is. What is happening now is a good example of what I want to avoid in the future. So, I am asking for broader views on what our policy should be, so we can write it down, and then there is no ambiguity. I am not 'replacing it with something different', and in fact I do not have any views on what the policy should be. I would hope this is clear from my above post when I presented a number of different options, without expressing any preference myself. My sole wish is that, as a community, we agree 'a view' and then write it down, so there is no ambiguity.

@Sean-Der, @davidzhao and others - I would appreciate your views.

adriancable avatar Aug 06 '22 17:08 adriancable

@adriancable I understand the wish to have an agreement as a community on what compilers Pion supports and I think that what @jech described above worked well in the past. One problem I see with this approach is that it is somewhat unclear when contributors are allowed to use new language features, which most of the time is not a big issue, but might be one soon, now that both of the last supported compiler versions support generics. I don't know how much they are actually used, so maybe that is nothing to worry about, but in the worst case, I think we may lose contributors by requiring them to implement patches without using new language features.

I think we should certainly avoid situations such as in pion/transport where the code is incompatible with what is declared in go.mod.

mengelbart avatar Aug 06 '22 21:08 mengelbart

@mengelbart - thanks for this - I don't have any issue with @jech's proposal for a system at all, but I think we (A) need to reconcile it with what happens on the contributor/CI side, which is currently that we only build and test for 1.17 and 1.18 (this may be 1.18 and 1.19 by now), and (B) write it down. My issue is not with any specific policy but with 'folklore' policies in general that are not written down. This just causes confusion, unless you have been following how things work at Pion for a long time. We shouldn't make 'learning folklore' a requirement to contribute to Pion.

I absolutely agree that code should not use language features that are not supported by what is declared in go.mod. (In the interest of transparency, I am guilty of this in at least one place in pion/transport.) My uncertainty is that there are 2 ways to fix this - remove the language features, or bump the version in go.mod. The former is what @jech would like and appears in line with the 'folklore' policy in effect. The latter would enable us to sync up the module versions with what we actually support, i.e. what the Pion CI system builds and tests against (1.17/1.18).

If we are happy with the policy that @jech described then please let's write it down! We also need to make it consistent with what the CI system does. Some possible wording we could add to the README that might achieve both things:

  • Pion aims to match Go's own policies on toolchain version support, that is, the current and previous major versions of the compiler. At the time of writing this means that Pion supports Go versions 1.18 and 1.17 (probably 1.19 and 1.18 by now). Pion's CI system builds and tests against these Go versions.
  • We recognize that older versions of Go are still in use by developers. We cannot guarantee compatibility with EOL versions of the Go toolchain, because we do not build or test against these compiler versions. However, if you do have an issue building or running with an EOL compiler version and have a fix for this, we are open to accepting PRs (provided that they do not break compatibility with supported versions of the toolchain).

Thoughts?

adriancable avatar Aug 06 '22 23:08 adriancable

Repeating parts of what I just wrote on pion/transport#200:

I'm running Galène (which depends on this project) as a production server on Debian stable which ships Go 1.15. Since this is a production environment I'd very much prefer to stick with the Go version it was shipped with (which has full security support) rather than using the one from backports (which may or may not get security updates).

If there's significant effort required to continue supporting a Go version for the lifetime of Debian stable (usually about 2 years) I can understand you'd prefer not to. I'll update to Go 1.18 from backports in that case. But at the very least the build should fail in a way that makes it obvious the reason for the failure is that I need a more recent Go version.

silbe avatar Sep 08 '22 09:09 silbe

Jumping into this late. Been thinking about this one for a bit.

I support having an official "version policy" around Go version support. My reasons are:

  • set clarity and expectations for contributors around compatibility expectations
  • stating what we won't support will reduce the perceived difficulty of contributing
  • it'd be hugely beneficial to adopt a version where newer features like generics could be used. we don't want Pion to be a project to be forever stuck on 1.15.

It's important to make the distinction that this is a developer-impacting decision that does not impact the end-users. As with all Go binaries, there are no dependencies on having a particular toolchain installed for end-users.

What are reasons for folks that develop with Pion to keep using Go 1.15/1.16?

davidzhao avatar Sep 11 '22 06:09 davidzhao

@davidzhao - one of the reasons I have heard is that Linux distros don't tend to update their Go toolchain as often as they could/should.

I'm not sure how much of a practical block this really is, though. The Go toolchain has been designed to be simple to install and update (copy and paste one command line into the terminal). @jech - do you know from Galene users why some people don't want to use more modern / supported versions of Go?

adriancable avatar Sep 11 '22 20:09 adriancable

I'm sorry, but I don't have either the time or the inclination to have this discussion.

In the meantime, however, compilation under gccgo is still broken. So please either apply pion/transport#200 or provide a different but tested fix yourself

jech avatar Sep 11 '22 22:09 jech

@jech - that's a shame. This discussion started, I believe, as a consequence of Galene users being frustrated by the inability to compile under older versions. If you 'drop out' at this point and we lose an understanding of this data point, it makes it harder to make good decisions (= more likely we will continue in this holding pattern for longer than we need to).

The gccgo compilation issue isn't (as I understand it) related to the Go version support discussion. Because I do not have or use gccgo, I can't provide a fix for this that I have tested, but I will gladly merge a PR submitted by anyone who does have the ability to test. (not pion/transport#200 as it stands, since that PR makes other changes not related to an uncontentious fix for gccgo)

adriancable avatar Sep 11 '22 22:09 adriancable

This discussion started, I believe, as a consequence of Galene users...

I'm pretty sure that's not the case. You started this discussion single-handed, thus creating a problem where there was none, and causing extra work for everyone.

jech avatar Sep 11 '22 22:09 jech

@jech - it's actually the opposite. By actually defining a toolchain support policy we will save people a lot of work in the future (e.g. writing code using generics, and then being told after the fact it isn't allowed and having to rewrite - or the opposite, e.g. writing code without generics when they could have saved development time by using generics). I acknowledge that this discussion requires effort from its participants in order to make a good decision for users and maintainers, but Pion like any open source project relies on that.

If anyone else feels that I am causing extra work for everyone, they haven't yet said so, so I will disregard that particular comment for the time being as it is probably inaccurate, but am always open to hear views from others.

adriancable avatar Sep 11 '22 23:09 adriancable

Based on where the project (all repos) is currently at, I feel like the next major version would be an appropriate point to have a toolchain adoption and obsolescence schedule. I say this because things are pretty mature at this point and it would be a shame to not be able to take improvements from new language versions. It also encourages new developers with go to contribute, especially if they're used to knowing "new" things recently introduced. Pion is a prime example of good and modern go engineering and I'd love to have that be easier to keep that way!

edaniels avatar Sep 06 '23 04:09 edaniels

This was discussed at the Pion quarterly meeting in Slack. Consensus was that 1.19 would be an acceptable upgrade

  • 1.19 is what Debian stable is at. This seems to be the platform where users are stuck on older versions
  • 1.19 does give us some features that people are excited for

@edaniels If you are interested want to make https://github.com/pion/.goassets/blob/master/scripts/lint-go-mod-version.sh fail the build. I can help make the PRs across all the repos if you like!

Happy to take the issue myself, but this seemed like something you were passionate about

Sean-Der avatar Mar 31 '24 03:03 Sean-Der

Yup I'll give it a stab. May need help but I'll lyk

edaniels avatar Mar 31 '24 16:03 edaniels

Done! go.mod version is 1.19 across all the repos.

Thank you so much for doing this @edaniels !

Sean-Der avatar Apr 03 '24 02:04 Sean-Der

Woo! Tag teamed!

edaniels avatar Apr 03 '24 03:04 edaniels