koding / multiconfig

Load configuration from multiple sources in Go
http://godoc.org/github.com/koding/multiconfig
MIT License
454 stars 64 forks source link

-help panicking with Go 1.7 #50

Closed deluan closed 8 years ago

deluan commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

rjeczalik commented 8 years ago

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

client9 commented 8 years ago

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 commented 8 years ago

@deluan I've sent https://github.com/koding/multiconfig/pull/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.