extended backwards compatibility for Go #56986
Comments
|
Change https://go.dev/cl/453605 mentions this issue: |
|
Change https://go.dev/cl/453604 mentions this issue: |
|
Change https://go.dev/cl/453603 mentions this issue: |
|
Design doc at https://go.dev/design/56986-godebug. |
Syntax error: Unterminated sentence (I assume). |
|
It would also be nice if runtime/debug.BuildInfo reported all the |
|
@rittneje @rsc
Or (And?) the runtime might re-set the |
|
@rittneje, great idea, thanks, added to the proposal and the implementation sketch as a new Setting with key "DefaultGODEBUG". @mateusz834 People in the wild should really not be parsing GODEBUG. But if they really want to, they can now use the BuildInfo setting to find the default and then consult os.Getenv("GODEBUG") for the overrides. Changing the environment variable to hold the entire unified setting is a mistake because it would affect subprocesses. @Merovius, fixed, thanks. |
|
Thanks, the design lgtm. I just had a couple thoughts as I read through it:
Can the design add details about what this will do when building a main module whose
This means that when eventually-removed godebug options are dropped (after 2+ years / 4+ releases), a program explicitly reverting a runtime behavior with this option will no longer build with new go versions. This seems reasonable to me (even preferable compared to silently ignoring an explicit go:debug directive) but should be explicitly documented:
|
|
@liggitt, if this lands in Go 1.21, then I think probably we would make any I agree with your comments about the unrecognized //go:debug lines. It is worth pointing out that these can only appear in a main package, so at least there is no chance of dependencies causing problems. |
|
Has there been a discussion of the extent to which person wishing to defeat security could use a GODEBUG environment variable to, for example, use SHA1 certificates? |
That matches my expectations. I agree we should not downgrade behaviors simply by updating the toolchain. Anyone updating their toolchain from earlier go versions (which would not get the automatic go.mod-informed defaulting) who needs to maintain compatibility with specific still-supported godebug options could use explicit Mentioning both of those points in the design/documentation would be helpful. |
|
Should we warn users when compiling code with Edit: probably only in |
What happens if my go.mod references an older Go version such that it is no longer possible to maintain compatibility because some of the requisite |
|
@davecb: If someone wants to defeat security by setting environment variables, PATH is often a better choice than GODEBUG. In any event, we already have the setting, so this proposal does not make any environment-based vulnerabilities worse. @mateusz834: I don't believe we should warn users during a build or install with GODEBUG settings. They're just a part of the program. There are plenty of ways a program can be made insecure. It would be strange to warn about just this one. (We don't warn users during a build or install of programs importing "crypto/sha1" either.) @rittneje: If go.mod mentions an old Go version for which some of the necessary GODEBUGs have been retired, then the build proceeds as before. Most programs that say 'go 1.14' don't need SHA1 certificates, so even if the SHA1 GODEBUG has been retired, the build should proceed in the most compatible way it can. The balance comes out differently in programs that say @liggitt: I will update the rationale to reflect this conversation. Thanks. |
I assume Millls -> Mills? |
|
This proposal has been added to the active column of the proposals project |
|
I hate to bikeshed but I never heard about GODEBUG before and I think it's not a very good name for enabling backwards compatible behavior. GOCOMPAT, could perhaps be better? |
|
Perhaps |
|
For reference, the name was previously discussed at #55090 (comment), including a suggestion of |
Based on discussion on golang/go#56986 with Jordan Liggitt. Change-Id: I7787155ea8194d879cadf0e0a9d043dd2ef5c38f Reviewed-on: https://go-review.googlesource.com/c/proposal/+/455316 Reviewed-by: Russ Cox <rsc@golang.org>
|
Change https://go.dev/cl/455316 mentions this issue: |
|
Does anyone have any concerns about accepting this proposal? |
|
@rsc One thing this proposal does not make clear is what happens if there are On a related note, how exactly will they work for unit testing? Will it respect Also, I don't find this rationale convincing:
Since the compiler cannot guess the intent, I believe the best thing it can do in this situation is fail (with a clear error message). Otherwise you can end up with an application that is subtly broken. The user has two options:
|
|
Change https://go.dev/cl/499415 mentions this issue: |
For #56986 For #59033 Change-Id: I7d03fe34d418aff97a551b236b5d43506e402871 Reviewed-on: https://go-review.googlesource.com/c/go/+/499415 TryBot-Bypass: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> Reviewed-by: Bryan Mills <bcmills@google.com> Auto-Submit: Ian Lance Taylor <iant@google.com>
|
Change https://go.dev/cl/500956 mentions this issue: |
Also handle go test -c TODO. For #15513. For #56986. For #57001. Change-Id: I571ae25d8d8fcd44cb38ac16cdd2a1180016eb94 Reviewed-on: https://go-review.googlesource.com/c/go/+/500956 TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> Reviewed-by: David Chase <drchase@google.com>
Long ago we decided that panic(nil) was too unlikely to bother
making a special case for purposes of recover. Unfortunately,
it has turned out not to be a special case. There are many examples
of code in the Go ecosystem where an author has written panic(nil)
because they want to panic and don't care about the panic value.
Using panic(nil) in this case has the unfortunate behavior of
making recover behave as though the goroutine isn't panicking.
As a result, code like:
func f() {
defer func() {
if err := recover(); err != nil {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
}
looks like it guarantees that call2 has been run any time f returns,
but that turns out not to be strictly true. If call1 does panic(nil),
then f returns "successfully", having recovered the panic, but
without calling call2.
Instead you have to write something like:
func f() {
done := false
defer func() {
if err := recover(); !done {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
done = true
}
which defeats nearly the whole point of recover. No one does this,
with the result that almost all uses of recover are subtly broken.
One specific broken use along these lines is in net/http, which
recovers from panics in handlers and sends back an HTTP error.
Users discovered in the early days of Go that panic(nil) was a
convenient way to jump out of a handler up to the serving loop
without sending back an HTTP error. This was a bug, not a feature.
Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature.
Any lingering code that uses panic(nil) to abort an HTTP handler
without a failure message should be changed to use http.ErrAbortHandler.
Programs that need the old, unintended behavior from net/http
or other packages can set GODEBUG=panicnil=1 to stop the run-time error.
Uses of recover that want to detect panic(nil) in new programs
can check for recover returning a value of type *runtime.PanicNilError.
Because the new GODEBUG is used inside the runtime, we can't
import internal/godebug, so there is some new machinery to
cross-connect those in this CL, to allow a mutable GODEBUG setting.
That won't be necessary if we add any other mutable GODEBUG settings
in the future. The CL also corrects the handling of defaulted GODEBUG
values in the runtime, for golang#56986.
Fixes golang#25448.
Change-Id: I2b39c7e83e4f7aa308777dabf2edae54773e03f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/461956
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Long ago we decided that panic(nil) was too unlikely to bother
making a special case for purposes of recover. Unfortunately,
it has turned out not to be a special case. There are many examples
of code in the Go ecosystem where an author has written panic(nil)
because they want to panic and don't care about the panic value.
Using panic(nil) in this case has the unfortunate behavior of
making recover behave as though the goroutine isn't panicking.
As a result, code like:
func f() {
defer func() {
if err := recover(); err != nil {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
}
looks like it guarantees that call2 has been run any time f returns,
but that turns out not to be strictly true. If call1 does panic(nil),
then f returns "successfully", having recovered the panic, but
without calling call2.
Instead you have to write something like:
func f() {
done := false
defer func() {
if err := recover(); !done {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
done = true
}
which defeats nearly the whole point of recover. No one does this,
with the result that almost all uses of recover are subtly broken.
One specific broken use along these lines is in net/http, which
recovers from panics in handlers and sends back an HTTP error.
Users discovered in the early days of Go that panic(nil) was a
convenient way to jump out of a handler up to the serving loop
without sending back an HTTP error. This was a bug, not a feature.
Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature.
Any lingering code that uses panic(nil) to abort an HTTP handler
without a failure message should be changed to use http.ErrAbortHandler.
Programs that need the old, unintended behavior from net/http
or other packages can set GODEBUG=panicnil=1 to stop the run-time error.
Uses of recover that want to detect panic(nil) in new programs
can check for recover returning a value of type *runtime.PanicNilError.
Because the new GODEBUG is used inside the runtime, we can't
import internal/godebug, so there is some new machinery to
cross-connect those in this CL, to allow a mutable GODEBUG setting.
That won't be necessary if we add any other mutable GODEBUG settings
in the future. The CL also corrects the handling of defaulted GODEBUG
values in the runtime, for golang#56986.
Fixes golang#25448.
Change-Id: I2b39c7e83e4f7aa308777dabf2edae54773e03f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/461956
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Long ago we decided that panic(nil) was too unlikely to bother
making a special case for purposes of recover. Unfortunately,
it has turned out not to be a special case. There are many examples
of code in the Go ecosystem where an author has written panic(nil)
because they want to panic and don't care about the panic value.
Using panic(nil) in this case has the unfortunate behavior of
making recover behave as though the goroutine isn't panicking.
As a result, code like:
func f() {
defer func() {
if err := recover(); err != nil {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
}
looks like it guarantees that call2 has been run any time f returns,
but that turns out not to be strictly true. If call1 does panic(nil),
then f returns "successfully", having recovered the panic, but
without calling call2.
Instead you have to write something like:
func f() {
done := false
defer func() {
if err := recover(); !done {
log.Fatalf("panicked! %v", err)
}
}()
call1()
call2()
done = true
}
which defeats nearly the whole point of recover. No one does this,
with the result that almost all uses of recover are subtly broken.
One specific broken use along these lines is in net/http, which
recovers from panics in handlers and sends back an HTTP error.
Users discovered in the early days of Go that panic(nil) was a
convenient way to jump out of a handler up to the serving loop
without sending back an HTTP error. This was a bug, not a feature.
Go 1.8 added panic(http.ErrAbortHandler) as a better way to access the feature.
Any lingering code that uses panic(nil) to abort an HTTP handler
without a failure message should be changed to use http.ErrAbortHandler.
Programs that need the old, unintended behavior from net/http
or other packages can set GODEBUG=panicnil=1 to stop the run-time error.
Uses of recover that want to detect panic(nil) in new programs
can check for recover returning a value of type *runtime.PanicNilError.
Because the new GODEBUG is used inside the runtime, we can't
import internal/godebug, so there is some new machinery to
cross-connect those in this CL, to allow a mutable GODEBUG setting.
That won't be necessary if we add any other mutable GODEBUG settings
in the future. The CL also corrects the handling of defaulted GODEBUG
values in the runtime, for golang#56986.
Fixes golang#25448.
Change-Id: I2b39c7e83e4f7aa308777dabf2edae54773e03f5
Reviewed-on: https://go-review.googlesource.com/c/go/+/461956
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
This comment was marked as spam.
This comment was marked as spam.
|
Change https://go.dev/cl/618115 mentions this issue: |
Fixes golang/go#69688 Updates golang/go#56986 Change-Id: I4c55b57d974b434fec74e38034d1ce033204ae12 Reviewed-on: https://go-review.googlesource.com/c/website/+/618115 Reviewed-by: Tim King <taking@google.com> Auto-Submit: Tim King <taking@google.com> Reviewed-by: Cherry Mui <cherryyz@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Matloob <matloob@golang.org>
Go's emphasis on backwards compatibility is one of its key strengths.
There are, however, times when we cannot maintain strict compatibility,
such as when changing sort algorithms or fixing clear bugs,
when existing code depends on the old algorithm or the buggy behavior.
This proposal aims to address many such situations by keeping older Go programs
executing the same way even when built with newer Go distributions.
See my talk on this topic at GopherCon for background.
See the design document for details.
The text was updated successfully, but these errors were encountered: