diff --git a/internal/cli/app.go b/internal/cli/app.go index aff39d8d..4d9ce128 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -3,12 +3,16 @@ package cli import ( "context" "encoding/json" + "fmt" "io" + "log/slog" "os" "github.com/labstack/driga/internal/cleanup" "github.com/labstack/driga/internal/config" "github.com/labstack/driga/internal/desktop" + "github.com/labstack/driga/internal/logfile" + "github.com/labstack/driga/internal/service" "github.com/labstack/driga/internal/view" "github.com/labstack/driga/internal/web" ) @@ -64,9 +68,14 @@ func (AppCmd) Run(g *Globals) error { if handed, err := handOffApp(systemLaunch); handed || err != nil { return err } + restore := appLogging() + defer restore() + // Opened from Finder or at login, an error here would reach no one: the + // app starts with the default settings and says what is wrong instead. cfg, err := config.Load(config.Path()) + var configErr error if err != nil { - return err + cfg, configErr = config.Default(), fmt.Errorf("read %s: %w", config.Path(), err) } ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -93,10 +102,21 @@ func (AppCmd) Run(g *Globals) error { }}, Supervisor: &desktop.Supervisor{Executable: collectorExecutable}, Config: cfg, + ConfigErr: configErr, Version: g.Version, }) } +// appLogging writes the app's log, and its collector's output, to the +// collector log, which driga service logs shows. A launch from Finder or at +// login has nowhere else to write; stderr keeps a terminal run readable. +func appLogging() func() { + sink := io.MultiWriter(&logfile.Writer{Path: service.LogPath()}, os.Stderr) + previous := slog.Default() + slog.SetDefault(slog.New(logfile.TextHandler(sink)).With("process", "app")) + return func() { slog.SetDefault(previous) } +} + // pageActivity keeps the collector's viewer heartbeat fresh while the page is used. func pageActivity(active bool) { if active { diff --git a/internal/cli/app_darwin.go b/internal/cli/app_darwin.go index e970985c..1038c51f 100644 --- a/internal/cli/app_darwin.go +++ b/internal/cli/app_darwin.go @@ -22,12 +22,23 @@ func handOffApp(system bool) (bool, error) { if !handsOff(system, exe) { return false, nil } - return true, handOffCommand(appBundle(exe)).Run() + return true, handOffCommand(appBundle(exe), os.Environ()).Run() } // handOffCommand opens bundle by its path: another copy of Driga.app, such -// as one in ~/Applications, has the same bundle identifier. -func handOffCommand(bundle string) *exec.Cmd { return exec.Command("open", "-a", bundle) } +// as one in ~/Applications, has the same bundle identifier. Launch Services +// does not pass on the caller's environment, so the DRIGA_* variables naming +// the data, config and settings go along explicitly. A running app keeps +// its own. +func handOffCommand(bundle string, environ []string) *exec.Cmd { + args := []string{} + for _, entry := range environ { + if strings.HasPrefix(entry, "DRIGA_") && !strings.HasPrefix(entry, "DRIGA_APP=") { + args = append(args, "--env", entry) + } + } + return exec.Command("open", append(args, "-a", bundle)...) +} // handsOff reports whether to hand off: a launch not made by the system, of // an executable inside an app bundle. A development build runs in place. diff --git a/internal/cli/app_darwin_test.go b/internal/cli/app_darwin_test.go index 30bb6af3..a76c25e1 100644 --- a/internal/cli/app_darwin_test.go +++ b/internal/cli/app_darwin_test.go @@ -37,10 +37,11 @@ func TestAppHandOff(t *testing.T) { } // The hand-off opens the bundle the command line ran from, not whichever -// bundle Launch Services knows by the identifier. +// bundle Launch Services knows by the identifier, with the command line's +// DRIGA_* variables. func TestAppHandOffCommand(t *testing.T) { - cmd := handOffCommand("/Users/someone/Applications/Driga.app") - if want := []string{"open", "-a", "/Users/someone/Applications/Driga.app"}; !slices.Equal(cmd.Args, want) { + cmd := handOffCommand("/Users/someone/Applications/Driga.app", []string{"PATH=/bin", "DRIGA_DATA_DIR=/data dir", "DRIGA_APP=1", "DRIGA_CONFIG="}) + if want := []string{"open", "--env", "DRIGA_DATA_DIR=/data dir", "--env", "DRIGA_CONFIG=", "-a", "/Users/someone/Applications/Driga.app"}; !slices.Equal(cmd.Args, want) { t.Fatalf("hand-off runs %q, want %q", cmd.Args, want) } } diff --git a/internal/cli/app_test.go b/internal/cli/app_test.go index 6116c89d..7798a6b6 100644 --- a/internal/cli/app_test.go +++ b/internal/cli/app_test.go @@ -4,6 +4,9 @@ import ( "bytes" "context" "os" + "path/filepath" + "reflect" + "strings" "testing" "github.com/labstack/driga/internal/config" @@ -121,3 +124,29 @@ func TestAppCommandRunsDesktop(t *testing.T) { t.Fatalf("page handlers: %+v", got.Web) } } + +// Opened from Finder or at login, an unreadable settings file reaches no +// terminal: the app starts with the defaults and is told why. +func TestAppCommandStartsWithBadConfig(t *testing.T) { + readEnv(t) + path := filepath.Join(t.TempDir(), "config.toml") + if err := os.WriteFile(path, []byte("[app]\ntray = \"no\"\n"), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("DRIGA_CONFIG", path) + defer func(prev func(desktop.Options) error) { runDesktop = prev }(runDesktop) + var got desktop.Options + runDesktop = func(opts desktop.Options) error { got = opts; return nil } + defer func(prev bool) { systemLaunch = prev }(systemLaunch) + systemLaunch = true + var out, errout bytes.Buffer + if code := run([]string{"app"}, "test-version", nil, &out, &errout); code != 0 { + t.Fatalf("driga app: %d %s", code, errout.String()) + } + if got.ConfigErr == nil || !strings.Contains(got.ConfigErr.Error(), path) { + t.Fatalf("config error = %v, want one naming %s", got.ConfigErr, path) + } + if !reflect.DeepEqual(got.Config, config.Default()) { + t.Fatal("the app did not start with the default settings") + } +} diff --git a/internal/cli/collect.go b/internal/cli/collect.go index 781e97ab..77995b9d 100644 --- a/internal/cli/collect.go +++ b/internal/cli/collect.go @@ -135,7 +135,7 @@ func (c CollectCmd) Run(g *Globals) error { } dir := config.DataDir() if !c.Once { - restore, err := collectorLogging() + restore, err := collectorLogging(c.Owner) if err != nil { return err } @@ -173,13 +173,15 @@ func watchParent(r io.Reader, cancel context.CancelFunc) { cancel() } -func collectorLogging() (func(), error) { +// collectorLogging writes the collector's log. On Linux a service's journal +// shows it too; the app logs a child's output itself, so that copy is left out. +func collectorLogging(owner string) (func(), error) { w := &logfile.Writer{Path: service.LogPath()} if _, err := w.Write(nil); err != nil { return nil, err } var sink io.Writer = w - if runtime.GOOS == "linux" { + if runtime.GOOS == "linux" && owner != "app" { sink = io.MultiWriter(sink, os.Stderr) } previous := slog.Default() diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index b9e8a51e..5ad75a50 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -138,18 +138,19 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { probe = collector.ReadOwner } owner, held, err := probe(d.DataDir) + restart := restartHint(goos, owner, held) switch { case err != nil: add("collector", "fail", err.Error(), "check access to "+d.DataDir+"/driga.lock") case !held: add("collector", "warn", "not running; snapshot mode has no history", collectorHint(goos)) case d.Store == nil: - add("collector", "warn", "lock held but database unavailable", "driga service restart") + add("collector", "warn", "lock held but database unavailable", restart) default: raw, err := d.Store.Meta("last_pass_at") last, parseErr := time.Parse(time.RFC3339Nano, raw) if err != nil || parseErr != nil || last.After(d.Now) || d.Now.Sub(last) > 3*intervals.Procs { - add("collector", "warn", "lock held but last pass is missing or stale", "driga service restart") + add("collector", "warn", "lock held but last pass is missing or stale", restart) } else { add("collector", "ok", fmt.Sprintf("running (%s, pid %d); last pass %s ago", owner.Kind, owner.PID, d.Now.Sub(last).Round(time.Second)), "") } @@ -157,7 +158,7 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { if d.Store != nil { sources, err := d.Store.Sources() if err != nil { - add("sources", "fail", err.Error(), "check database access; run driga service restart") + add("sources", "fail", err.Error(), "check database access; "+restart) } for _, source := range sources { if source.Name == "otlp" || source.Name == "chat" { @@ -177,7 +178,7 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { detail = "not yet receiving events" } if fresh.Stale || fresh.Status == "" && (source.LastOKAt.After(d.Now) || source.LastErrorAt.After(source.LastOKAt)) { - status, fix = "warn", "driga service restart" + status, fix = "warn", restart } if source.LastError != "" { detail += "; last error: " + source.LastError @@ -187,7 +188,7 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { } procs, err := d.Store.Procs() if err != nil { - add("collector_rss", "fail", err.Error(), "check database access; run driga service restart") + add("collector_rss", "fail", err.Error(), "check database access; "+restart) } else { sigs, _ := signatures.Load(nil) for _, p := range procs { @@ -240,9 +241,9 @@ func Doctor(ctx context.Context, d DoctorDeps) []Check { detail = fmt.Sprintf("%.1f MB footprint (RSS %.1f MB)", float64(footprint)/1e6, float64(rss)/1e6) } if !found { - status, detail, fix = "warn", "collector absent from process snapshot", "driga service start" + status, detail, fix = "warn", "collector absent from process snapshot", restart } else if budget > 50_000_000 { - status, fix = "warn", "driga service restart; inspect driga service logs" + status, fix = "warn", restart+"; inspect driga service logs" } add("collector_rss", status, detail, fix) } diff --git a/internal/cli/doctor_event_sources_test.go b/internal/cli/doctor_event_sources_test.go index ffb89845..59b5d69f 100644 --- a/internal/cli/doctor_event_sources_test.go +++ b/internal/cli/doctor_event_sources_test.go @@ -23,5 +23,5 @@ func TestDoctorQuietEventSources(t *testing.T) { if err := st.Tx(context.Background(), func(tx *store.Tx) error { return tx.SetSourceError("sockets", "probe failed", d.Now) }); err != nil { t.Fatal(err) } - requireCheck(t, Doctor(context.Background(), d), "source:sockets", "warn", "driga service restart") + requireCheck(t, Doctor(context.Background(), d), "source:sockets", "warn", "stop and rerun driga collect") } diff --git a/internal/cli/doctor_test.go b/internal/cli/doctor_test.go index 4a2cf656..0d66ea63 100644 --- a/internal/cli/doctor_test.go +++ b/internal/cli/doctor_test.go @@ -168,7 +168,7 @@ func TestDoctorStaleSource(t *testing.T) { if err := st.Tx(context.Background(), func(tx *store.Tx) error { return tx.SetSourceOK("git", d.Now.Add(-16*time.Minute)) }); err != nil { t.Fatal(err) } - requireCheck(t, Doctor(context.Background(), d), "source:git", "warn", "driga service restart") + requireCheck(t, Doctor(context.Background(), d), "source:git", "warn", "stop and rerun driga collect") } func TestDoctorIntakeRejections(t *testing.T) { @@ -226,8 +226,8 @@ func TestDoctorCadenceAndRSS(t *testing.T) { t.Fatal(err) } checks := Doctor(context.Background(), d) - requireCheck(t, checks, "collector", "warn", "driga service restart") - requireCheck(t, checks, "source:machine", "warn", "driga service restart") + requireCheck(t, checks, "collector", "warn", "stop and rerun driga collect") + requireCheck(t, checks, "source:machine", "warn", "stop and rerun driga collect") c := requireCheck(t, checks, "collector_rss", "warn", "") if !strings.Contains(c.Detail, "51.0 MB") { t.Fatal(c) @@ -447,3 +447,24 @@ func TestDoctorMalformedExcludeWarning(t *testing.T) { t.Fatal(cfg.Exclude) } } + +// The fix restarts whichever collector holds the lock. +func TestRestartHintFollowsOwner(t *testing.T) { + for _, tc := range []struct { + goos, kind string + held bool + want string + }{ + {"darwin", "", false, "driga app"}, + {"linux", "", false, "driga service install"}, + {"darwin", "app", true, "quit and reopen Driga"}, + {"darwin", "service", true, "driga service restart"}, + {"linux", "manual", true, "stop and rerun driga collect"}, + {"windows", "unknown", true, "quit and reopen Driga"}, + {"linux", "unknown", true, "driga service restart"}, + } { + if got := restartHint(tc.goos, collector.Owner{Kind: tc.kind}, tc.held); got != tc.want { + t.Errorf("restartHint(%s, %q, %v) = %q, want %q", tc.goos, tc.kind, tc.held, got, tc.want) + } + } +} diff --git a/internal/cli/service.go b/internal/cli/service.go index 17b44ada..6e278831 100644 --- a/internal/cli/service.go +++ b/internal/cli/service.go @@ -100,6 +100,11 @@ func (c serviceControl) Run(g *Globals) error { } switch c.Action { case "install": + // A service installed while the app collects would only wait for the + // lock, and take over collection from the app once it quits. + if owner, held, err := collector.ReadOwner(config.DataDir()); err == nil && held && owner.Kind == "app" { + return errors.New("the Driga app collects on this computer; quit Driga first to collect with the service instead") + } if runtime.GOOS == "darwin" { // launchd opens these streams before the collector starts. dir := filepath.Dir(service.LogPath()) @@ -184,6 +189,27 @@ func collectorHint(goos string) string { return "driga service install" } +// restartHint says how to restart the collector that holds the lock: the +// app, the service or a collector run by hand. With none holding it, the +// collector is started the way this platform runs it. +func restartHint(goos string, owner collector.Owner, held bool) string { + if !held { + return collectorHint(goos) + } + switch owner.Kind { + case "app": + return "quit and reopen Driga" + case "service": + return "driga service restart" + case "manual": + return "stop and rerun driga collect" + } + if goos == "darwin" || goos == "windows" { + return "quit and reopen Driga" + } + return "driga service restart" +} + func serviceBinaryPath(bin string) (string, error) { if runtime.GOOS == "windows" { // Keep Scoop's stable current junction. The OS can execute this path diff --git a/internal/collector/daemon.go b/internal/collector/daemon.go index 6aa9f750..03f1ee06 100644 --- a/internal/collector/daemon.go +++ b/internal/collector/daemon.go @@ -212,6 +212,34 @@ func (s *schedule) wait(_ time.Time, _ Intervals) time.Duration { return time.Second } +// lockWait is how often a service collector asks again for a lock another +// collector holds; tests shorten it. +var lockWait = 5 * time.Second + +// lockFor takes the collector lock. A service waits while another collector, +// such as the desktop app's, holds it: the service manager would otherwise +// restart it every few seconds for as long as the other one runs. +func lockFor(ctx context.Context, clock daemonClock, dataDir string, owner Owner) (*os.File, error) { + logged := false + for { + lock, err := Lock(dataDir, owner) + if owner.Kind != "service" || !errors.Is(err, ErrAlreadyRunning) { + return lock, err + } + if !logged { + slog.Info("another collector holds the lock; waiting for it to stop", "event", "lock_wait", "err", err.Error()) + logged = true + } + timer := clock.NewTimer(lockWait) + select { + case <-ctx.Done(): + timer.Stop() + return nil, ctx.Err() + case <-timer.Chan(): + } + } +} + // Run collects until cancellation while holding the exclusive writer lock. // Cancellation, by a signal or the parent going away, is a clean stop at // whatever step it lands, startup included: whatever a step returned then @@ -232,7 +260,7 @@ func run(ctx context.Context, d Deps, dataDir string) error { if clock == nil { clock = realDaemonClock{} } - lock, err := Lock(dataDir, d.Owner) + lock, err := lockFor(ctx, clock, dataDir, d.Owner) if err != nil { return err } diff --git a/internal/collector/daemon_test.go b/internal/collector/daemon_test.go index 0a77e5b3..5b899637 100644 --- a/internal/collector/daemon_test.go +++ b/internal/collector/daemon_test.go @@ -39,7 +39,9 @@ func TestRunExclusiveLock(t *testing.T) { if err != nil || !held || owner.Kind != "service" || owner.Version != "test" || owner.PID != os.Getpid() { t.Fatalf("run owner: %+v, %v, %v", owner, held, err) } - err = Run(context.Background(), d, dir) + manual := d + manual.Owner = Owner{Kind: "manual"} + err = Run(context.Background(), manual, dir) if !errors.Is(err, ErrAlreadyRunning) || !strings.Contains(err.Error(), "another driga collector is running (service, pid ") { t.Fatalf("lock: %v", err) } @@ -64,6 +66,45 @@ func TestRunExclusiveLock(t *testing.T) { lock.Close() } +// A service waits for a lock another collector holds, rather than exiting +// for the service manager to restart it, and collects once it is free. +func TestRunServiceWaitsForLock(t *testing.T) { + d, _, _, a := setup(t) + lockWait = 10 * time.Millisecond + t.Cleanup(func() { lockWait = 5 * time.Second }) + d.Owner = Owner{Kind: "service"} + dir := config.DataDir() + app, err := Lock(dir, Owner{Kind: "app"}) + if err != nil { + t.Fatal(err) + } + ready := make(chan struct{}) + a.collect = func(ctx context.Context) { close(ready); <-ctx.Done() } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + done := make(chan error, 1) + go func() { done <- Run(ctx, d, dir) }() + select { + case err := <-done: + t.Fatalf("service exited while the lock was held: %v", err) + case <-ready: + t.Fatal("service collected while the lock was held") + case <-time.After(200 * time.Millisecond): + } + if err := app.Close(); err != nil { + t.Fatal(err) + } + select { + case <-ready: + case <-time.After(5 * time.Second): + t.Fatal("service never took the freed lock") + } + cancel() + if err := <-done; err != nil { + t.Fatal(err) + } +} + func TestCadenceFastWithHeartbeat(t *testing.T) { fast := cadence(time.Second) idle := cadence(5 * time.Second) diff --git a/internal/desktop/app.go b/internal/desktop/app.go index b47fe3df..053625fb 100644 --- a/internal/desktop/app.go +++ b/internal/desktop/app.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "strconv" "strings" "sync" @@ -47,7 +48,10 @@ type Options struct { // fills Start, ReadOwner, Ready and Executable when they are unset. Supervisor *Supervisor Config config.Config - Version string + // ConfigErr is why the settings file could not be read; Config then + // holds the defaults, and the app says so once it is ready. + ConfigErr error + Version string } // shell is the running app: the main window, the tray and the collector. @@ -84,6 +88,8 @@ type shell struct { status Status envErr error serviceErr error + // shadowed are the DRIGA_* variables the login shell sets otherwise. + shadowed []string // restartPending is set once an upgrade's relaunch was cancelled. restartPending bool needs []view.BoardRow @@ -159,6 +165,9 @@ func Run(opts Options) error { } }) mygo.App.WhenReady(func() { + if opts.ConfigErr != nil { + s.reportConfigError(opts.ConfigErr) + } _ = s.applyOpenAtLogin(opts.Config.App.LaunchAtLogin) s.applyTray(opts.Config.App.Tray) if !startHidden(s.goos, mygo.App.WasOpenedAtLogin(), s.trayCreated()) { @@ -211,13 +220,22 @@ func (s *shell) prepareSupervisor(dataDir string) (string, error) { sup.Start = func(ctx context.Context) (Child, error) { // Without the login environment the collector still runs, with // the app's own; the status says what is missing. + if s.upgrade != nil && s.upgrade.pending() { + return nil, errUpgradePending + } env, err := s.loginEnv.get(ctx) + shadowed := shadowedKeys(env, os.Getenv) s.mu.Lock() s.envErr = err + logShadowed := !slices.Equal(s.shadowed, shadowed) + s.shadowed = shadowed s.mu.Unlock() if err != nil { slog.Warn("login environment unavailable", "event", "login_env_failed", "err", err.Error()) } + if logShadowed && len(shadowed) > 0 { + slog.Warn("the login shell sets DRIGA_* variables the app does not use", "event", "login_env_shadowed", "keys", strings.Join(shadowed, ",")) + } return StartCollector(ctx, sup.Executable, env) } } @@ -246,6 +264,16 @@ func (s *shell) replaceService() { }() } +// reportConfigError says the settings could not be read. The collector +// reads the same file, so the tray shows its error until the file is fixed. +func (s *shell) reportConfigError(err error) { + slog.Warn("settings unreadable; using the defaults", "event", "config_invalid", "err", err.Error()) + go func() { + _, _ = mygo.Dialog.Message(mygo.MessageOptions{Parent: s.mainWindow(), Type: mygo.MessageWarning, + Message: "Driga could not read its settings and uses the defaults.", Detail: err.Error() + "\n\nFix the file, then quit and reopen Driga.", Buttons: []string{"OK"}}) + }() +} + // removeService removes the background service when the app replaces it, and // reports whether it did. A failure is kept for the tray, whose status line // then says how to remove the service by hand. @@ -372,6 +400,8 @@ func (s *shell) trayStatus() Status { st.Err = errors.New("background service not removed; run driga service uninstall") case s.envErr != nil && st.Collecting: st.Err = fmt.Errorf("without login shell PATH: %w", s.envErr) + case len(s.shadowed) > 0 && st.Collecting: + st.Err = fmt.Errorf("ignoring %s from the login shell; quit Driga and run driga app from it", strings.Join(s.shadowed, ", ")) } return st } diff --git a/internal/desktop/child.go b/internal/desktop/child.go index 6ba9be27..639e5eba 100644 --- a/internal/desktop/child.go +++ b/internal/desktop/child.go @@ -1,12 +1,15 @@ package desktop import ( + "bytes" "context" "errors" "fmt" "io" + "log/slog" "os" "os/exec" + "strings" "sync" "time" ) @@ -29,7 +32,9 @@ type execChild struct { // StartCollector runs exe as the app's collector with login merged over a // minimal environment. Notifications arrive on one pipe; the other is the -// lifeline. The collector writes its own log, so its output is discarded. +// lifeline. The collector writes its own log once it runs; what it prints +// before that, such as why it could not start, goes to the app's log and +// ends the error Wait returns. func StartCollector(_ context.Context, exe string, login map[string]string) (Child, error) { notifyR, notifyW, err := os.Pipe() if err != nil { @@ -41,6 +46,10 @@ func StartCollector(_ context.Context, exe string, login map[string]string) (Chi } cmd := exec.Command(exe) cmd.Env = childEnvironment(login) + output := &outputLog{} + cmd.Stdout, cmd.Stderr = output, output + // A program the collector started may hold its output open after it exits. + cmd.WaitDelay = time.Second notifyFD, parentFD, err := passFiles(cmd, notifyW, parentR) if err == nil { cmd.Args = append(cmd.Args, "collect", "--owner=app", fmt.Sprintf("--notify-fd=%d", notifyFD), fmt.Sprintf("--parent-fd=%d", parentFD)) @@ -61,6 +70,10 @@ func StartCollector(_ context.Context, exe string, login map[string]string) (Chi c := &execChild{cmd: cmd, notes: &closingReader{f: notifyR}, lifeline: parentW, done: make(chan struct{})} go func() { c.err = cmd.Wait() + output.flush() + if line := output.last(); c.err != nil && line != "" { + c.err = fmt.Errorf("%w: %s", c.err, line) + } _ = c.lifeline.Close() close(c.done) }() @@ -103,3 +116,57 @@ func (r *closingReader) Read(p []byte) (int, error) { } return n, err } + +// outputLog logs each line the collector prints and keeps the last one, +// which is why it exited when it could not start. +type outputLog struct { + mu sync.Mutex + partial []byte + line string +} + +// maxOutputLine bounds a line held while it has no newline yet. +const maxOutputLine = 4 << 10 + +func (o *outputLog) Write(p []byte) (int, error) { + o.mu.Lock() + defer o.mu.Unlock() + o.partial = append(o.partial, p...) + for { + i := bytes.IndexByte(o.partial, '\n') + if i < 0 { + break + } + o.emit(o.partial[:i]) + o.partial = o.partial[i+1:] + } + if len(o.partial) > maxOutputLine { + o.emit(o.partial) + o.partial = nil + } + return len(p), nil +} + +// flush logs a last line that ended without a newline. +func (o *outputLog) flush() { + o.mu.Lock() + defer o.mu.Unlock() + o.emit(o.partial) + o.partial = nil +} + +func (o *outputLog) emit(b []byte) { + line := strings.TrimSpace(string(b)) + if line == "" { + return + } + // The CLI prefixes its fatal error with its name. + o.line = strings.TrimPrefix(line, "driga: ") + slog.Warn("collector output", "event", "collector_output", "line", line) +} + +func (o *outputLog) last() string { + o.mu.Lock() + defer o.mu.Unlock() + return o.line +} diff --git a/internal/desktop/child_test.go b/internal/desktop/child_test.go index 98156748..4ab7e673 100644 --- a/internal/desktop/child_test.go +++ b/internal/desktop/child_test.go @@ -20,8 +20,14 @@ const helperEnv = "DESKTOP_TEST_COLLECTOR" // arguments and environment as a notification, then runs until its parent // closes the lifeline. func TestMain(m *testing.M) { - if os.Getenv(helperEnv) == "" { + switch os.Getenv(helperEnv) { + case "": os.Exit(m.Run()) + case "fail": + // A collector that cannot start says why on stderr, as cli.Main does. + _, _ = os.Stdout.WriteString("starting\n") + _, _ = os.Stderr.WriteString("driga: open store: file is not a database (26)") + os.Exit(1) } args := os.Args[1:] fs := flag.NewFlagSet("collect", flag.ExitOnError) @@ -93,3 +99,34 @@ func TestStartCollectorRunsUntilStopped(t *testing.T) { t.Fatal("notification stream did not end with the child") } } + +// Why a collector could not start ends the error the app reports. +func TestStartCollectorReportsWhyItExited(t *testing.T) { + child, err := StartCollector(context.Background(), os.Args[0], map[string]string{helperEnv: "fail"}) + if err != nil { + t.Fatal(err) + } + _, _ = io.Copy(io.Discard, child.Notifications()) + err = child.Wait() + if err == nil || !strings.HasSuffix(err.Error(), ": open store: file is not a database (26)") { + t.Fatalf("Wait = %v, want the collector's last line", err) + } +} + +func TestOutputLogKeepsLastLine(t *testing.T) { + var o outputLog + _, _ = o.Write([]byte("first\nsec")) + _, _ = o.Write([]byte("ond\n\n \n")) + if got := o.last(); got != "second" { + t.Fatalf("last = %q, want second", got) + } + _, _ = o.Write([]byte("driga: no newline")) + o.flush() + if got := o.last(); got != "no newline" { + t.Fatalf("last = %q, want the unterminated line without the CLI prefix", got) + } + _, _ = o.Write([]byte(strings.Repeat("x", maxOutputLine+1))) + if got := len(o.last()); got != maxOutputLine+1 { + t.Fatalf("long line held back: last has %d bytes", got) + } +} diff --git a/internal/desktop/env.go b/internal/desktop/env.go index d3410398..3670173f 100644 --- a/internal/desktop/env.go +++ b/internal/desktop/env.go @@ -192,3 +192,17 @@ func childEnvironment(login map[string]string) []string { slices.Sort(out) return out } + +// shadowedKeys returns, sorted, the DRIGA_* variables the login environment +// sets to another value than the app's own. The collector uses the app's, so +// a data directory exported in a shell profile is not the one the app shows. +func shadowedKeys(login map[string]string, getenv func(string) string) []string { + var keys []string + for key, value := range login { + if strings.HasPrefix(key, "DRIGA_") && getenv(key) != value { + keys = append(keys, key) + } + } + slices.Sort(keys) + return keys +} diff --git a/internal/desktop/env_test.go b/internal/desktop/env_test.go index 3a4b788e..1a01f0a4 100644 --- a/internal/desktop/env_test.go +++ b/internal/desktop/env_test.go @@ -13,17 +13,14 @@ import ( "time" ) -// fakeShell writes a login shell under the isolated HOME that prints a fixed +// fakeShell writes a login shell in a fresh directory that prints a fixed // environment, with noise a profile might print, and records its arguments. func fakeShell(t *testing.T, body string) string { t.Helper() if runtime.GOOS == "windows" { t.Skip("login shells are a Unix concept") } - dir := filepath.Join(os.Getenv("HOME"), t.Name()) - if err := os.MkdirAll(dir, 0o700); err != nil { - t.Fatal(err) - } + dir := t.TempDir() path := filepath.Join(dir, "shell") if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body), 0o700); err != nil { t.Fatal(err) @@ -212,3 +209,12 @@ func TestChildEnvironmentDrigaFromAppOnly(t *testing.T) { } } } + +func TestShadowedKeys(t *testing.T) { + login := map[string]string{"PATH": "/bin", "DRIGA_DATA_DIR": "/shell", "DRIGA_CONFIG": "/same", "DRIGA_SPOOL_DIR": "/spool"} + own := map[string]string{"DRIGA_CONFIG": "/same", "DRIGA_SPOOL_DIR": "/other"} + got := shadowedKeys(login, func(k string) string { return own[k] }) + if want := []string{"DRIGA_DATA_DIR", "DRIGA_SPOOL_DIR"}; !slices.Equal(got, want) { + t.Fatalf("shadowed = %q, want %q", got, want) + } +} diff --git a/internal/desktop/notify_test.go b/internal/desktop/notify_test.go index 3a94fbe5..2df7797d 100644 --- a/internal/desktop/notify_test.go +++ b/internal/desktop/notify_test.go @@ -3,6 +3,8 @@ package desktop import ( "context" "errors" + "os" + "path/filepath" "slices" "strings" "testing" @@ -358,3 +360,51 @@ func TestClickTargetFromStore(t *testing.T) { t.Fatalf("ClickTarget = %d %v; want local session %d", id, ok, local) } } + +// A bundle's version is read from its Info.plist, never by running the +// executable, which can wait on Gatekeeper after an upgrade. +func TestInstalledVersionReadsBundle(t *testing.T) { + contents := filepath.Join(t.TempDir(), "Driga.app", "Contents") + if err := os.MkdirAll(filepath.Join(contents, "MacOS"), 0o755); err != nil { + t.Fatal(err) + } + plist := ` + + LSEnvironmentDrigaReleaseVersionnested + CFBundleVersion2026.10.17 + DrigaReleaseVersion2026.10.17-alpha +` + if err := os.WriteFile(filepath.Join(contents, "Info.plist"), []byte(plist), 0o644); err != nil { + t.Fatal(err) + } + // No executable exists: running it would fail. + got, err := installedVersion(context.Background(), filepath.Join(contents, "MacOS", "driga")) + if err != nil || got != "2026.10.17-alpha" { + t.Fatalf("installedVersion = %q, %v", got, err) + } + if _, err := plistString(strings.NewReader(plist), "Missing"); err == nil { + t.Fatal("a missing key was found") + } +} + +// Collector restarts wait while the executable changed and its version is +// not read yet. +func TestUpgradeWatchPending(t *testing.T) { + stamp := time.Unix(100, 0) + w := &upgradeWatch{ + exe: "/app/driga", version: "v1", + stat: func(string) (time.Time, error) { return stamp, nil }, + read: func(context.Context, string) (string, error) { return "v1", nil }, + } + w.start() + if w.pending() { + t.Fatal("unchanged executable pending") + } + stamp = time.Unix(200, 0) + if !w.pending() { + t.Fatal("replaced executable not pending") + } + if w.changed(context.Background()) || w.pending() { + t.Fatal("same version still pending after it was read") + } +} diff --git a/internal/desktop/upgrade.go b/internal/desktop/upgrade.go index 30bb8dcb..07d8e820 100644 --- a/internal/desktop/upgrade.go +++ b/internal/desktop/upgrade.go @@ -3,9 +3,14 @@ package desktop import ( "context" "encoding/json" + "encoding/xml" "errors" + "io" "os" "os/exec" + "path/filepath" + "strings" + "sync" "time" ) @@ -21,22 +26,45 @@ type upgradeWatch struct { interval time.Duration stat func(path string) (time.Time, error) read func(ctx context.Context, exe string) (string, error) - stamp time.Time + + mu sync.Mutex + stamp time.Time } func newUpgradeWatch(exe, version string) *upgradeWatch { - return &upgradeWatch{exe: exe, version: version, stat: modTime, read: exeVersion} + return &upgradeWatch{exe: exe, version: version, stat: modTime, read: installedVersion} } +// errUpgradePending holds collector restarts while the executable changed. +var errUpgradePending = errors.New("waiting for Driga to restart after an update") + // start records the executable as it is now. -func (w *upgradeWatch) start() { w.stamp, _ = w.stat(w.exe) } +func (w *upgradeWatch) start() { + stamp, _ := w.stat(w.exe) + w.mu.Lock() + w.stamp = stamp + w.mu.Unlock() +} + +// pending reports whether the executable changed since the watch last read +// its version. A collector started now could be another version than the +// app, and migrate the store the app reads. +func (w *upgradeWatch) pending() bool { + stamp, err := w.stat(w.exe) + w.mu.Lock() + defer w.mu.Unlock() + return err == nil && !w.stamp.IsZero() && !stamp.Equal(w.stamp) +} // changed reports whether the executable now holds another version. It asks // the executable only after its modification time moved, and asks again on // the next check while it cannot answer, as during an upgrade. func (w *upgradeWatch) changed(ctx context.Context) bool { stamp, err := w.stat(w.exe) - if err != nil || stamp.Equal(w.stamp) { + w.mu.Lock() + unchanged := stamp.Equal(w.stamp) + w.mu.Unlock() + if err != nil || unchanged { return false } version, err := w.read(ctx, w.exe) @@ -46,7 +74,9 @@ func (w *upgradeWatch) changed(ctx context.Context) bool { if version != w.version { return true } + w.mu.Lock() w.stamp = stamp + w.mu.Unlock() return false } @@ -81,6 +111,60 @@ func modTime(path string) (time.Time, error) { return info.ModTime(), nil } +// installedVersion reads the version of the app bundle the executable is +// in from its Info.plist, and asks any other executable. Running a freshly +// installed bundle can wait on Gatekeeper's first-launch prompt. +func installedVersion(ctx context.Context, exe string) (string, error) { + contents := filepath.Dir(filepath.Dir(exe)) + if filepath.Base(filepath.Dir(exe)) == "MacOS" && filepath.Base(contents) == "Contents" && strings.HasSuffix(filepath.Dir(contents), ".app") { + f, err := os.Open(filepath.Join(contents, "Info.plist")) + if err != nil { + return "", err + } + defer f.Close() + return plistString(f, "DrigaReleaseVersion") + } + return exeVersion(ctx, exe) +} + +// plistString returns the string under key in an XML property list's +// top-level dictionary. +func plistString(r io.Reader, key string) (string, error) { + dec := xml.NewDecoder(r) + depth, found := 0, false + for { + tok, err := dec.Token() + if err != nil { + if errors.Is(err, io.EOF) { + return "", errors.New(key + " not in Info.plist") + } + return "", err + } + switch t := tok.(type) { + case xml.StartElement: + depth++ + // plist > dict > key/string: the top-level entries sit at depth 3. + if depth != 3 { + continue + } + var text string + if err := dec.DecodeElement(&text, &t); err != nil { + return "", err + } + depth-- + if found { + if t.Name.Local != "string" || text == "" { + return "", errors.New(key + " is not a string") + } + return text, nil + } + found = t.Name.Local == "key" && text == key + case xml.EndElement: + depth-- + } + } +} + // exeVersion asks a driga executable for its version. func exeVersion(ctx context.Context, exe string) (string, error) { ctx, cancel := context.WithTimeout(ctx, 10*time.Second) diff --git a/site/src/content/docs/start/install.mdx b/site/src/content/docs/start/install.mdx index 65519e79..1423750c 100644 --- a/site/src/content/docs/start/install.mdx +++ b/site/src/content/docs/start/install.mdx @@ -27,6 +27,11 @@ new version. If an earlier release installed the collector service, the app removes it on its first launch, says so once, and collects itself. +The app reads `DRIGA_*` variables from its own environment, not your shell +profile: `driga app` passes the terminal's on to it. It logs to the collector +log, which `driga service logs` prints, and `driga service install` refuses +while the app collects. +