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/digraph: permit larger tokens in use of bufio.Scanner #57807

Closed
ukai opened this issue Jan 16, 2023 · 11 comments
Closed

x/tools/cmd/digraph: permit larger tokens in use of bufio.Scanner #57807

ukai opened this issue Jan 16, 2023 · 11 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone
Unreleased

Comments

@ukai
Copy link
Contributor

ukai commented Jan 16, 2023

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

$ go version
go version go1.20-20221229-RC00 cl/498461153 +db36eca33c linux/amd64

Does this issue reproduce with the latest release?

yes

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/ukai/.cache/go-build"
GOENV="/usr/local/google/home/ukai/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/usr/local/google/home/ukai/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/usr/local/google/home/ukai/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/google-golang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20-20221229-RC00 cl/498461153 +db36eca33c"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build1558298642=/tmp/go-build -gno-record-gcc-switches"

What did you do?

when we pass digraph-input.txt (not related with go tools, though), which has long lines.

$ digraph nodes  < digraph-input.txt

What did you expect to see?

shows the set of all nodes

What did you see instead?

digraph: bufio.Scanner: token too long

can we have an option to specify buffer for bufio.Scanner for digraph command to handle long lines?
(or make digraph package (graph, nodeset type and digraph func) out of digraph command,
so we could use digraph functionalities in other tools?)
@seankhliao
Copy link
Member

digraph does not look like any tool under the Go project.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2023
@ianlancetaylor
Copy link
Contributor

To increase the maximum token size, use the (*bufio.Scanner).Buffer method. https://pkg.go.dev/bufio#Scanner.Buffer

@ukai
Copy link
Contributor Author

ukai commented Jan 17, 2023

ok to create a CL to use (*bufio.Scanner).Buffer with some flags in digraph?

is it an bad idea to change bufio.Scanner to use different size of buffer with some environment variables, so we don't need to change application that uses bufio.Scanner like digraph when long token is used?

@rittneje
Copy link

@ukai Are you referring to this? https://github.com/golang/tools/blob/master/cmd/digraph/digraph.go

@ukai
Copy link
Contributor Author

ukai commented Jan 17, 2023

yes

@ukai
Copy link
Contributor Author

ukai commented Jan 17, 2023

https://pkg.go.dev/golang.org/x/tools/cmd/digraph

@rittneje
Copy link

@seankhliao Given that digraph is part of x/tools, can you reopen this issue?

@ianlancetaylor ianlancetaylor changed the title digraph: bufio.Scanner: token too long x/tools/cmd/digraph: permit larger tokens in use of bufio.Scanner Jan 17, 2023
@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jan 17, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jan 17, 2023
@ianlancetaylor
Copy link
Contributor

CC @adonovan

Perhaps somebody should rewrite digraph to not use bufio.Scanner. It looks like it would be straightforward to use bufio.ReadString instead.

ukai added a commit to ukai/tools that referenced this issue Jan 17, 2023
@ukai
cmd/digraph: use ReadString rather than bufio.Scanner
9933ef4 
if an input line is too long (more than bufio.MaxScanTokenSize),
bufio.Scanner returns bufio.ErrToolong.

Use bufio's ReadString instead.

Fixes golang/go#57807
@ukai
Copy link
Contributor Author

ukai commented Jan 17, 2023

like this? golang/tools#423

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 17, 2023
ukai added a commit to ukai/tools that referenced this issue Jan 18, 2023
@ukai
cmd/digraph: use ReadString rather than bufio.Scanner
e7057d0 
if an input line is too long (more than bufio.MaxScanTokenSize),
bufio.Scanner returns bufio.ErrToolong.

Use bufio's ReadString instead.

Fixes golang/go#57807
@qiulaidongfeng
Copy link
Contributor

https://go-review.googlesource.com/c/tools/+/462053 merged , Is it possible to close this issue?

@ukai
Copy link
Contributor Author

ukai commented Jan 26, 2024

ah, I think so.

@ukai ukai closed this as completed Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@ukai @ianlancetaylor @gopherbot @seankhliao @rittneje @qiulaidongfeng