Skip to content
This repository has been archived by the owner on Jan 31, 2023. It is now read-only.

Add support for go modules. #1

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add support for go modules. #1

wants to merge 7 commits into from

Conversation

crufter
Copy link

@crufter crufter commented Oct 16, 2019

This PR does 4 things:

  • Adds support for Go modules
  • Moves the build from Travis to CircleCI
  • Fixes a race condition that appeared in the tests.
  • Fixes the local import pathes (mwitkow/go-flagz -> improbable-eng/go-flagz) because it is a recently forked library.

stefan-improbable and others added 4 commits October 16, 2019 13:40
- create go.sum
- do not use .. in filenames in this repo due to golang/go#27299

Testing strategy: I almost successfully ran `test_all.sh` and found a
data race, but I don't think it's related to this PR:

```
WARNING: DATA RACE
Read at 0x00c00016cac0 by goroutine 29:
  github.com/spf13/pflag.(*FlagSet).VisitAll()
      /home/stefan/usr/go/main/pkg/mod/github.com/spf13/pflag@v1.0.3/flag.go:277 +0x14a
  github.com/mwitkow/go-flagz.ChecksumFlagSet()
      /home/stefan/Projects/go-flagz/checksum.go:15 +0xc8
  github.com/mwitkow/go-flagz/monitoring.(*flagSetCollector).Collect()
      /home/stefan/Projects/go-flagz/monitoring/collector.go:57 +0x2a2
  github.com/prometheus/client_golang/prometheus.(*Registry).Gather.func1()
      /home/stefan/usr/go/main/pkg/mod/github.com/prometheus/client_golang@v0.9.2/prometheus/registry.go:434 +0x1eb

Previous write at 0x00c00016cac0 by goroutine 28:
  github.com/spf13/pflag.sortFlags()
      /home/stefan/usr/go/main/pkg/mod/github.com/spf13/pflag@v1.0.3/flag.go:204 +0x2f2
  github.com/spf13/pflag.(*FlagSet).VisitAll()
      /home/stefan/usr/go/main/pkg/mod/github.com/spf13/pflag@v1.0.3/flag.go:270 +0x1b0
  github.com/mwitkow/go-flagz.ChecksumFlagSet()
      /home/stefan/Projects/go-flagz/checksum.go:15 +0xc8
  github.com/mwitkow/go-flagz/monitoring.(*flagSetCollector).Collect()
      /home/stefan/Projects/go-flagz/monitoring/collector.go:55 +0xe9
  github.com/prometheus/client_golang/prometheus.(*Registry).Gather.func1()
      /home/stefan/usr/go/main/pkg/mod/github.com/prometheus/client_golang@v0.9.2/prometheus/registry.go:434 +0x1eb
```
@crufter crufter mentioned this pull request Oct 16, 2019
@crufter
Copy link
Author

crufter commented Oct 16, 2019

Edit: not relevant anymore.

// ChecksumFlagSet will generate a FNV of the *set* values in a FlagSet.
func ChecksumFlagSet(flagSet *pflag.FlagSet, flagFilter func(flag *pflag.Flag) bool) []byte {
h := fnv.New32a()

visitAllMutex.Lock()
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Admitteldy this feels heavy handed and not elegant considering the package mostly uses atomic instead of locking, but the reads and writes happening in VisitAll triggered the race detector.

@crufter crufter changed the title [WIP] Add support for go modules. Add support for go modules. Oct 18, 2019
@ldemailly
Copy link

is this going in / still being worked on / maintained?

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

Successfully merging this pull request may close these issues.

4 participants