encoding/protobuf: dependency on glog adds some flags to a program's std flag list #1199
Comments
|
This seems unfortunate at best, but also something that we can fix via a PR to the |
|
having the same problem when trying to update to version 0.4.0 |
|
Of note, using Without it, you are using a "global" scope for flags, where you can have name collisions or extra flags added from deps that use the "global" flag space. |
|
@myitcv can you please describe what's the difference between |
|
@gurjeet ah that was just me trying to recategorise issues we needed to discuss prior to FOSDEM. The |
This problem has been reported quite a few times over the years; for example, see it reported at golang-nuts mailing list [1], and cue lang issue [2]. [1]: https://groups.google.com/g/golang-nuts/c/vj8ozVqemnQ [2]: cue-lang/cue#1199 The problem is that, that glog package registers some flags in its init() function. The list of registered flags also includes the `-v` flag, which is usually used by developers either to control verbosity of their code-execution, or to show the software version. It's notable that all the complaints are regarding the `-v` flag, and none about the other flags, since those other flags are unlikely be used by any other developer. The proposed fix allows the user of the glog package to change/prefix glog's flags' names, so that they will not conflict with any flags that they want to use. This approach to the problem has a few advantages, compared to other options like, disabling all the flags in glog. .) The default behaviour of the glog library is unchanged. So the current users of the library will not be affected. .) Any new users who wish to use the -v, or other glog-occupied flag, can do so at build time. .) The new users can still use the glog features/flags, albeit with a prefix. .) We are not enforcing some specific prefix, which may also conflict. .) The --help flag, correctly reports the changed/prefixd flag names. ``` $ ./main --help Usage of ./main: ... other glog: prefixed flags ... -glog:v value log level for V logs -glog:vmodule value comma-separated list of pattern=N settings for file-filtered logging -v value Emit verbose execution progress ```
This problem has been reported quite a few times over the years; for example, see it reported at golang-nuts mailing list [1], and cue lang issue [2]. [1]: https://groups.google.com/g/golang-nuts/c/vj8ozVqemnQ [2]: cue-lang/cue#1199 The problem is that, that glog package registers some flags in its init() function. The list of registered flags also includes the `-v` flag, which is usually used by developers either to control verbosity of their code-execution, or to show the software version. It's notable that all the complaints are regarding the `-v` flag, and none about the other flags, since those other flags are unlikely be used by any other developer. The proposed fix allows the user of the glog package to change/prefix glog's flags' names, so that they will not conflict with any flags that they want to use. This approach to the problem has a few advantages, compared to other options like, disabling all the flags in glog. 1. The default behaviour of the glog library is unchanged. So the current users of the library will not be affected. 2. Any new users who wish to use the -v, or other glog-occupied flag, can do so at build time. 3. The new users can still use the glog features/flags, albeit with a prefix. 4. We are not enforcing some specific prefix, which may also conflict. 5. The --help flag, correctly reports the changed/prefixed flag names. ``` $ ./main --help Usage of ./main: ... other glog: prefixed flags ... -glog:v value log level for V logs -glog:vmodule value comma-separated list of pattern=N settings for file-filtered logging -v value Emit verbose execution progress ```
This problem has been reported quite a few times over the years; for example, see it reported at golang-nuts mailing list [1], and cue lang issue [2]. [1]: https://groups.google.com/g/golang-nuts/c/vj8ozVqemnQ [2]: cue-lang/cue#1199 The problem is that, that glog package registers some flags in its init() function. The list of registered flags also includes the `-v` flag, which is usually used by developers either to control verbosity of their code-execution, or to show the software version. It's notable that all the complaints are regarding the `-v` flag, and none about the other flags, since those other flags are unlikely be used by any other developer. The proposed fix allows the user of the glog package to change/prefix glog's flags' names, so that they will not conflict with any flags that they want to use. This approach to the problem has a few advantages, compared to other options like, disabling all the flags in glog. 1. The default behaviour of the glog library is unchanged. So the current users of the library will not be affected. 2. Any new users who wish to use the -v, or other glog-occupied flag, can do so at build time. 3. The new users can still use the glog features/flags, albeit with a prefix. 4. We are not enforcing some specific prefix, which may also conflict. 5. The --help flag, correctly reports the changed/prefixed flag names. ``` $ ./main --help Usage of ./main: ... other glog: prefixed flags ... -glog:v value log level for V logs -glog:vmodule value comma-separated list of pattern=N settings for file-filtered logging -v value Emit verbose execution progress ``` Please also see sample code [3] that demonstrates the problem, and how the patch fixes the problem. [3]: https://github.com/gurjeet/glog_fix_test
|
The But I have take a crack at solving this problem, so if the fix is merged, all users of the |
|
I think an easier way to fix this issue might be to change txtpbfmt to not import a CLI logging library in its relatively low level parser package: I personally think that's reasonable, because the user of the library should be in control of the logging - such as choosing what log library to use, or whether to use one at all. It seems like the parser only uses the library to make calls to |
|
I agree with @mvdan. It is overbearing for We may also consider using a different package or forking |
|
I've gone ahead and asked nicely: protocolbuffers/txtpbfmt#70 |
|
Took a long time to agree with upstream and get the changes reviewed, but it finally looks like both of my PRs are being merged. We should be able to update txtpbfmt and drop glog as a dependency soon. |
mvdan
removed
good first issue
|
Now merged :) The glog dependency should be gone in master and the upcoming v0.6.0-alpha.1 release. |
Originally posted by knieriem August 8, 2021 in #1196
When upgrading a program importing a package from cue v0.3.0-beta* to v0.4.0 I noted that CUE's new dependency github.com/protocolbuffers/txtpbfmt adds another dependency, github.com/golang/glog (leveled execution logs for Go).
go mod whylists, for instance:Glog, as a package, in its init function defines some command line flags using Go's standard flag package:
As a result, any program importing, for example,
cuelang.org/go/cue/loadwill automatically inherit these flags. In my program, which makes use of the standard flag package, I had been defining an ownflag.Uint("v", 0, "verbosity level").With cue v0.4.0 I'm getting a
panic: ... flag redefined: verror now.Since the
cueprogram itself is using cobra+pflag, this effect is not visible when using cue itself, since flags defined using Go's standard flag package are ignored then.An old thread at golang-nuts, glog and testing flag "pollution" mentions a workaround: defining one's own flag.FlagSet instead of using the default
flag.CommandLine. But this also means, that all flags defined by a program's internal packages using e.g.flag.Boolmust be adapted to the new FlagSet.An alternative to this workaround would be to make
textprotodepend on a modifiedtxtpbfmtthat does not depend on glog. I think for my use-case I will apply the method described in that golang-nuts thread.The text was updated successfully, but these errors were encountered: