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

-help panicking with Go 1.7 #50

Closed
deluan opened this issue Sep 8, 2016 · 6 comments
Closed

-help panicking with Go 1.7 #50

deluan opened this issue Sep 8, 2016 · 6 comments
Assignees
Labels

Comments

@deluan
Copy link

deluan commented Sep 8, 2016

After upgrading to Go 1.7 (and 1.7.1), -help gives a panic:

$ brew switch go 1.6.3
Cleaning /usr/local/Cellar/go/1.6.3
Cleaning /usr/local/Cellar/go/1.7
Cleaning /usr/local/Cellar/go/1.7.1
3 links created for /usr/local/Cellar/go/1.6.3
$ go build
$ ./gosonic -help
Usage of ./gosonic:
  -dbpath
        Change value of DbPath. (default ./devDb)
  -disabledownsampling
        Change value of DisableDownsampling.
  -disablevalidation
        Change value of DisableValidation. (default true)
...

Generated environment variables:
   GOSONIC_PORT
   GOSONIC_MUSICFOLDER
   GOSONIC_DBPATH
...

$ brew switch go 1.7.1
Cleaning /usr/local/Cellar/go/1.6.3
Cleaning /usr/local/Cellar/go/1.7
Cleaning /usr/local/Cellar/go/1.7.1
3 links created for /usr/local/Cellar/go/1.7.1
$ go build
$ ./gosonic -help
Usage of ./gosonic:
panic: reflect: call of reflect.Value.Interface on zero Value

goroutine 1 [running]:
panic(0x5c8280, 0xc4202c0360)
    /usr/local/Cellar/go/1.7.1/libexec/src/runtime/panic.go:500 +0x1a1
reflect.valueInterface(0x0, 0x0, 0x0, 0x28744d88401, 0x0, 0xd6e84)
    /usr/local/Cellar/go/1.7.1/libexec/src/reflect/value.go:912 +0x204
reflect.Value.Interface(0x0, 0x0, 0x0, 0x11bcb, 0x6089a0)
    /usr/local/Cellar/go/1.7.1/libexec/src/reflect/value.go:907 +0x44
github.com/deluan/gosonic/vendor/github.com/fatih/structs.(*Field).Value(0xc4202d0360, 0xf547, 0x5dc700)
    /Users/deluan/Development/go/src/github.com/deluan/gosonic/vendor/github.com/fatih/structs/field.go:31 +0x40
github.com/deluan/gosonic/vendor/github.com/koding/multiconfig.(*fieldValue).String(0xc4202d0360, 0x5ef2a0, 0xc4202d0360)
    /Users/deluan/Development/go/src/github.com/deluan/gosonic/vendor/github.com/koding/multiconfig/flag.go:151 +0x2f
flag.isZeroValue(0xc4202ab180, 0xc420297f00, 0x7, 0xc4202c0180)
    /usr/local/Cellar/go/1.7.1/libexec/src/flag/flag.go:393 +0x12d
flag.(*FlagSet).PrintDefaults.func1(0xc4202ab180)
    /usr/local/Cellar/go/1.7.1/libexec/src/flag/flag.go:467 +0x1d5
flag.(*FlagSet).VisitAll(0xc4202bcea0, 0xc420053ab8)
    /usr/local/Cellar/go/1.7.1/libexec/src/flag/flag.go:323 +0x67
flag.(*FlagSet).PrintDefaults(0xc4202bcea0)
    /usr/local/Cellar/go/1.7.1/libexec/src/flag/flag.go:476 +0x46
github.com/deluan/gosonic/vendor/github.com/koding/multiconfig.(*FlagLoader).Load.func1()
    /Users/deluan/Development/go/src/github.com/deluan/gosonic/vendor/github.com/koding/multiconfig/flag.go:67 +0x118
flag.(*FlagSet).usage(0xc4202bcea0)
    /usr/local/Cellar/go/1.7.1/libexec/src/flag/flag.go:828 +0x5c
flag.(*FlagSet).parseOne(0xc4202bcea0, 0x617be0, 0x1, 0xc4202d40f0)
    /usr/local/Cellar/go/1.7.1/libexec/src/flag/flag.go:870 +0xb4c
flag.(*FlagSet).Parse(0xc4202bcea0, 0xc420094070, 0x1, 0x1, 0x0, 0x0)
    /usr/local/Cellar/go/1.7.1/libexec/src/flag/flag.go:915 +0x60
github.com/deluan/gosonic/vendor/github.com/koding/multiconfig.(*FlagLoader).Load(0xc4202bb900, 0x584160, 0xc420121810, 0xc420139ee0, 0x0)
    /Users/deluan/Development/go/src/github.com/deluan/gosonic/vendor/github.com/koding/multiconfig/flag.go:82 +0x25f
github.com/deluan/gosonic/conf.LoadFromFlags()
    /Users/deluan/Development/go/src/github.com/deluan/gosonic/conf/configuration.go:33 +0x79
main.main()
    /Users/deluan/Development/go/src/github.com/deluan/gosonic/main.go:15 +0x2b

For reference, my code is available here: https://github.com/deluan/gosonic/blob/master/conf/configuration.go

@rjeczalik
Copy link
Member

This var here is passed as a nil pointer here.

Looking at the stacktrace I think error originates from github.com/fatih/structs and probably it should be reported upstream.

@deluan
Copy link
Author

deluan commented Sep 8, 2016

The var is initialized in the init() function. I'll try to debug the structs package and see if I can find the problem.

@deluan
Copy link
Author

deluan commented Sep 8, 2016

I've created a small program using the example code from the README, and it gives the same error when called with -help.

@rjeczalik
Copy link
Member

It's issue with github.com/fatih/structs, the check for IsExported should be adapted as described in golang.org/issue/12367.

@deluan deluan changed the title -help panicing with Go 1.7 -help panicking with Go 1.7 Sep 9, 2016
@client9
Copy link
Contributor

client9 commented Sep 9, 2016

golang/go#12367 says:

Code that assumes

f.PkgPath != nil 
means a field is unexported and must be ignored must now be revised to check for

f.PkgPath != nil && !f.Anonymous
for it to walk into the embedded structs to look for exported fields contained within.

rjeczalik added a commit to rjeczalik/multiconfig that referenced this issue Sep 11, 2016
Flag package tries not to call String() on a zero-value
of type registered to a FlagSet:

    https://github.com/golang/go/blob/7419073/src/flag/flag.go#L390-L394

This PR makes the fieldValue zero-value safe, at least to not
panic when calling String().

Fixes koding#50.
@rjeczalik rjeczalik added the bug label Sep 11, 2016
@rjeczalik rjeczalik self-assigned this Sep 11, 2016
@rjeczalik
Copy link
Member

@deluan I've sent #51 to fix this issue. I was wrong, it's not handling of exported fields in fatih/structs reason for this panic, but the new behaviour of flag package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants