Multiple libraries, init() and a request for help

[Sean Russell]

Hi,

I've encountered a situation for which I have so far been unable to think of a solution.  While the issue itself is a problem, it demonstrates a troublesome use case, and I'm interested in the recommended solution.  If there's a thread that can be referenced that discusses this use case, feel free to point me at it; my search queries in the group failed to identify such a discussion.

I use, and like, namsral/flag library.  It's a drop-in replacement for the standard flag library, but provides support for pulling flag options from environment variables: it is simple and low impact.  What I've seen is an interaction with some libraries that reference the standard library; fvbock/endless (a zero-downtime restarts library) is an example; it defines flags in init() which it uses for process behavior control. It does this by calling flag.Parse() in a constructor and dealing with the flags.

The problem is that namsral/flag's flag set and the default library's flag set are disjoint, meaning that any call to Parse() on either library (which obviously share the same input) is illegal. In other words, any os.Args passed in that are legal for namsral/flag will be illegal for the default flag library, which both (by default) reference the same input.

Is this a unique situation? Is endless violating some unspoken best-practice -- or is namsral/flag?  If neither of them are violating best practices, then what is the solution for this situation?

Thanks,

--- SER

[Chris Hines]

Not everyone may agree with me, but I believe that only package main should parse the command line.

Non-main packages that use package flag (or similar) are coupling themselves tightly to the command line. This practice not only interferes with package main's prerogative to control configuration, but also make the offending package more difficult to test and (re)use.

IMHO, non-main packages should expose their configuration points as part of their normal API and leave command line parsing to package main.

Based on that opinion, I would call out fvbock/endless for overstepping its responsibilities.

Chris

[Edward Muller]

+1. Libraries should have some sort of config API (functions, config structs, etc) and should not rely on command line flags, environment vars, etc.

They could however provide some helper functions that the integrating main package could optionally use.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Edward Muller
@freeformz

[Dave Cheney]

I agree with Chris. Because testing main packages is a pain, I prefer to keep as much logic out of the command as possible. The command should just parse the flags and arguments and marshal them into the form that the logic, in another package, requires. If this can be outsourced to a library like cli or cobra, even better.

Thanks

Dave

[James Aguilar]

On Monday, August 17, 2015 at 11:54:53 AM UTC-7, Dave Cheney wrote:

I agree with Chris. Because testing main packages is a pain, I prefer to keep as much logic out of the command as possible. The command should just parse the flags and arguments and marshal them into the form that the logic, in another package, requires. If this can be outsourced to a library like cli or cobra, even better. 

As much as one might like this to be the case, that's not really the way the flag library is designed. There isn't any protection from other packages using it and, indeed, as far as I can tell, it's a blessed use case. Wishing that it weren't is tilting against windmills, especially since there are already tons of libraries out there using it like that.

And honestly, having that capability does save the user some fairly strenuous plumbing under certain circumstances. For example, suppose your package is using a leveled logging library. Do you want to make a logging control structure part of your public API? Does the user have to pass down this control structure for every library and transitive library that uses that logging package? And do all of these libraries have to incorporate that piping? Do you break API compatibility when your internal logging layer does?

Or does the logging library define libname_loglevel and get all the plumbing done for free? I like the latter world better than the former.

OP: I'd suggest you use the default flag package to eliminate the conflict. After flag.Parse(), you could search the environment for any flags that were not set. I would guess that this is the quickest way to a resolution. Alternately, you could try convincing the authors of namsral/flag to use flag under the covers, to eliminate the conflicts at that level. Or ask on the namsral/flag mailing list? I'm really surprised if you're the first person to run into this problem.

[Chris Hines]

My opinions about logging align with my opinions about command line parsing. I will provide my opinions about logging via this reference: github.com/go-kit/kit/issues/42.

As I said before, others may not agree. I'm sure one's opinion depends on the type of software one writes. It is certainly true that several popular packages adhere more to your approach.

With that approach there is a danger that two packages you want to use define the same flag and then your application panics.

Another scenario is encountered when using the popular package github.com/golang/glog. It provides leveled logging with a global singleton and also gets its configuration from the flag package. I used glog for several months until I became frustrated by its lack of flexibility. One such annoyance is that glog defines a -v flag to control verbosity. That's great, except go test also defines a -v flag (which it passes down to the -test.v flag defined in package testing). Thus, if you would like to change logging verbosity for a test run the test code must define a separate flag to collect the logging verbosity and then copy the value into the glog flags. That adding the following two bits to the package test code:

// In test code
var glogv = flag.Int("glogv", 0, "")

// Once after flag.Parse is called, but before logging starts.
// Probably in TestMain, but that was only added in Go 1.4.
// I was doing it in a SetUpSuite method handled by gopkg.in/check.v1.
flag.Lookup("v").Value.Set(fmt.Sprint(*glogv))

I sympathize with the OP because the apps I've been writing lately are services that must run on both Windows and Linux. On the Windows side it is rather inconvenient to change the command line arguments for an installed service, while updating a configuration file is far less burdensome. As a result the application gets all of its local configuration from a config file. During development and testing it is convenient to override the config file (or not have one) and supply configuration on the command line. For these purposes I use github.com/cespare/flagconf.

You are correct that keeping these concerns more orthogonal as I suggest requires more plumbing. I think the flexibility gained is worth the extra glue code in package main code. Again, just my opinion.

Chris

[James Aguilar]

On Monday, August 17, 2015 at 2:00:45 PM UTC-7, Chris Hines wrote:

Another scenario is encountered when using the popular package github.com/golang/glog. It provides leveled logging with a global singleton and also gets its configuration from the flag package. I used glog for several months until I became frustrated by its lack of flexibility. One such annoyance is that glog defines a -v flag to control verbosity. That's great, except go test also defines a -v flag (which it passes down to the -test.v flag defined in package testing). Thus, if you would like to change logging verbosity for a test run the test code must define a separate flag to collect the logging verbosity and then copy the value into the glog flags. That adding the following two bits to the package test code:

// In test code
var glogv = flag.Int("glogv", 0, "")

// Once after flag.Parse is called, but before logging starts.
// Probably in TestMain, but that was only added in Go 1.4.
// I was doing it in a SetUpSuite method handled by gopkg.in/check.v1.
flag.Lookup("v").Value.Set(fmt.Sprint(*glogv))

This doesn't seem like it should be necessary. Unless I'm totally misreading the code, the test flag is called "test.v". There is no -v flag for the go test command. There is an inherited one from go build, but it shouldn't (??) have any effect during testing proper. From go help build:

-v
 print the names of packages as they are compiled.

To avoid the code you just posted, I think all that would be needed is

go test packagename -v verboseLevel -test.v testVerboseLevel

This doesn't really address any of your broader thematic points, but I just wanted to point out what seems to me as a bug with the details of this objection.

 

[Chris Hines]

The confusing part about go test flags is that the go test command generates a test binary that imports the code for the package under test and then runs that binary as a separate process. It prepends known test flags with "test." when constructing the command line for the test binary. This behavior is explained in the output of `go help testflag` and can also be seen here: https://github.com/golang/go/blob/release-branch.go1.4/src/cmd/go/test.go#L213

The result is that if the package under test declares one of the flags listed by `go help testflag` then go test will not pass it through cleanly to the package.

Chris

[James Aguilar]

I have to take this back. I went and tested it and it looks like something in go test is interpreting the -v and prepending "test." to it when passing it to the binary. There must be a workaround, right? But I haven't found it yet.

[Matt Harden]

The documentation (https://golang.org/cmd/go/#hdr-Test_packages) says this:

go test [-c] [-i] [build and test flags] [packages] [flags for test binary]

and

If the test binary needs any other flags, they should be presented after the package names. The go tool treats as a flag the first argument that begins with a minus sign that it does not recognize itself; that argument and all subsequent arguments are passed as arguments to the test binary.

On the one hand it suggests that any flags passed after packages names are treated as flags for the program under test, but then the text indicates different behavior. The code itself seems to keep looking for recognized flags even after it finds the first flag it doesn't recognize.

I think the only clean option is to build the test binary with -c (and optionally -o <filename>), and then run the test binary with the options you choose.

--

[James Aguilar]

At a minimum, it seems like a problem that the docs disagree with the actual behavior. Also, I think the actual behavior would be good to have. I filed golang/go#12177 about this.

[James Aguilar]

s/actual behavior/documented behavior/

[edunc...@gmail.com]

Not sure when Golang added the -args option to Tests.  But this now works:

    $ go test -cover -v -run TestEventsTableTests -args -v 3 -logtostderr true

The -args parameter allows you to pass in package-level flags into the binary and test package to run.  

For example, the glog logging package has options for -v and -logtostderr.    Previously we had to jump through lots of hoops to get glog logging in tests.  But now, we can just use -args and pass in any parameters we want.

Baked into Makefiles, it's solid in most of the projects I use it in.

On Monday, August 17, 2015 at 1:24:32 PM UTC-4, Sean Russell wrote: