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.

Sign up for GitHub

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

Jump to bottom

expvar: Go 1.9 changes expvar.Map.Init() #21619

Closed
nurse opened this issue Aug 25, 2017 · 10 comments
Closed

expvar: Go 1.9 changes expvar.Map.Init() #21619

nurse opened this issue Aug 25, 2017 · 10 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Milestone
Go1.9.2

Comments

@nurse
Copy link

nurse commented Aug 25, 2017

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

1.9

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/naruse/go"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.9/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.9/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/kj/8_qvvl6n4fb_p9554p99wcd40000gn/T/go-build576752229=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

# a.go
package main

import (
	"expvar"
	"fmt"
)

func main() {
	m := new(expvar.Map).Init()
	m.Add("foo", 1)
	fmt.Printf("%v\n", m.Get("foo"))
	m.Init()
	fmt.Printf("%v\n", m.Get("foo"))

What did you expect to see?

% /usr/local/Cellar/go/1.8.3/bin/go run a.go
1
<nil>

What did you see instead?

/usr/local/Cellar/go/1.9/bin/go run a.go
1
1

Note

The behavior is changed at fb0fe42#diff-e4fb3a2d42311171535831443fb1fca2R128.
I want a way to clear expvar.Map.
I used it in tests.
@ianlancetaylor ianlancetaylor changed the title Go 1.9 changes expvar.Map.Init() expvar: Go 1.9 changes expvar.Map.Init() Aug 25, 2017
@ianlancetaylor
Copy link
Contributor

CC @bcmills

The Init method is not documented. In Go 1.8, it would reset the expvar.Map to contain no entries. In Go 1.9 it has changed to not do that. The question is whether we should restore the old behavior, or whether we should document the new behavior.

@ianlancetaylor ianlancetaylor added this to the Go1.9.1 milestone Aug 25, 2017
@bcmills
Copy link
Contributor

bcmills commented Aug 25, 2017

The question is whether we should restore the old behavior, or whether we should document the new behavior.

Either seems fine by me.

I have a slight preference for documenting Init as deprecated and leaving it as a no-op: it seems odd for an Init method to be anything other than a no-op on an already-initialized map, it was never documented to begin with, and it is out-of-place in a Go API (where we tend to have either New functions that return already-initialized objects, or meaningful zero-values).

Perhaps I am missing something, but what's the use-case for the old behavior? (Why does the test need to clear the existing map rather than allocating a new one?)

@nurse
Copy link
Author

nurse commented Aug 25, 2017

Why does the test need to clear the existing map rather than allocating a new one?

There's no way to replace a map registered by expvar.NewMap.

@gopherbot
Copy link

Change https://golang.org/cl/62370 mentions this issue: expvar: make (*Map).Init clear existing keys

@bcmills
Copy link
Contributor

bcmills commented Sep 12, 2017

This is fixed at head. @ianlancetaylor, are there instructions somewhere on how to backport a patch to the 1.9.1 branch?

@ianlancetaylor
Copy link
Contributor

I am in no way a git export, and I don't know of any written instructions. What I do is, after the CL has been committed to tip,

git checkout release-branch.go1.9
git codereview change mybranchname
git cherry-pick <SHA>
git codereview change
git codereview mail

Here is the git revision number for the change to tip. The second git codereview change seems to be necessary to create the CL properly for the branch.

There is probably some more clever way to do this.

@nurse
Copy link
Author

nurse commented Sep 13, 2017

thanks!

@rsc rsc modified the milestones: Go1.9.1, Go1.9.2 Oct 4, 2017
@rsc rsc reopened this Oct 13, 2017
@rsc
Copy link
Contributor

rsc commented Oct 13, 2017 •
edited

CL 62370 OK for Go 1.9.2.

@rsc rsc added the CherryPickApproved Used during the release process for point releases label Oct 14, 2017
@rsc rsc mentioned this issue Oct 15, 2017
Closed
@gopherbot
Copy link

Change https://golang.org/cl/70973 mentions this issue: [release-branch.go1.9] expvar: make (*Map).Init clear existing keys

gopherbot pushed a commit that referenced this issue Oct 25, 2017
@rsc
[release-branch.go1.9] expvar: make (*Map).Init clear existing keys
8c7fa95 
fixes #21619

Change-Id: I5bb513dfc8cac875b06a262eec40b5863ae23a4c
Reviewed-on: https://go-review.googlesource.com/62370
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/70973
Run-TryBot: Russ Cox <rsc@golang.org>
@rsc
Copy link
Contributor

rsc commented Oct 26, 2017

go1.9.2 has been packaged and includes:

The release is posted at golang.org/dl.

— golang.org/x/build/cmd/releasebot, Oct 26 21:09:08 UTC

@rsc rsc closed this as completed Oct 26, 2017
@golang golang locked and limited conversation to collaborators Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.9.2
Development

No branches or pull requests
5 participants
@nurse @rsc @ianlancetaylor @bcmills @gopherbot