-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Update go.mod to specify Go 1.19 as base #4330
Conversation
I've had to remove GopherJS from our tests as they don't support anything over Go 1.17 in their stable releases. Their beta release has 1.18 support but there seems to be nothing happening with newer versions. I see no other option than removing support for it if we want to keep using modern Go versions... |
That make it a bigger move. Currently wasm support is not a released one/ stable. Gopherjs is maintained by one person so far, so slow change. Wasm is currently being focused toward edge computing. Not to sure what's the best option here. |
It is tough. We can't stay on 1.17 forever and there are a bunch of changes in Go 1.19 that we would benefit from. I don't think we have a choice honestly |
Can we support the web concept with only WASM? It seems to be where people are putting their time and attention... |
I would like to think so and presume that there is a reason that gopherjs isn't very popular. My suggestion would be to leave the gopherjs code in for now (just tweaking |
But I suppose @Bluebugs has more opinions on the matter? |
It seems that GopherJS is moving to 1.19 soon: gopherjs/gopherjs#1140 Generics are nearly to be supported. I don't have any opinion... WASM works for me 😃 but GopherJS is, it seems, a bit more efficient. |
Yeah, I read that as well. That was four months ago but no update since. While I do like to see that they are moving to 1.19, we are on Go 1.21 now and 1.22 is approaching soon. What's to say that we won't be held up by GooherJS next time we update our base as well? PS: No offence to the people developing GooherJS. I don't doubt that it takes a lot of effort to maintain and they've done a fine job thus far. Just thinking about our project and our use case here :) |
The future is likely wasm, but we are not there yet. With the latest go release it start to go into a good direction. Now, the likely future is that wasm will drop support for the "syscall/js" package which is source of most of the slowness. This drop will break our compatibility with go wasm at that point (go wasm has no stable API). I would expect this change to happen in not a too far future, maybe a year from now (this will require a lot of work in all our layer that support wasm to have it working again). On the other hand, gopherjs is slowly keeping up with go and is currently faster. It's API is also stable. So at some point, I expect our wasm port to be completely broken by a go release. And I hope that gopherjs by then would have catch up with the minimum version so that we can rely on it for that below broke the world release. |
Hmm. That sounds no good at all. Sounds like we'll be stuck on Go 1.17 (or maybe Go 1.18 if GooherJS releases a new stable version) for some time then :/ |
I've thought some more about this and I came to wonder how many people are going to install an older version of Go just to compile and run their application? It seems to me like most people just use what is the latest and/or what their Linux/BSD distribution provides by default. There is a big difference between the case of supporting older Go versions and the case of not supporting new versions. I think we need to discuss this on an upcoming contributor meeting and come to come sort of agreement. |
I would agree with this statement - but without this update are we really not supporting new Go versions? |
I meant on the web support side of things. With GopherJS, anyone that wants to use it would have to always download an older Go version that neither receives bug fixes nor security fixes. I don't think a lot of people will want to install Go 1.17 just for serving with GopherJS support; especially not on Linux/BSD where people seem to mostly use what their package manager provides. We are trading a niece use case for the inability to use features in new Go versions. |
I don't know what to do with this ticket. The GopherJS support is in to provide support for some browsers where the WASM does not work well. We could decide to take it out so there are fewer supported browsers, or we could choose to keep it in for a tougher developer experience. As far as I can see there is nothing we are doing that actually requires go 1.19 so it seems like we don't have to make this trade-off at this time... Perhaps when we do have to there will be more info on supported versions or the non-WASM browsers will be far less popular? |
That’s the idea yes, but I doubt that many people are actually going to manually download Go 1.17 (which is far behind on security fixes among other things) just to be able to support older browser versions.
That’s intentional. The actual changes to use newer APIs was intentionally not in this PR to keep it smaller and easier to review. There are a bunch of nice new APIs that I want to use in newer Go versions. The atomic variables in 1.19 would be wonderful. |
You don't need to. I'm the same way I don't have an environment for windows compilation, I can rely on fyne-cross for this. |
That solves the need to download Go 1.17 yes, but it doesn't change the fact that now are running binaries that are years behind on security updates (Go 1.17 is 2.5 years old and stopped reviving security updates 1.5 years ago). As the application developer, I'm basically responsible for putting my users at risk there in my opinion. While at the same time it is stopping adoption of newer APIs not just for Fyne but also for the project using Fyne with GooherJS. |
That's not quite how things are - you don't have to compile with old Go just for GopherJS to work. Yes gopherJS projects won't be able to use newer APIs, but given we were on 1.14 until recently is that really a big deal? Hopefully it gets resolved soon of course, but I don't see the need for hurry. |
From the call today we discussed GopherJS and version requirements - along with our plans for web release etc. Given that the web driver is not nearly ready, and that many of the dependencies are still in beta we had to come to the conclusion that release was not coming soon. And given this browser support will move on before we get there. So the proposal is that we explore the concept that "Web driver, when released, will target only WASM" and see where that takes us. Obviously it will mean less supported platforms, but by the time release it may be OK. And it takes the pressure of maintaining more code in the meantime... |
Cool. Sounds like a very sensible conclusion from the discussion. I'm sorry that I couldn't attend. Will fix the conflicts and get this PR into shape |
I figured we can drop GopherJS in a future PR and also implement other Go 1.19 changes later on as well. |
The web test requirement will have to be lifted for this to pass. I can revert back if you'd like |
Not quite sure I follow - if you mean should it go back to testing on previous versions just for web then I guess so, as you say dealing with GopherJS would be a follow-up |
I mean the requirement for the web test to pass. I updated it to test on latest Go as well so the action has a new name and no longer matches the workflow requirement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the web - I cannot see it successfully building web in actions to replace the old web_tests action, am I missing something?
.github/workflows/web_tests.yml
Outdated
@@ -6,16 +6,19 @@ permissions: | |||
jobs: | |||
web_tests: | |||
runs-on: ubuntu-latest | |||
runs-on: ${{ matrix.os }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks wrong - the matrix.os is not defined but above it is hard-coded to ubuntu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right. Good catch
Isn't that Windows 1.19 failure what we were seeing with 1.17 before? |
Yes, of course. The fix is in 1.20 so we’re going to see it for a while more unless I patch it to run Windows on 1.20 instead. |
Oh, I thought this was the test fix, sorry. |
I figured I'd merge this first and work on that later as it wasn't directly related. This one is just about raising the go.mod version |
Ok, I guess I've been thinking about other things alongside this. What benefit does raising the version in go.mod if it's not for test fixes or using new features? |
Sure, I reverted the update of fredbi/uri (that requires Go 1.19) so we could compile with Go 1.17 still and leave the web workflow intact (like you wanted) until the upcoming PR that removes GopherJS but that doesn't mean that we aren't going to use any new features from Go 1.19 going forward. Like loosely implied in the README, I didn't want to bunch it all up in a single PR and instead opted to divide it up into multiple more easily-reviewed PRs (cleaning up code, switching to the new safer and faster atomic APIs, etc.). As soon as this is merged, I plan to open new PRs and as such I'm kind of hesitant to get this to land soon. |
The following PRs are basically going to try to implement the checkboxes in #3242 separated into multiple easily reviewed PRs of sensible size. Doing it all in this PR would be terrible. |
Ok I guess I don't want to hold this back any longer, but please bear in mind:
I know this must be frustrating but given the number of platforms we juggle an upgrade like this should communicate the balance of loss with tangible benefits. In previous discussions I had mistakenly thought this would fix our test failures. |
I believe that to not be an issue at all. The removed versions do not get security updates any more and newer versions do not remove hardware like Apple so it's fine. |
All your concerns will be resolved. You need not worry. Merry Christmas and happy new year ;) |
Description:
Updating to Go 1.19 introduces support for a lot of new features like improved atomic types, possibility of using generics (though we ought to be careful there) and allows us to keep dependencies updated in the cases where they have dropped support for Go 1.17.
We can still compile with Go 1.17 at the moment for GopherJS support but that will be removed going forward and other Go 1.19 specific changes will also follow.
For #3242
Checklist:
Where applicable: