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
'Fix' the Sierra build error by adding -tags legacy #2594
Conversation
Will an app built with |
yes it will |
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.
Just one minor build tag change looks like a great solution around the LDFLAGS issue. I wonder introduce a legacy
tag is fix the problem specifically is indeed a workaround, is there any chance to make the build tag more general? Do we have a document about the build tag convention?
// +build !ci | ||
// +build !legacy |
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.
// +build !ci | |
// +build !legacy | |
//go:build !ci && !legacy | |
// +build !ci,!legacy |
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.
Sorry, this is built on the release branch - that has not migrated to Go 1.17 build flag convention yet, that is only on develop
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.
I don't know how the tag can be more general than legacy
. I commented above how I think this can apply.
After release I will add it to https://developer.fyne.io/started/compiling
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.
Is the code dedicated in the current release branch and will never be contained in future releases? If we have everything in the develop compatible with go:build whereas this newly introduced one is not, then when the develop branch is merged into the release branch, we will have missed cases such as this one.
Having an additional go:build should be no problem at all since they are useless for those older compilers. If 1.18 is released, then users who use the newest compiler will not be able to use the current release as a module because their compiler cannot recognize the build tags. So in any case, we will have to adapt the go:build tag for every supported release.
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.
Is the code dedicated in the current release branch and will never be contained in future releases?
When the release branch does merge to develop this will be addressed - the linters will not allow it in without that code added :). However adding it to this PR and the branch would be in an inconsistent state.
I don't think we will still be working on v2.0.x bug fix releases when Go 1.18 is released. If we are then this branch can be updated.
We do have https://developer.fyne.io/started/compiling for docs. |
Thanks for the doc. Indeed I found that before, but it turns out the document is for users, not development purposes. For instance, the build tag ci, arm, arm64, etc. are not entirely documented. The supported combination is also not very clear. |
These are part of the Go documentation on build tags. We do indeed have |
Description:
Cannot find a way to make this work automatically - we need to update cgo LDFLAGS based on OS version which seems to not be supported.
So add
legacy
tag that turns off the new notification center usage.Fixes #2478
Checklist: