cmd/dlv,config: obey logflags config for LoadConfig warnings (#2701)

LoadConfig warnings should obey the logflags configuration and should
also be delayed until after the "listening at" message, which should
always be the first thing output.

Co-authored-by: Suzy Mueller <suzmue@golang.org>

cmd/dlv/cmds/commands.go

@@ -90,6 +90,7 @@ var (
 	allowNonTerminalInteractive bool
 
 	conf        *config.Config
+	loadConfErr error
 )
 
 const dlvCommandLongDesc = `Delve is a source level debugger for Go programs.
@@ -106,7 +107,10 @@ Pass flags to the program you are debugging using ` + "`--`" + `, for example:
 // New returns an initialized command tree.
 func New(docCall bool) *cobra.Command {
 	// Config setup and load.
-	conf = config.LoadConfig()
+	conf, loadConfErr = config.LoadConfig()
+	// Delay reporting errors about configuration loading delayed until after the
+	// server is started so that the "server listening at" message is always
+	// the first thing emitted. Also logflags hasn't been setup yet at this point.
 	buildFlagsDefault := ""
 	if runtime.GOOS == "windows" {
 		ver, _ := goversion.Installed()
@@ -433,6 +437,10 @@ func dapCmd(cmd *cobra.Command, args []string) {
 		}
 		defer logflags.Close()
 
+		if loadConfErr != nil {
+			logflags.DebuggerLogger().Errorf("%v", loadConfErr)
+		}
+
 		if cmd.Flag("headless").Changed {
 			fmt.Fprintf(os.Stderr, "Warning: dap mode is always headless\n")
 		}
@@ -544,6 +552,9 @@ func traceCmd(cmd *cobra.Command, args []string) {
 			fmt.Fprintf(os.Stderr, "%v\n", err)
 			return 1
 		}
+		if loadConfErr != nil {
+			logflags.DebuggerLogger().Errorf("%v", loadConfErr)
+		}
 
 		if headless {
 			fmt.Fprintf(os.Stderr, "Warning: headless mode not supported with trace\n")
@@ -752,6 +763,15 @@ func coreCmd(cmd *cobra.Command, args []string) {
 }
 
 func connectCmd(cmd *cobra.Command, args []string) {
+	if err := logflags.Setup(log, logOutput, logDest); err != nil {
+		fmt.Fprintf(os.Stderr, "%v\n", err)
+		os.Exit(1)
+		return
+	}
+	defer logflags.Close()
+	if loadConfErr != nil {
+		logflags.DebuggerLogger().Errorf("%v", loadConfErr)
+	}
 	addr := args[0]
 	if addr == "" {
 		fmt.Fprint(os.Stderr, "An empty address was provided. You must provide an address as the first argument.\n")
@@ -830,6 +850,9 @@ func execute(attachPid int, processArgs []string, conf *config.Config, coreFile
 		return 1
 	}
 	defer logflags.Close()
+	if loadConfErr != nil {
+		logflags.DebuggerLogger().Errorf("%v", loadConfErr)
+	}
 
 	if headless && (initFile != "") {
 		fmt.Fprint(os.Stderr, "Warning: init file ignored with --headless\n")

pkg/config/config.go

@@ -91,34 +91,27 @@ func (c *Config) GetSourceListLineCount() int {
 }
 
 // LoadConfig attempts to populate a Config object from the config.yml file.
-func LoadConfig() *Config {
+func LoadConfig() (*Config, error) {
 	err := createConfigPath()
 	if err != nil {
-		fmt.Printf("Could not create config directory: %v.", err)
+		return &Config{}, fmt.Errorf("could not create config directory: %v", err)
-		return &Config{}
 	}
 	fullConfigFile, err := GetConfigFilePath(configFile)
 	if err != nil {
-		fmt.Printf("Unable to get config file path: %v.", err)
+		return &Config{}, fmt.Errorf("unable to get config file path: %v", err)
-		return &Config{}
 	}
 
-	hasOldConfig, err := hasOldConfig()
+	hasOldConfig, _ := hasOldConfig()
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "Unable to determine if old config exists: %v\n", err)
-	}
 
 	if hasOldConfig {
 		userHomeDir := getUserHomeDir()
 		oldLocation := path.Join(userHomeDir, configDirHidden)
 		if err := moveOldConfig(); err != nil {
-			fmt.Fprintf(os.Stderr, "Unable to move old config: %v\n", err)
+			return &Config{}, fmt.Errorf("unable to move old config: %v", err)
-			return &Config{}
 		}
 
 		if err := os.RemoveAll(oldLocation); err != nil {
-			fmt.Fprintf(os.Stderr, "Unable to remove old config location: %v\n", err)
+			return &Config{}, fmt.Errorf("unable to remove old config location: %v", err)
-			return &Config{}
 		}
 		fmt.Fprintf(os.Stderr, "Successfully moved config from: %s to: %s\n", oldLocation, fullConfigFile)
 	}
@@ -127,35 +120,27 @@ func LoadConfig() *Config {
 	if err != nil {
 		f, err = createDefaultConfig(fullConfigFile)
 		if err != nil {
-			fmt.Printf("Error creating default config file: %v", err)
+			return &Config{}, fmt.Errorf("error creating default config file: %v", err)
-			return &Config{}
 		}
 	}
-	defer func() {
+	defer f.Close()
-		err := f.Close()
-		if err != nil {
-			fmt.Printf("Closing config file failed: %v.", err)
-		}
-	}()
 
 	data, err := ioutil.ReadAll(f)
 	if err != nil {
-		fmt.Printf("Unable to read config data: %v.", err)
+		return &Config{}, fmt.Errorf("unable to read config data: %v", err)
-		return &Config{}
 	}
 
 	var c Config
 	err = yaml.Unmarshal(data, &c)
 	if err != nil {
-		fmt.Printf("Unable to decode config file: %v.", err)
+		return &Config{}, fmt.Errorf("unable to decode config file: %v", err)
-		return &Config{}
 	}
 
 	if len(c.DebugInfoDirectories) == 0 {
 		c.DebugInfoDirectories = []string{"/usr/lib/debug/.build-id"}
 	}
 
-	return &c
+	return &c, nil
 }
 
 // SaveConfig will marshal and save the config struct