From b66e9054824270a934fba64bffc185501ef7bfc1 Mon Sep 17 00:00:00 2001 From: Spencer Brower Date: Thu, 18 Jun 2026 11:58:33 -0400 Subject: [PATCH] refactor: Gave db its own allocator. --- cmd_check.odin | 2 - cmd_list.odin | 2 - config.odin | 21 +++++----- db.odin | 110 +++++++++++++++++++++++++++++++++++-------------- db_test.odin | 103 +++++++++++++++++---------------------------- 5 files changed, 126 insertions(+), 112 deletions(-) diff --git a/cmd_check.odin b/cmd_check.odin index 3d926d1..e59b6d8 100644 --- a/cmd_check.odin +++ b/cmd_check.odin @@ -54,8 +54,6 @@ cmd_check :: proc(cmd: ^Command) { if !list_ok { return } - defer delete(db_files) - defer for &file in db_files {delete_envfile(&file)} not_backed := find_unbacked(files_in_path[:], db_files[:]) diff --git a/cmd_list.odin b/cmd_list.odin index 17157b2..f6c4e6f 100644 --- a/cmd_list.odin +++ b/cmd_list.odin @@ -25,8 +25,6 @@ cmd_list :: proc(cmd: ^Command) { if !list_ok { return } - defer delete(rows) - defer for &row in rows {delete_envfile(&row)} if terminal.is_terminal(os.stdout) { headers := []string{"Directory", "Path"} diff --git a/config.odin b/config.odin index b313bad..9373d83 100644 --- a/config.odin +++ b/config.odin @@ -25,17 +25,16 @@ Config :: struct { config_path: string `json:"-"`, } -load_config :: proc(config_path: string) -> (Config, bool) { - data, read_err := os.read_entire_file_from_path(config_path, context.allocator) +load_config :: proc(config_path: string, allocator := context.allocator) -> (Config, bool) { + // TODO: Should we use context.allocator + defer delete()? + data, read_err := os.read_entire_file_from_path(config_path, context.temp_allocator) if read_err != nil { fmt.println("No config file found. Please run `envr init` to generate one.") return Config{}, false } - defer delete(data) cfg: Config - // TODO: use json 5 - err := json.unmarshal(data, &cfg) + err := json.unmarshal(data, &cfg, .JSON5, allocator) if err != nil { fmt.printf("Error parsing config: %v\n", err) return Config{}, false @@ -53,22 +52,22 @@ default_config_path :: proc(home: string, allocator := context.allocator) -> str return path } -delete_config :: proc(cfg: ^Config) { +delete_config :: proc(cfg: ^Config, allocator := context.allocator) { for key in cfg.Keys { - delete(key.Private) - delete(key.Public) + delete(key.Private, allocator) + delete(key.Public, allocator) } delete(cfg.Keys) - delete(cfg.ScanConfig.Matcher) + delete(cfg.ScanConfig.Matcher, allocator) for exclude in cfg.ScanConfig.Exclude { - delete(exclude) + delete(exclude, allocator) } delete(cfg.ScanConfig.Exclude) for include in cfg.ScanConfig.Include { - delete(include) + delete(include, allocator) } delete(cfg.ScanConfig.Include) } diff --git a/db.odin b/db.odin index 8ccd190..99f2554 100644 --- a/db.odin +++ b/db.odin @@ -5,6 +5,7 @@ import "core:encoding/hex" import "core:encoding/ini" import "core:encoding/json" import "core:fmt" +import "core:mem" import "core:os" import "core:path/filepath" import "core:strings" @@ -31,6 +32,7 @@ Db :: struct { db: ^rawptr, cfg: Config, changed: bool, + arena: mem.Dynamic_Arena, } EnvFile :: struct { @@ -51,25 +53,15 @@ delete_envfile :: proc(f: ^EnvFile) { delete(f.contents) } -db_open :: proc(cfg_path: string) -> (database: Db, ok: bool) { - database.cfg = load_config(cfg_path) or_return +db_open :: proc(cfg_path: string) -> (Db, bool) { + database, ok := db_init() + if !ok { + return database, ok + } - { - db: ^rawptr - rc := sqlite.db_open(":memory:", &db) - if rc != sqlite.OK { - fmt.printf("Error opening in-memory database: %s\n", sqlite.db_errmsg(db)) - return - } - - create_sql: cstring = "CREATE TABLE IF NOT EXISTS envr_env_files (path TEXT PRIMARY KEY NOT NULL, remotes TEXT, sha256 TEXT NOT NULL, contents TEXT NOT NULL)" - rc = sqlite.db_exec(db, create_sql, nil, nil, nil) - if rc != sqlite.OK { - fmt.printf("Error creating table: %s\n", sqlite.db_errmsg(db)) - sqlite.db_close(db) - return - } - database.db = db + database.cfg, ok = load_config(cfg_path, db_allocator(&database)) + if !ok { + return database, ok } // TODO: Use different allocators? @@ -77,7 +69,7 @@ db_open :: proc(cfg_path: string) -> (database: Db, ok: bool) { if os.exists(data_path) { if ok = db_restore_from_encrypted(&database, data_path); !ok { sqlite.db_close(database.db) - return + return database, ok } } else { // DB was created @@ -87,6 +79,36 @@ db_open :: proc(cfg_path: string) -> (database: Db, ok: bool) { return database, true } +// Creates a database an allocator and fresh, empty table, with zero encryption. +// In production, you most likely want to use `db_open`. +db_init :: proc() -> (database: Db, ok: bool) { + db: ^rawptr + rc := sqlite.db_open(":memory:", &db) + if rc != sqlite.OK { + fmt.printf("Error opening in-memory database: %s\n", sqlite.db_errmsg(db)) + return + } + + create_sql: cstring = "CREATE TABLE IF NOT EXISTS envr_env_files (path TEXT PRIMARY KEY NOT NULL, remotes TEXT, sha256 TEXT NOT NULL, contents TEXT NOT NULL)" + rc = sqlite.db_exec(db, create_sql, nil, nil, nil) + if rc != sqlite.OK { + fmt.printf("Error creating table: %s\n", sqlite.db_errmsg(db)) + sqlite.db_close(db) + return + } + database.db = db + + mem.dynamic_arena_init(&database.arena) + + return database, true +} + +// TODO: Should allocator live on the db? +db_allocator :: proc(db: ^Db) -> mem.Allocator { + return mem.dynamic_arena_allocator(&db.arena) +} + +// TODO: use db allocator? db_restore_from_encrypted :: proc(db: ^Db, data_path: string) -> bool { encrypted_data, read_err := os.read_entire_file_from_path(data_path, context.allocator) defer delete(encrypted_data) @@ -95,6 +117,7 @@ db_restore_from_encrypted :: proc(db: ^Db, data_path: string) -> bool { return false } + // TODO: allow allocator selection? plaintext, dec_ok := decrypt(encrypted_data, db.cfg.Keys[:]) if !dec_ok { fmt.println("Error: decryption failed") @@ -127,9 +150,15 @@ db_restore_from_encrypted :: proc(db: ^Db, data_path: string) -> bool { return true } +// TODO: Return error enum? db_close :: proc(d: ^Db) { - defer sqlite.db_close(d.db) - defer delete_config(&d.cfg) + allocator := db_allocator(d) + + defer { + sqlite.db_close(d.db) + delete_config(&d.cfg, allocator) + mem.dynamic_arena_destroy(&d.arena) + } if d.changed { rc := sqlite.db_exec(d.db, "VACUUM", nil, nil, nil) @@ -147,13 +176,15 @@ db_close :: proc(d: ^Db) { defer sqlite.free(data) sqlite_data := data[:sz] + // TODO: PAss allocator chain + // FIXME Warning gets printed in tests. encrypted, enc_ok := encrypt(sqlite_data, d.cfg.Keys[:]) if !enc_ok { fmt.println("Error: encryption failed") return } - data_path := data_path(d.cfg.config_path) + data_path := data_path(d.cfg.config_path, allocator) envr_d := envr_dir(d.cfg.config_path) os.mkdir_all(envr_d) @@ -168,7 +199,8 @@ db_close :: proc(d: ^Db) { } } -db_list :: proc(d: ^Db, allocator := context.allocator) -> (results: [dynamic]EnvFile, ok: bool) { +// Results will be freed when `db_close` is called. +db_list :: proc(d: ^Db) -> ([]EnvFile, bool) { stmt: ^rawptr rc := sqlite.prepare_v2( d.db, @@ -179,18 +211,22 @@ db_list :: proc(d: ^Db, allocator := context.allocator) -> (results: [dynamic]En ) if rc != sqlite.OK { fmt.printf("Error preparing query: %s\n", sqlite.db_errmsg(d.db)) - return + return []EnvFile{}, false } defer sqlite.finalize(stmt) + allocator := db_allocator(d) + // TODO: Is 10 a good size? + results := make([dynamic]EnvFile, 0, 10, allocator) + for { rc = sqlite.step(stmt) if rc == sqlite.DONE { break } - if rc != sqlite.ROW { + #no_bounds_check if rc != sqlite.ROW { fmt.printf("Error stepping query: %s\n", sqlite.db_errmsg(d.db)) - return + return results[:], false } remotes_json := string(sqlite.column_text(stmt, 1)) @@ -212,10 +248,10 @@ db_list :: proc(d: ^Db, allocator := context.allocator) -> (results: [dynamic]En ) } - ok = true - return + #no_bounds_check return results[:], true } +// FIXME: Needs to use the allocator db_insert :: proc(d: ^Db, file: EnvFile) -> bool { remotes_json, marshal_err := json.marshal(file.Remotes) if marshal_err != nil { @@ -278,7 +314,8 @@ db_insert :: proc(d: ^Db, file: EnvFile) -> bool { return true } -db_fetch :: proc(d: ^Db, path: string, allocator := context.allocator) -> (EnvFile, bool) { +// Result will be freed when `db_close` is called. +db_fetch :: proc(d: ^Db, path: string) -> (EnvFile, bool) { sql: cstring = "SELECT path, remotes, sha256, contents FROM envr_env_files WHERE path = ?" stmt: ^rawptr rc := sqlite.prepare_v2(d.db, sql, -1, &stmt, nil) @@ -288,8 +325,8 @@ db_fetch :: proc(d: ^Db, path: string, allocator := context.allocator) -> (EnvFi } defer sqlite.finalize(stmt) - cpath := to_cstring(path, allocator) - defer delete(cpath, allocator) + // TODO: Use standard allocator + delete here? + cpath := to_cstring(path, context.temp_allocator) rc = sqlite.bind_text(stmt, 1, cpath, -1, nil) if rc != sqlite.OK { fmt.printf("Error binding path: %s\n", sqlite.db_errmsg(d.db)) @@ -305,13 +342,15 @@ db_fetch :: proc(d: ^Db, path: string, allocator := context.allocator) -> (EnvFi return EnvFile{}, false } + allocator := db_allocator(d) + remotes_json := string(sqlite.column_text(stmt, 1)) remotes: [dynamic]string = --- if len(remotes_json) > 0 { json.unmarshal_string(remotes_json, &remotes, allocator = allocator) } - file_path := clone_cstring(sqlite.column_text(stmt, 0)) + file_path := clone_cstring(sqlite.column_text(stmt, 0), allocator) return EnvFile { Path = file_path, @@ -333,6 +372,7 @@ db_delete :: proc(d: ^Db, path: string) -> bool { } defer sqlite.finalize(stmt) + // TODO: Use db allocator here? cpath := to_cstring(path) defer delete(cpath) rc = sqlite.bind_text(stmt, 1, cpath, -1, nil) @@ -392,6 +432,7 @@ db_sync :: proc(d: ^Db, f: ^EnvFile) -> (SyncFlag, string) { } // If SyncFlag is .BackedUp, Caller is responsible for calling delete on f.contents and f.Sha256 +// TODO: Should this use the db allocator? env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncFlag, string) { result: SyncFlag = {} @@ -463,6 +504,7 @@ env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncFlag, str return result, "" } +// TODO: Should this use the db allocator? find_moved_dirs :: proc(d: ^Db, f: ^EnvFile) -> ([dynamic]string, bool) { roots, roots_ok := find_git_roots(d.cfg) if !roots_ok { @@ -480,6 +522,7 @@ find_moved_dirs :: proc(d: ^Db, f: ^EnvFile) -> ([dynamic]string, bool) { return moved, true } +// TODO: Should this use the db allocator? update_dir :: proc(f: ^EnvFile, new_dir: string) { f.Dir = new_dir base := filepath.base(f.Path) @@ -491,6 +534,8 @@ update_dir :: proc(f: ^EnvFile, new_dir: string) { // Loads the contents of the the file at f.Path into f.contents // // Caller is responsible for calling delete on f.contents and f.Sha256 +// +// TODO: Should this use the db allocator? env_file_backup :: proc(f: ^EnvFile) -> bool { data, read_err := os.read_entire_file_from_path(f.Path, context.allocator) if read_err != nil { @@ -520,6 +565,7 @@ shares_remote :: proc(f: ^EnvFile, remotes: []string) -> bool { return false } +// TODO: Should this use the db allocator? get_git_remotes :: proc(dir: string) -> [dynamic]string { remotes: [dynamic]string remote_set: map[string]bool diff --git a/db_test.odin b/db_test.odin index bf6a91f..06c0605 100644 --- a/db_test.odin +++ b/db_test.odin @@ -8,23 +8,6 @@ import "core:testing" import "sqlite" -make_test_db :: proc() -> (Db, bool) { - db: ^rawptr - rc := sqlite.db_open(":memory:", &db) - if rc != sqlite.OK { - return Db{}, false - } - - create_sql: cstring = "CREATE TABLE IF NOT EXISTS envr_env_files (path TEXT PRIMARY KEY NOT NULL, remotes TEXT, sha256 TEXT NOT NULL, contents TEXT NOT NULL)" - rc = sqlite.db_exec(db, create_sql, nil, nil, nil) - if rc != sqlite.OK { - sqlite.db_close(db) - return Db{}, false - } - - return Db{db = db}, true -} - make_test_env_file :: proc(path, sha, contents: string, remotes: []string = {}) -> EnvFile { f := EnvFile { Path = path, @@ -41,10 +24,10 @@ make_test_env_file :: proc(path, sha, contents: string, remotes: []string = {}) @(test) test_db_insert_and_fetch :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) path := "/project/.env" sha := "abc123" @@ -56,7 +39,7 @@ test_db_insert_and_fetch :: proc(t: ^testing.T) { testing.expect(t, db_insert(&d, f), "insert should succeed") fetched, fetch_ok := db_fetch(&d, "/project/.env") - defer delete_envfile(&fetched) + // defer delete_envfile(&fetched) testing.expect(t, fetch_ok, "fetch should succeed") if !fetch_ok do return @@ -69,10 +52,10 @@ test_db_insert_and_fetch :: proc(t: ^testing.T) { @(test) test_db_fetch_missing :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) _, fetch_ok := db_fetch(&d, "/nonexistent/.env") testing.expect(t, !fetch_ok, "fetch missing should return false") @@ -80,10 +63,9 @@ test_db_fetch_missing :: proc(t: ^testing.T) { @(test) test_db_insert_or_replace :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() + defer db_close(&d) testing.expect(t, ok, "failed to create test db") - if !ok do return - defer sqlite.db_close(d.db) f1 := make_test_env_file("/project/.env", "sha1", "KEY=old") defer delete(f1.Remotes) @@ -95,18 +77,11 @@ test_db_insert_or_replace :: proc(t: ^testing.T) { results, list_ok := db_list(&d) testing.expect(t, list_ok, "list should succeed") - if !list_ok do return - defer delete(results) - for &result in results { - defer delete_envfile(&result) - } testing.expect(t, len(results) == 1, "should have 1 row, not 2") fetched, fetch_ok := db_fetch(&d, "/project/.env") testing.expect(t, fetch_ok, "fetch should succeed") - if !fetch_ok do return - defer delete_envfile(&fetched) testing.expect_value(t, fetched.contents, "KEY=new") testing.expect_value(t, fetched.Sha256, "sha2") @@ -114,10 +89,10 @@ test_db_insert_or_replace :: proc(t: ^testing.T) { @(test) test_db_delete_existing :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) f := make_test_env_file("/project/.env", "sha", "KEY=val") defer delete(f.Remotes) @@ -131,20 +106,19 @@ test_db_delete_existing :: proc(t: ^testing.T) { @(test) test_db_delete_missing :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) testing.expect(t, !db_delete(&d, "/nonexistent/.env"), "delete missing should return false") } @(test) test_db_list_multiple :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") - if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) f1 := make_test_env_file("/proj1/.env", "sha1", "A=1", []string{"git@github.com:a/repo.git"}) defer delete(f1.Remotes) @@ -158,36 +132,27 @@ test_db_list_multiple :: proc(t: ^testing.T) { results, list_ok := db_list(&d) testing.expect(t, list_ok, "list should succeed") - if !list_ok do return - defer delete(results) - defer { - for &result in results { - delete_envfile(&result) - } - } testing.expect_value(t, len(results), 3) } @(test) test_db_list_empty :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") - if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) results, list_ok := db_list(&d) testing.expect(t, list_ok, "list should succeed on empty db") testing.expect(t, len(results) == 0, "should have 0 rows") - if list_ok do delete(results) } @(test) test_db_insert_sets_changed :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) testing.expect(t, !d.changed, "changed should start false") @@ -200,10 +165,10 @@ test_db_insert_sets_changed :: proc(t: ^testing.T) { @(test) test_db_delete_sets_changed :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) f := make_test_env_file("/project/.env", "sha", "KEY=val") defer delete(f.Remotes) @@ -216,10 +181,10 @@ test_db_delete_sets_changed :: proc(t: ^testing.T) { @(test) test_db_serialize :: proc(t: ^testing.T) { - d, ok := make_test_db() + d, ok := db_init() testing.expect(t, ok, "failed to create test db") if !ok do return - defer sqlite.db_close(d.db) + defer db_close(&d) f := make_test_env_file("/project/.env", "sha", "KEY=val") defer delete(f.Remotes) @@ -464,7 +429,7 @@ test_update_dir :: proc(t: ^testing.T) { Dir = "/old/project", Remotes = make([dynamic]string, 0), } - defer delete_envfile(&f) + defer delete(f.Remotes) update_dir(&f, "/new/location") @@ -481,13 +446,14 @@ test_closing_db_has_no_leaks :: proc(t: ^testing.T) { cfg_path, err := filepath.join([]string{base, "config.json"}, context.temp_allocator) testing.expect(t, err == nil, "cfgPath should build successfully") - cfg := new_config([]string{"fixtures/keys/insecure-test-key"}, cfg_path) - defer delete_config(&cfg) - testing.expect(t, save_config(cfg, force = true), "save should succeed") + { + cfg := new_config([]string{"fixtures/keys/insecure-test-key"}, cfg_path) + testing.expect(t, save_config(cfg, force = true), "save should succeed") + delete_config(&cfg) + } db, ok := db_open(cfg_path) testing.expect(t, ok, "db should open") - if !ok do return db_close(&db) } @@ -500,15 +466,22 @@ test_open_existing_db_has_no_leaks :: proc(t: ^testing.T) { cfg_path, err := filepath.join([]string{base, "config.json"}, context.temp_allocator) testing.expect(t, err == nil, "cfgPath should build successfully") - cfg := new_config([]string{"fixtures/keys/insecure-test-key"}, cfg_path) - defer delete_config(&cfg) - testing.expect(t, save_config(cfg, force = true), "save should succeed") + { + cfg := new_config([]string{"fixtures/keys/insecure-test-key"}, cfg_path) + testing.expect(t, save_config(cfg, force = true), "save should succeed") + delete_config(&cfg) + } // First open/close creates data.envr on disk db, ok := db_open(cfg_path) testing.expect(t, ok, "db should open") if !ok do return - f := make_test_env_file("/project/.env", "abc123", "SECRET=value", []string{"git@github.com:user/repo.git"}) + f := make_test_env_file( + "/project/.env", + "abc123", + "SECRET=value", + []string{"git@github.com:user/repo.git"}, + ) defer delete(f.Remotes) testing.expect(t, db_insert(&db, f), "insert should succeed") db_close(&db)