spencer/envr
1
0
Fork 0
mirror of https://github.com/sbrow/envr.git synced 2026-06-27 18:48:33 -04:00
Code Issues Packages Projects Releases Wiki Activity

fix: Fixed logic bug in db.

Browse Source
This commit is contained in:
Spencer Brower
2026-06-12 17:16:51 -04:00
parent d620e2646e
commit b7fdb88f34
4 changed files with 38 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
TODOS.md
View File

@@ -10,8 +10,6 @@ Note: These todos can wait until all the subcommands have been ported.
30. **cmd_sync.odin:46-50, 64-68** — Double `db_insert` when `BackedUp`: first insert on line 48, then `db_update_required` is also true for `BackedUp` so second insert runs on line 65. Redundant and wasteful. 30. **cmd_sync.odin:46-50, 64-68** — Double `db_insert` when `BackedUp`: first insert on line 48, then `db_update_required` is also true for `BackedUp` so second insert runs on line 65. Redundant and wasteful.
31. **db.odin:626 & env_file.go:183** — `BackedUp` discards `DirUpdated`. When `TrustFilesystem` is used and the hash differs, the result is just `BackedUp` (not `BackedUp | DirUpdated`). If a file's directory was moved AND its contents changed, the old DB entry won't be deleted because the `DirUpdated` check at `cmd_sync.odin:59` never fires. Bug exists in both Go and Odin.
## MEDIUM ## MEDIUM
4. **db.odin:29-35** — `make_temp_path` never calls `strings.builder_destroy`. Leaks builder buffer every call. 4. **db.odin:29-35** — `make_temp_path` never calls `strings.builder_destroy`. Leaks builder buffer every call.

18
cmd_sync.odin
View File

@@ -11,6 +11,7 @@ SyncEntry :: struct {
Status: string `json:"status"`, Status: string `json:"status"`,
} }
// TODO: Check for quiet failures.
cmd_sync :: proc(cmd: ^Command) { cmd_sync :: proc(cmd: ^Command) {
db, db_ok := db_open() db, db_ok := db_open()
if !db_ok { if !db_ok {
@@ -33,28 +34,25 @@ cmd_sync :: proc(cmd: ^Command) {
result, err_msg := db_sync(&db, &file) result, err_msg := db_sync(&db, &file)
status: string status: string
s := i32(result) is_dir_updated := .DirUpdated in result
is_error := (s & i32(SyncResult.Error)) != 0
is_backed := (s & i32(SyncResult.BackedUp)) != 0
is_restored := (s & i32(SyncResult.Restored)) != 0
is_dir_updated := (s & i32(SyncResult.DirUpdated)) != 0
if is_error { switch {
case .Error in result:
if len(err_msg) > 0 { if len(err_msg) > 0 {
status = err_msg status = err_msg
} else { } else {
status = "error" status = "error"
} }
} else if is_backed { case .BackedUp in result:
status = "Backed Up" status = "Backed Up"
if !db_insert(&db, file) { if !db_insert(&db, file) {
return return
} }
} else if is_restored { case .Restored in result:
status = "Restored" status = "Restored"
} else if is_dir_updated && !is_restored { case .DirUpdated in result:
status = "Moved" status = "Moved"
} else { case:
status = "OK" status = "OK"
} }

49
db.odin
View File

@@ -12,14 +12,16 @@ import "core:time"
import "sqlite" import "sqlite"
SyncResult :: enum i32 { SyncFlagEnum :: enum {
Noop = 0, Noop,
DirUpdated = 1, DirUpdated,
Restored = 1 << 1, Restored,
BackedUp = 1 << 2, BackedUp,
Error = 1 << 3, Error,
} }
SyncFlag :: bit_set[SyncFlagEnum]
SyncDirection :: enum { SyncDirection :: enum {
TrustDatabase, TrustDatabase,
TrustFilesystem, TrustFilesystem,
@@ -449,9 +451,8 @@ string_to_cstring :: proc(s: string) -> cstring {
return cs return cs
} }
db_update_required :: proc(status: SyncResult) -> bool { db_update_required :: proc(status: SyncFlag) -> bool {
s := i32(status) return .BackedUp in status || .DirUpdated in status
return (s & (i32(SyncResult.BackedUp) | i32(SyncResult.DirUpdated))) != 0
} }
shares_remote :: proc(f: ^EnvFile, remotes: []string) -> bool { shares_remote :: proc(f: ^EnvFile, remotes: []string) -> bool {
@@ -510,8 +511,8 @@ env_file_backup :: proc(f: ^EnvFile) -> bool {
return true return true
} }
env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncResult, string) { env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncFlag, string) {
result: SyncResult = .Noop result: SyncFlag = {}
err_msg: string err_msg: string
_, stat_err := os.stat(f.Dir, context.allocator) _, stat_err := os.stat(f.Dir, context.allocator)
@@ -521,18 +522,18 @@ env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncResult, s
if d != nil { if d != nil {
dirs, dirs_ok := find_moved_dirs(d, f) dirs, dirs_ok := find_moved_dirs(d, f)
if !dirs_ok { if !dirs_ok {
return .Error, "failed to find moved dirs" return {.Error}, "failed to find moved dirs"
} }
moved_dirs = dirs moved_dirs = dirs
} }
if len(moved_dirs) == 0 { if len(moved_dirs) == 0 {
return .Error, "directory missing" return {.Error}, "directory missing"
} else if len(moved_dirs) == 1 { } else if len(moved_dirs) == 1 {
update_dir(f, moved_dirs[0]) update_dir(f, moved_dirs[0])
result = .DirUpdated result = {.DirUpdated}
} else { } else {
return .Error, "multiple directories found" return {.Error}, "multiple directories found"
} }
} }
@@ -541,11 +542,10 @@ env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncResult, s
write_err := os.write_entire_file(f.Path, f.contents) write_err := os.write_entire_file(f.Path, f.contents)
if write_err != nil { if write_err != nil {
msg, _ := strings.concatenate({"failed to write file: ", fmt.tprintf("%v", write_err)}) msg, _ := strings.concatenate({"failed to write file: ", fmt.tprintf("%v", write_err)})
return .Error, msg return {.Error}, msg
} }
s := i32(result) | i32(SyncResult.Restored) return result + {.Restored}, ""
return SyncResult(s), ""
} }
data, read_err := os.read_entire_file_from_path(f.Path, context.allocator) data, read_err := os.read_entire_file_from_path(f.Path, context.allocator)
@@ -553,7 +553,7 @@ env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncResult, s
msg, _ := strings.concatenate( msg, _ := strings.concatenate(
{"failed to read file for SHA comparison: ", fmt.tprintf("%v", read_err)}, {"failed to read file for SHA comparison: ", fmt.tprintf("%v", read_err)},
) )
return .Error, msg return {.Error}, msg
} }
digest := hash.hash_bytes(hash.Algorithm.SHA256, data) digest := hash.hash_bytes(hash.Algorithm.SHA256, data)
@@ -569,21 +569,20 @@ env_file_sync :: proc(f: ^EnvFile, dir: SyncDirection, d: ^Db) -> (SyncResult, s
write_err := os.write_entire_file(f.Path, f.contents) write_err := os.write_entire_file(f.Path, f.contents)
if write_err != nil { if write_err != nil {
msg, _ := strings.concatenate({"failed to write file: ", fmt.tprintf("%v", write_err)}) msg, _ := strings.concatenate({"failed to write file: ", fmt.tprintf("%v", write_err)})
return .Error, msg return {.Error}, msg
} }
s := i32(result) | i32(SyncResult.Restored) return result + {.Restored}, ""
return SyncResult(s), ""
case .TrustFilesystem: case .TrustFilesystem:
if !env_file_backup(f) { if !env_file_backup(f) {
return .Error, "failed to backup file" return {.Error}, "failed to backup file"
} }
return .BackedUp, "" return result + {.BackedUp}, ""
} }
return result, "" return result, ""
} }
db_sync :: proc(d: ^Db, f: ^EnvFile) -> (SyncResult, string) { db_sync :: proc(d: ^Db, f: ^EnvFile) -> (SyncFlag, string) {
return env_file_sync(f, .TrustFilesystem, d) return env_file_sync(f, .TrustFilesystem, d)
} }

13
db_test.odin
View File

@@ -4,33 +4,32 @@ import "core:testing"
@(test) @(test)
test_db_update_required_noop :: proc(t: ^testing.T) { test_db_update_required_noop :: proc(t: ^testing.T) {
testing.expect(t, !db_update_required(.Noop), "Noop should not require update") testing.expect(t, !db_update_required({}), "Noop should not require update")
} }
@(test) @(test)
test_db_update_required_backed_up :: proc(t: ^testing.T) { test_db_update_required_backed_up :: proc(t: ^testing.T) {
testing.expect(t, db_update_required(.BackedUp), "BackedUp should require update") testing.expect(t, db_update_required({.BackedUp}), "BackedUp should require update")
} }
@(test) @(test)
test_db_update_required_dir_updated :: proc(t: ^testing.T) { test_db_update_required_dir_updated :: proc(t: ^testing.T) {
testing.expect(t, db_update_required(.DirUpdated), "DirUpdated should require update") testing.expect(t, db_update_required({.DirUpdated}), "DirUpdated should require update")
} }
@(test) @(test)
test_db_update_required_restored :: proc(t: ^testing.T) { test_db_update_required_restored :: proc(t: ^testing.T) {
testing.expect(t, !db_update_required(.Restored), "Restored alone should not require update") testing.expect(t, !db_update_required({.Restored}), "Restored alone should not require update")
} }
@(test) @(test)
test_db_update_required_error :: proc(t: ^testing.T) { test_db_update_required_error :: proc(t: ^testing.T) {
testing.expect(t, !db_update_required(.Error), "Error alone should not require update") testing.expect(t, !db_update_required({.Error}), "Error alone should not require update")
} }
@(test) @(test)
test_db_update_required_combined :: proc(t: ^testing.T) { test_db_update_required_combined :: proc(t: ^testing.T) {
s := i32(SyncResult.DirUpdated) | i32(SyncResult.Restored) combined := SyncFlag{.DirUpdated, .Restored}
combined := SyncResult(s)
testing.expect(t, db_update_required(combined), "DirUpdated|Restored should require update") testing.expect(t, db_update_required(combined), "DirUpdated|Restored should require update")
} }