Merge pull request #2671 from jesseduffield/global-logging-for-development

CONTRIBUTING.md

@@ -105,30 +105,11 @@ Boy that's a hard word to spell. Anyway, lazygit is translated into several lang
 
 The easiest way to debug lazygit is to have two terminal tabs open at once: one for running lazygit (via `go run main.go -debug` in the project root) and one for viewing lazygit's logs (which can be done via `go run main.go --logs` or just `lazygit --logs`).
 
-From most places in the codebase you have access to a logger e.g. `gui.Log.Warn("blah")`.
+From most places in the codebase you have access to a logger e.g. `gui.Log.Warn("blah")` or `self.c.Log.Warn("blah")`.
 
 If you find that the existing logs are too noisy, you can set the log level with e.g. `LOG_LEVEL=warn go run main.go -debug` and then only use `Warn` logs yourself.
 
-If you need to log from code in the vendor directory (e.g. the `gocui` package), you won't have access to the logger, but you can easily add logging support by adding the following:
+If you need to log from code in the vendor directory (e.g. the `gocui` package), you won't have access to the logger, but you can easily add logging support by setting the `LAZYGIT_LOG_PATH` environment variable and using `logs.Global.Warn("blah")`. This is a global logger that's only intended for development purposes.
-
-```go
-func newLogger() *logrus.Entry {
-	// REPLACE THE BELOW PATH WITH YOUR ACTUAL LOG PATH (YOU'LL SEE THIS PRINTED WHEN YOU RUN `lazygit --logs`
-	logPath := "/Users/jesseduffield/Library/Application Support/jesseduffield/lazygit/development.log"
-	file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666)
-	if err != nil {
-		panic("unable to log to file")
-	}
-	logger := logrus.New()
-	logger.SetLevel(logrus.WarnLevel)
-	logger.SetOutput(file)
-	return logger.WithFields(logrus.Fields{})
-}
-
-var Log = newLogger()
-...
-Log.Warn("blah")
-```
 
 If you keep having to do some setup steps to reproduce an issue, read the Testing section below to see how to create an integration test by recording a lazygit session. It's pretty easy!
 

pkg/app/app.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 
 	"github.com/go-errors/errors"
+	"github.com/sirupsen/logrus"
 
 	"github.com/jesseduffield/generics/slices"
 	appTypes "github.com/jesseduffield/lazygit/pkg/app/types"
@@ -22,6 +23,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/env"
 	"github.com/jesseduffield/lazygit/pkg/gui"
 	"github.com/jesseduffield/lazygit/pkg/i18n"
+	"github.com/jesseduffield/lazygit/pkg/logs"
 	"github.com/jesseduffield/lazygit/pkg/updates"
 )
 
@@ -76,6 +78,18 @@ func NewCommon(config config.AppConfigurer) (*common.Common, error) {
 	}, nil
 }
 
+func newLogger(cfg config.AppConfigurer) *logrus.Entry {
+	if cfg.GetDebug() {
+		logPath, err := config.LogPath()
+		if err != nil {
+			log.Fatal(err)
+		}
+		return logs.NewDevelopmentLogger(logPath)
+	} else {
+		return logs.NewProductionLogger()
+	}
+}
+
 // NewApp bootstrap a new application
 func NewApp(config config.AppConfigurer, common *common.Common) (*App, error) {
 	app := &App{

pkg/app/entry_point.go

@@ -16,7 +16,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/config"
 	"github.com/jesseduffield/lazygit/pkg/env"
 	integrationTypes "github.com/jesseduffield/lazygit/pkg/integration/types"
-	"github.com/jesseduffield/lazygit/pkg/logs"
+	"github.com/jesseduffield/lazygit/pkg/logs/tail"
 	"github.com/jesseduffield/lazygit/pkg/secureexec"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 	"github.com/samber/lo"
@@ -106,7 +106,12 @@ func Start(buildInfo *BuildInfo, integrationTest integrationTypes.IntegrationTes
 	}
 
 	if cliArgs.TailLogs {
-		logs.TailLogs()
+		logPath, err := config.LogPath()
+		if err != nil {
+			log.Fatal(err.Error())
+		}
+
+		tail.TailLogs(logPath)
 		os.Exit(0)
 	}
 

pkg/app/logging.go

@@ -1,56 +0,0 @@
-package app
-
-import (
-	"io"
-	"log"
-	"os"
-
-	"github.com/jesseduffield/lazygit/pkg/config"
-	"github.com/sirupsen/logrus"
-)
-
-func newLogger(config config.AppConfigurer) *logrus.Entry {
-	var log *logrus.Logger
-	if config.GetDebug() {
-		log = newDevelopmentLogger()
-	} else {
-		log = newProductionLogger()
-	}
-
-	// highly recommended: tail -f development.log | humanlog
-	// https://github.com/aybabtme/humanlog
-	log.Formatter = &logrus.JSONFormatter{}
-
-	return log.WithFields(logrus.Fields{})
-}
-
-func newProductionLogger() *logrus.Logger {
-	log := logrus.New()
-	log.Out = io.Discard
-	log.SetLevel(logrus.ErrorLevel)
-	return log
-}
-
-func newDevelopmentLogger() *logrus.Logger {
-	logger := logrus.New()
-	logger.SetLevel(getLogLevel())
-	logPath, err := config.LogPath()
-	if err != nil {
-		log.Fatal(err)
-	}
-	file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666)
-	if err != nil {
-		log.Fatalf("Unable to log to log file: %v", err)
-	}
-	logger.SetOutput(file)
-	return logger
-}
-
-func getLogLevel() logrus.Level {
-	strLevel := os.Getenv("LOG_LEVEL")
-	level, err := logrus.ParseLevel(strLevel)
-	if err != nil {
-		return logrus.DebugLevel
-	}
-	return level
-}

pkg/config/app_config.go

@@ -301,5 +301,9 @@ func getDefaultAppState() *AppState {
 }
 
 func LogPath() (string, error) {
+	if os.Getenv("LAZYGIT_LOG_PATH") != "" {
+		return os.Getenv("LAZYGIT_LOG_PATH"), nil
+	}
+
 	return configFilePath("development.log")
 }

pkg/logs/logs.go

@@ -1,34 +1,64 @@
 package logs
 
 import (
-	"fmt"
+	"io"
 	"log"
 	"os"
 
-	"github.com/aybabtme/humanlog"
+	"github.com/sirupsen/logrus"
-	"github.com/jesseduffield/lazygit/pkg/config"
 )
 
-// TailLogs lets us run `lazygit --logs` to print the logs produced by other lazygit processes.
+// It's important that this package does not depend on any other package because we
-// This makes for easier debugging.
+// may want to import it from anywhere, and we don't want to create a circular dependency
-func TailLogs() {
+// (because Go refuses to compile circular dependencies).
-	logFilePath, err := config.LogPath()
+
+// Global is a global logger that can be used anywhere in the app, for
+// _development purposes only_. I want to avoid global variables when possible,
+// so if you want to log something that's printed when the -debug flag is set,
+// you'll need to ensure the struct you're working with has a logger field (
+// and most of them do).
+// Global is only available if the LAZYGIT_LOG_PATH environment variable is set.
+var Global *logrus.Entry
+
+func init() {
+	logPath := os.Getenv("LAZYGIT_LOG_PATH")
+	if logPath != "" {
+		Global = NewDevelopmentLogger(logPath)
+	}
+}
+
+func NewProductionLogger() *logrus.Entry {
+	logger := logrus.New()
+	logger.Out = io.Discard
+	logger.SetLevel(logrus.ErrorLevel)
+	return formatted(logger)
+}
+
+func NewDevelopmentLogger(logPath string) *logrus.Entry {
+	logger := logrus.New()
+	logger.SetLevel(getLogLevel())
+
+	file, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666)
 	if err != nil {
-		log.Fatal(err)
+		log.Fatalf("Unable to log to log file: %v", err)
+	}
+	logger.SetOutput(file)
+	return formatted(logger)
 }
 
-	fmt.Printf("Tailing log file %s\n\n", logFilePath)
+func formatted(log *logrus.Logger) *logrus.Entry {
+	// highly recommended: tail -f development.log | humanlog
+	// https://github.com/aybabtme/humanlog
+	log.Formatter = &logrus.JSONFormatter{}
 
-	opts := humanlog.DefaultOptions
+	return log.WithFields(logrus.Fields{})
-	opts.Truncates = false
+}
 
-	_, err = os.Stat(logFilePath)
+func getLogLevel() logrus.Level {
+	strLevel := os.Getenv("LOG_LEVEL")
+	level, err := logrus.ParseLevel(strLevel)
 	if err != nil {
-		if os.IsNotExist(err) {
+		return logrus.DebugLevel
-			log.Fatal("Log file does not exist. Run `lazygit --debug` first to create the log file")
 	}
-		log.Fatal(err)
+	return level
-	}
-
-	TailLogsForPlatform(logFilePath, opts)
 }

pkg/logs/tail/logs_default.go

@@ -1,7 +1,7 @@
 //go:build !windows
 // +build !windows
 
-package logs
+package tail
 
 import (
 	"log"
@@ -11,7 +11,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/secureexec"
 )
 
-func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) {
+func tailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) {
 	cmd := secureexec.Command("tail", "-f", logFilePath)
 
 	stdout, _ := cmd.StdoutPipe()

pkg/logs/tail/logs_windows.go

@@ -1,7 +1,7 @@
 //go:build windows
 // +build windows
 
-package logs
+package tail
 
 import (
 	"bufio"
@@ -13,7 +13,7 @@ import (
 	"github.com/aybabtme/humanlog"
 )
 
-func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) {
+func tailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) {
 	var lastModified int64 = 0
 	var lastOffset int64 = 0
 	for {
@@ -22,7 +22,7 @@ func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) {
 			log.Fatal(err)
 		}
 		if stat.ModTime().Unix() > lastModified {
-			err = TailFrom(lastOffset, logFilePath, opts)
+			err = tailFrom(lastOffset, logFilePath, opts)
 			if err != nil {
 				log.Fatal(err)
 			}
@@ -32,7 +32,7 @@ func TailLogsForPlatform(logFilePath string, opts *humanlog.HandlerOptions) {
 	}
 }
 
-func OpenAndSeek(filepath string, offset int64) (*os.File, error) {
+func openAndSeek(filepath string, offset int64) (*os.File, error) {
 	file, err := os.Open(filepath)
 	if err != nil {
 		return nil, err
@@ -46,8 +46,8 @@ func OpenAndSeek(filepath string, offset int64) (*os.File, error) {
 	return file, nil
 }
 
-func TailFrom(lastOffset int64, logFilePath string, opts *humanlog.HandlerOptions) error {
+func tailFrom(lastOffset int64, logFilePath string, opts *humanlog.HandlerOptions) error {
-	file, err := OpenAndSeek(logFilePath, lastOffset)
+	file, err := openAndSeek(logFilePath, lastOffset)
 	if err != nil {
 		return err
 	}

pkg/logs/tail/tail.go

@@ -0,0 +1,28 @@
+package tail
+
+import (
+	"fmt"
+	"log"
+	"os"
+
+	"github.com/aybabtme/humanlog"
+)
+
+// TailLogs lets us run `lazygit --logs` to print the logs produced by other lazygit processes.
+// This makes for easier debugging.
+func TailLogs(logFilePath string) {
+	fmt.Printf("Tailing log file %s\n\n", logFilePath)
+
+	opts := humanlog.DefaultOptions
+	opts.Truncates = false
+
+	_, err := os.Stat(logFilePath)
+	if err != nil {
+		if os.IsNotExist(err) {
+			log.Fatal("Log file does not exist. Run `lazygit --debug` first to create the log file")
+		}
+		log.Fatal(err)
+	}
+
+	tailLogsForPlatform(logFilePath, opts)
+}