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.

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

x/tools/cmd/bundle: bundle discard some comments #20627

Closed
hirochachacha opened this issue Jun 9, 2017 · 5 comments
Closed

x/tools/cmd/bundle: bundle discard some comments #20627

hirochachacha opened this issue Jun 9, 2017 · 5 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@hirochachacha
Copy link
Contributor

hirochachacha commented Jun 9, 2017 •
edited

Please answer these questions before submitting your issue. Thanks!

What did you do?

If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

Apply following patch.

diff --git a/cmd/bundle/testdata/out.golden b/cmd/bundle/testdata/out.golden
index b76c5ae7..4729cee6 100644
--- a/cmd/bundle/testdata/out.golden
+++ b/cmd/bundle/testdata/out.golden
@@ -32,7 +32,8 @@ func prefixbar(s *prefixS) {
 
 type prefixt int
 
-const prefixc = 1
+// const1
+const prefixc = 1 // const2
 
 func prefixfoo() {
 	fmt.Println(importdecl.F())
diff --git a/cmd/bundle/testdata/src/initial/b.go b/cmd/bundle/testdata/src/initial/b.go
index 5ec45ad4..653a28ce 100644
--- a/cmd/bundle/testdata/src/initial/b.go
+++ b/cmd/bundle/testdata/src/initial/b.go
@@ -9,7 +9,8 @@ import (
 
 type t int
 
-const c = 1
+// const1
+const c = 1 // const2
 
 func foo() {
 	fmt.Println(importdecl.F())

then, go test.

What did you expect to see?

no errors.

What did you see instead?

--- FAIL: TestBundle (0.01s)
	main_test.go:47: -- got --
		// Code generated by golang.org/x/tools/cmd/bundle. DO NOT EDIT.
		//   $ bundle
		
		// The package doc comment
		//
		
		package dest
		
		import (
			"fmt"
			. "fmt"
			_ "fmt"
			renamedfmt "fmt"
			renamedfmt2 "fmt"
		
			"domain.name/importdecl"
		)
		
		// init functions are not renamed
		func init() { prefixfoo() }
		
		// Type S.
		type prefixS struct {
			prefixt
			u int
		}
		
		// Function bar.
		func prefixbar(s *prefixS) {
			fmt.Println(s.prefixt, s.u) // comment inside function
		}
		
		type prefixt int
		
		// const1
		const prefixc = 1
		
		func prefixfoo() {
			fmt.Println(importdecl.F())
		}
		
		func prefixbaz() {
			renamedfmt.Println()
			renamedfmt2.Println()
			Println()
		}
		
		-- want --
		// Code generated by golang.org/x/tools/cmd/bundle. DO NOT EDIT.
		//   $ bundle
		
		// The package doc comment
		//
		
		package dest
		
		import (
			"fmt"
			. "fmt"
			_ "fmt"
			renamedfmt "fmt"
			renamedfmt2 "fmt"
		
			"domain.name/importdecl"
		)
		
		// init functions are not renamed
		func init() { prefixfoo() }
		
		// Type S.
		type prefixS struct {
			prefixt
			u int
		}
		
		// Function bar.
		func prefixbar(s *prefixS) {
			fmt.Println(s.prefixt, s.u) // comment inside function
		}
		
		type prefixt int
		
		// const1
		const prefixc = 1 // const2
		
		func prefixfoo() {
			fmt.Println(importdecl.F())
		}
		
		func prefixbaz() {
			renamedfmt.Println()
			renamedfmt2.Println()
			Println()
		}
		
		-- diff --
	main_test.go:52: --- testdata/out.golden	2017-06-09 19:04:50.000000000 +0900
		+++ testdata/out.got	2017-06-09 19:10:09.000000000 +0900
		@@ -33,7 +33,7 @@
		 type prefixt int
		 
		 // const1
		-const prefixc = 1 // const2
		+const prefixc = 1
		 
		 func prefixfoo() {
		 	fmt.Println(importdecl.F())
		
FAIL
exit status 1
FAIL	golang.org/x/tools/cmd/bundle	0.023s

Does this issue reproduce with the latest release (go1.8.3)?

I think so.

System details

go version devel +760636d55a Thu Jun 8 23:54:27 2017 +0000 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/hiro/.go"
GORACE=""
GOROOT="/Users/hiro/go"
GOTOOLDIR="/Users/hiro/go/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/wq/dwn8hs0x7njbzty9f68y61700000gn/T/go-build749631531=/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"
GOROOT/bin/go version: go version devel +760636d55a Thu Jun 8 23:54:27 2017 +0000 darwin/amd64
GOROOT/bin/go tool compile -V: compile version devel +760636d55a Thu Jun 8 23:54:27 2017 +0000 X:framepointer
uname -v: Darwin Kernel Version 16.6.0: Fri Apr 14 16:21:16 PDT 2017; root:xnu-3789.60.24~6/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.12.5
BuildVersion:	16F73
lldb --version: lldb-370.0.42
  Swift-3.1
gdb --version: GNU gdb (GDB) 7.12.1

I suspect go/printer package. But I didn't investigate it yet.
@gopherbot gopherbot added this to the Unreleased milestone Jun 9, 2017
@gopherbot
Copy link

CL https://golang.org/cl/45190 mentions this issue.

@hirochachacha
Copy link
Contributor Author

bundle tries to add prefixes to identifiers. But it doesn't update line info in token.FileSet.
As a result, output format is unreliable. I don't have a good idea to solve it now.

@bradfitz
Copy link
Contributor

bradfitz commented Jun 9, 2017

/cc @alandonovan @griesemer

@hirochachacha
Copy link
Contributor Author

bundle tries to add prefixes to identifiers. But it doesn't update line info in token.FileSet.
As a result, output format is unreliable. I don't have a good idea to solve it now.

No, first impression is right. The underlying problem is #20635

@gopherbot
Copy link

CL https://golang.org/cl/45700 mentions this issue.

gopherbot pushed a commit that referenced this issue Jun 14, 2017
@bradfitz
net/http: update bundled http2
8c4bec8 
Update x/net/http2 to git rev 6b17b9baf5 for:

   http2: stop rejecting outgoing paths beginning with two slashes
   https://golang.org/cl/45773

This also uses an updated version of x/tools/cmd/bundle (CL 45190)
that fixes an edge case where it used to drop some comments.

Updates #20627
Fixes #19103

Change-Id: I450d61485e66098f4f8a79954f729f7bcd85856f
Reviewed-on: https://go-review.googlesource.com/45700
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>
@golang golang locked and limited conversation to collaborators Jul 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
3 participants
@bradfitz @hirochachacha @gopherbot