Skip to content

Commit

Permalink
Fix wierd interactions due to Cobra and glog
Browse files Browse the repository at this point in the history
The glog package force the use of golang's underyling flag package,
which Cobra does not use. To work around this, we had a complicated
dance around defining flags in multiple places, calling flag.Parse
explicitly and then stomping values in the flag package with values we
got from Cobra.

Because we ended up parsing parts of the command line twice, each with
a different set of semantics, we ended up with bad UX in some
cases. For example:

`$ pulumi -v=10 --logflow update`

Would fail with an error message that looked nothing like normal CLI
errors, where as:

`$ pulumi -v=10 update --logflow`

Would behave as you expect. To address this, we now do two things:

- We never call flag.Parse() anymore. Wacking the flags with values we
  got from Cobra is sufficent for what we care about.

- We use a forked copy of glog which does not complain when
  flag.Parse() is not called before logging.

Fixes #301
Fixes #710
Fixes #968
  • Loading branch information
ellismg committed Aug 20, 2018
1 parent 347148c commit acf0fb2
Show file tree
Hide file tree
Showing 5 changed files with 12 additions and 14 deletions.
7 changes: 4 additions & 3 deletions Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions Gopkg.toml
Expand Up @@ -6,3 +6,8 @@
[[constraint]]
name = "github.com/golang/protobuf"
version = "v1.1.0"

[[override]]
name = "github.com/golang/glog"
source = "github.com/pulumi/glog"
branch = "pulumi-master"
7 changes: 0 additions & 7 deletions cmd/pulumi.go
Expand Up @@ -17,7 +17,6 @@ package cmd
import (
"bufio"
"encoding/json"
"flag"
"fmt"
"os"
"os/exec"
Expand Down Expand Up @@ -166,12 +165,6 @@ func NewPulumiCmd() *cobra.Command {
"Include the tracing header with the given contents.")
}

// Tell flag about -C, so someone can do pulumi -C <working-directory> stack and the call to cmdutil.InitLogging
// which calls flag.Parse under the hood doesn't yell at you.
//
// TODO[pulumi/pulumi#301]: when we move away from using glog, it should be safe to remove this.
flag.StringVar(&cwd, "C", "", "Run pulumi as if it had been started in another directory")

return cmd
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/resource/plugin/plugin.go
Expand Up @@ -229,7 +229,7 @@ func execPlugin(bin string, pluginArgs []string, pwd string) (*plugin, error) {
// Flow the logging information if set.
if logging.LogFlow {
if logging.LogToStderr {
args = append(args, "--logtostderr")
args = append(args, "-logtostderr")
}
if logging.Verbose > 0 {
args = append(args, "-v="+strconv.Itoa(logging.Verbose))
Expand Down
5 changes: 2 additions & 3 deletions pkg/util/logging/log.go
Expand Up @@ -71,9 +71,8 @@ func InitLogging(logToStderr bool, verbose int, logFlow bool) {
Verbose = verbose
LogFlow = logFlow

// Ensure the logging library has been initialized, including calling flag.Parse beforehand. Unfortunately,
// this is the only way to control the way logging runs. That includes poking around at flags below.
flag.Parse()
// glog uses golang's built in flags package to set configuration values, which is incompatible with how
// we use cobra. So we explicitly set the flags we care about here, and don't call flag.Parse()
if logToStderr {
err := flag.Lookup("logtostderr").Value.Set("true")
assertNoError(err)
Expand Down

0 comments on commit acf0fb2

Please sign in to comment.