diff --git a/backend/system/userdata.go b/backend/system/userdata.go index 9f232fe..665c962 100644 --- a/backend/system/userdata.go +++ b/backend/system/userdata.go @@ -52,6 +52,75 @@ func UseHomeOverride(base string) { _ = os.Setenv(envHomeOverride, base) } +// envTempDir is the variable Go's os.TempDir() reads, and through it +// every library in the process that asks for a temporary file. +const envTempDir = "TMPDIR" + +// tempDirName is the subdirectory of the app's own storage that +// becomes that answer. +const tempDirName = "tmp" + +// UseTempDir gives the process a temporary directory that exists. +// +// **Android has no /tmp and hands an app no TMPDIR**, and Go's +// os.TempDir() falls back to "/tmp" when the variable is unset -- so +// every library in the process that wants scratch space is handed a +// path that has never existed. SQLite is the one that noticed: an +// INSERT ... SELECT large enough to spill returned +// SQLITE_IOERR_GETTEMPPATH (disk I/O error 6410), which is how the +// champion search index came to fail its rebuild on every launch while +// the app otherwise looked healthy (#190). +// +// It is the *class* that is fixed here rather than that statement. +// Anything that spills fails the same way on that platform -- large +// sorts, large joins, VACUUM -- so the repair belongs at the process's +// one answer to the question rather than at each caller. The +// alternative considered was PRAGMA temp_store = MEMORY, which is +// cheaper and more local and is a promise that every future spill fits +// in RAM on a phone; the catalog is the largest thing in this app and +// that is not a promise worth making silently. +// +// The rules are UseHomeOverride's, for the same reasons. **An empty +// base is a no-op**, because that is what +// application.Mobile.StoragePath() returns on desktop -- so this needs +// no build tag and changes nothing off mobile, where /tmp is real. And +// **an explicit TMPDIR wins**, so anyone who set one deliberately gets +// what they asked for; nothing sets it on the platform this exists for. +// +// It returns its error rather than swallowing it because a temp +// directory that could not be created is the same silent failure one +// step earlier, and since #160 a log line on that platform is +// something a person can actually read. +func UseTempDir(base string) error { + if base == "" || os.Getenv(envTempDir) != "" { + return nil + } + + dir := filepath.Join(base, tempDirName) + + if err := os.MkdirAll(dir, os.ModePerm); err != nil { + return fmt.Errorf("could not make the temp directory %s: %w", dir, err) + } + + // Writability is checked rather than assumed: the whole failure + // this repairs is a directory that is named and cannot be used, and + // MkdirAll on an existing unwritable directory succeeds. + probe, err := os.CreateTemp(dir, "probe") + if err != nil { + return fmt.Errorf("temp directory %s is not writable: %w", dir, err) + } + + name := probe.Name() + _ = probe.Close() + _ = os.Remove(name) + + if err := os.Setenv(envTempDir, dir); err != nil { + return fmt.Errorf("could not set %s: %w", envTempDir, err) + } + + return nil +} + // getUserDirPath returns and creates the path for a user directory. func getUserDirPath(dt dirType) (string, error) { path, err := resolveUserDirPath(dt) diff --git a/backend/system/userdata_test.go b/backend/system/userdata_test.go index 3829360..42030a1 100644 --- a/backend/system/userdata_test.go +++ b/backend/system/userdata_test.go @@ -87,3 +87,116 @@ func TestUseHomeOverride(t *testing.T) { }) } } + +// UseTempDir carries UseHomeOverride's two rules for the same reasons, +// plus one of its own: the directory it names has to be usable. +// +// **The only tier that can compile the platform this exists for is a +// phone**, so everything decidable off one is decided here -- which is +// androidpayload.go's discipline, and is why the platform call is a +// parameter rather than something this package reaches for. The +// device's half is a single measurement: no /tmp, no TMPDIR (#190). +func TestUseTempDir(t *testing.T) { + t.Run("an empty base is a no-op", func(t *testing.T) { + // This is the desktop case in full: StoragePath() answers "" + // off mobile, where /tmp is real and must be left alone. + t.Setenv(envTempDir, "") + + if err := UseTempDir(""); err != nil { + t.Fatalf("UseTempDir(\"\") = %v, want nil", err) + } + + if got := os.Getenv(envTempDir); got != "" { + t.Errorf("%s = %q, want it untouched", envTempDir, got) + } + }) + + t.Run("an explicit TMPDIR wins", func(t *testing.T) { + const chosen = "/somewhere/deliberate" + + // The base is taken before TMPDIR moves, because t.TempDir() + // reads TMPDIR too -- which is the same fact this function is + // about, met from the other side. + base := t.TempDir() + + t.Setenv(envTempDir, chosen) + + if err := UseTempDir(base); err != nil { + t.Fatalf("UseTempDir = %v, want nil", err) + } + + if got := os.Getenv(envTempDir); got != chosen { + t.Errorf("%s = %q, want the explicit %q", envTempDir, got, chosen) + } + }) + + t.Run("points at a real directory under the base", func(t *testing.T) { + base := t.TempDir() + + t.Setenv(envTempDir, "") + + if err := UseTempDir(base); err != nil { + t.Fatalf("UseTempDir = %v, want nil", err) + } + + got := os.Getenv(envTempDir) + + want := filepath.Join(base, tempDirName) + if got != want { + t.Fatalf("%s = %q, want %q", envTempDir, got, want) + } + + // The whole failure being repaired is a temp directory that is + // named and does not exist, so naming one is not enough. + info, err := os.Stat(got) + if err != nil { + t.Fatalf("the temp directory was named but not created: %v", err) + } + + if !info.IsDir() { + t.Fatalf("%s is not a directory", got) + } + }) + + t.Run("os.TempDir then answers with it", func(t *testing.T) { + // The point of setting the variable at all: this is what every + // library in the process reads, SQLite's driver included. + base := t.TempDir() + + t.Setenv(envTempDir, "") + + if err := UseTempDir(base); err != nil { + t.Fatalf("UseTempDir = %v, want nil", err) + } + + if got := os.TempDir(); got != filepath.Join(base, tempDirName) { + t.Errorf("os.TempDir() = %q, want the directory we made", got) + } + }) + + t.Run("an unwritable directory is an error, not a silent success", func(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("root can write anywhere, so there is nothing to refuse") + } + + base := t.TempDir() + + // MkdirAll on an existing directory succeeds whatever its + // mode, so without the write probe this case would set TMPDIR + // to a directory nothing can use -- which is the bug again, + // one directory over. + if err := os.Mkdir(filepath.Join(base, tempDirName), 0o500); err != nil { + t.Fatalf("prepare the unwritable directory: %v", err) + } + + t.Setenv(envTempDir, "") + + if err := UseTempDir(base); err == nil { + t.Fatal("UseTempDir accepted a directory it cannot write to") + } + + if got := os.Getenv(envTempDir); got != "" { + t.Errorf("%s was set to %q despite the failure", envTempDir, got) + } + }) +} diff --git a/main.go b/main.go index b489659..e853b3e 100644 --- a/main.go +++ b/main.go @@ -113,6 +113,19 @@ func main() { slog.SetDefault(sLogger) sLogger.Info("starting yellowjacket", "version", version, "commit", commit) + // Android has no /tmp and gives an app no TMPDIR, so anything in + // this process that spills to a temporary file is handed a path that + // does not exist -- see system.UseTempDir. It runs here rather than + // beside UseHomeOverride above because it has something to say when + // it fails and the logger does not exist up there; what matters is + // that it is before NewYellowJacketApp, which opens the database. + // + // A failure is not fatal: it leaves the platform's own answer in + // place, which is what every release before this one ran with. + if err := system.UseTempDir(application.Mobile.StoragePath()); err != nil { + sLogger.Error("could not set up a temp directory", "err", err.Error()) + } + // Start profiling server (pprof + trace). In production builds this // is a no-op — the compiler eliminates all profiling code. stopProfiler := profiling.Start(sLogger)