diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2dc611e..8d2618f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -48,6 +48,23 @@ jobs: working-directory: frontend run: npm test + - name: Vet + # Both builds (see internal/features): the personal tag compiles the + # other half of a few files. + run: | + go vet ./... + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + go vet -tags personal ./... + + - name: Lint + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 + with: + version: v2.15.0 # the version to run locally too (CONTRIBUTING.md) + + - name: Lint (personal build) + # The action above put golangci-lint on PATH. + run: golangci-lint run --build-tags personal ./... + - name: Test run: go test ./... @@ -55,6 +72,14 @@ jobs: # The personal build is tested too (see internal/features). run: go test -tags personal ./... + - name: Test with the race detector + # The sync code is concurrent; -short keeps the randomized crash test + # in internal/accounts small (the full run is above). + run: | + go test -race -short ./... + if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } + go test -race -short -tags personal ./... + - name: Version # Release builds carry their tag (shown in the app, used by the update # check) and put it in the exe's file properties; others are "dev". diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..c02cf9a --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,21 @@ +version: "2" + +linters: + # The standard set: govet, ineffassign, staticcheck, unused (errcheck is in + # it too, see below). CI pins the golangci-lint version in + # .github/workflows/build.yml; run the same one locally. + default: standard + disable: + # Not enabled yet: it finds about 50 unchecked Close/Remove calls that need + # fixing one by one before it can gate CI. + - errcheck + settings: + staticcheck: + checks: + - all + # Error strings are shown to the user as they are, so they are + # sentences: capitalized and with a full stop. + - -ST1005 + # Doc comments on vars and consts here often read "Name, when set, ..." + # or "Name: ..."; the name still comes first, which is what matters. + - -ST1022 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 503127e..86b30ea 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -16,7 +16,9 @@ Thanks for helping. Syncer is a one-person project, so small, focused changes ge ```bash go vet ./... && go vet -tags personal ./... + golangci-lint run ./... && golangci-lint run --build-tags personal ./... go test ./... && go test -tags personal ./... + go test -race -short ./... && go test -race -short -tags personal ./... cd frontend && npm ci && npm run check && npx vitest run cd .. && wails build ``` @@ -26,7 +28,7 @@ Thanks for helping. Syncer is a one-person project, so small, focused changes ge ## Build and layout -Requirements: Go 1.26+, Node 22+, [Wails v2](https://wails.io) (`go install github.com/wailsapp/wails/v2/cmd/wails@v2.16.0`). +Requirements: Go 1.26+, Node 22+, [Wails v2](https://wails.io) (`go install github.com/wailsapp/wails/v2/cmd/wails@v2.16.0`), [golangci-lint](https://golangci-lint.run) at the version pinned in `.github/workflows/build.yml` (`go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.15.0`), and a C compiler such as mingw gcc for `go test -race`. ```bash wails dev # live-reloading app diff --git a/internal/accounts/ops.go b/internal/accounts/ops.go index b1a5cde..aea3639 100644 --- a/internal/accounts/ops.go +++ b/internal/accounts/ops.go @@ -949,7 +949,7 @@ func swap(ctx context.Context, env Env, j *Journal, m *Move) error { } } if m.Step == swapHold { - if !(exists(m.Hold) && !exists(m.Live)) { + if !exists(m.Hold) || exists(m.Live) { if err := os.Rename(m.Live, m.Hold); err != nil { return back(fmt.Errorf("could not move the current saves (is the game running?): %w", err)) } @@ -975,7 +975,7 @@ func swap(ctx context.Context, env Env, j *Journal, m *Move) error { return j.failAt(err) } } - if !(exists(m.Live) && !exists(m.In)) { + if !exists(m.Live) || exists(m.In) { if err := moveDir(m.In, m.Live); err != nil { return back(fmt.Errorf("could not bring in the other saves: %w", err)) } @@ -995,7 +995,7 @@ func swap(ctx context.Context, env Env, j *Journal, m *Move) error { } } if m.Step == swapOut { - if !(exists(m.Out) && !exists(m.Hold)) { + if !exists(m.Out) || exists(m.Hold) { if exists(m.Out) { if _, err := retire(m.Out); err != nil { return j.failAt(err) diff --git a/internal/app/app.go b/internal/app/app.go index 229ae7e..7090472 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -1,3 +1,6 @@ +// Package app is the desktop app: the Wails App whose methods the frontend +// calls, the tray, the background and scheduled runs, the local launcher API +// and the glue that wires the internal packages to the UI. package app import ( @@ -201,13 +204,13 @@ type Overview struct { Target string `json:"target"` LastBackup *store.BackupRun `json:"lastBackup"` // the last attempt, failed or not // LastSuccess is when a backup last finished without errors; zero if never. - LastSuccess time.Time `json:"lastSuccess"` - BackingUp bool `json:"backingUp"` - Gaming bool `json:"gaming"` // automatic backups are holding off right now - Paused bool `json:"paused"` // syncing and automatic backups are paused (until settings.pausedUntil) - Settings store.Settings `json:"settings"` - Version string `json:"version"` - Update *UpdateInfo `json:"update"` // a newer release, if one is out + LastSuccess time.Time `json:"lastSuccess"` + BackingUp bool `json:"backingUp"` + Gaming bool `json:"gaming"` // automatic backups are holding off right now + Paused bool `json:"paused"` // syncing and automatic backups are paused (until settings.pausedUntil) + Settings store.Settings `json:"settings"` + Version string `json:"version"` + Update *UpdateInfo `json:"update"` // a newer release, if one is out // VersionGaps: linked PCs running an older or newer Syncer than this one. VersionGaps []VersionGap `json:"versionGaps"` // Waiting: folders from other PCs that wait for the user's OK. @@ -490,7 +493,7 @@ func normalizeID(id string) (string, error) { return "", errors.New("that doesn't look like a device ID (expected 56 letters/digits)") } for _, ch := range raw { - if !(ch >= 'A' && ch <= 'Z' || ch >= '2' && ch <= '7') { + if (ch < 'A' || ch > 'Z') && (ch < '2' || ch > '7') { return "", errors.New("device IDs only contain letters A–Z and digits 2–7") } } @@ -909,7 +912,7 @@ func applySettingsInput(s *store.Settings, in, old store.Settings) { // modsAutoOn reports whether adding mod folders automatically just started // (or its size limit changed), so it should run now. func modsAutoOn(old, s store.Settings) bool { - return features.Mods && s.FindMods && s.AutoAddMods && (!(old.FindMods && old.AutoAddMods) || s.ModsMaxGB != old.ModsMaxGB) + return features.Mods && s.FindMods && s.AutoAddMods && (!old.FindMods || !old.AutoAddMods || s.ModsMaxGB != old.ModsMaxGB) } // publishSettings shares a change of the mod options with the other PCs now. diff --git a/internal/backup/backup.go b/internal/backup/backup.go index da63bee..0e05b4c 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -275,7 +275,7 @@ func mirror(ctx context.Context, f Folder, opts Options, stamp string, p *Progre } if m.Ignored(rel) || sc.skip(rel, d.IsDir()) { // A negation can reach into an ignored folder. - if d.IsDir() && !(m.HasNegation() && !sc.on) { + if d.IsDir() && (!m.HasNegation() || sc.on) { return filepath.SkipDir } return nil diff --git a/internal/meta/meta.go b/internal/meta/meta.go index 1d3f44d..7f7d236 100644 --- a/internal/meta/meta.go +++ b/internal/meta/meta.go @@ -1062,15 +1062,15 @@ func ModVersionsDir(id string) string { return filepath.Join(paths.Root(paths.Local), "Syncer", "mod-versions", id) } +// HoldNewMod, when set, says whether a new mod folder must start paused +// (its mod manager is open) and remembers to resume it. +var HoldNewMod func(id string) bool + // ModSpec is how a mod folder of kind is added. A staging folder uses // Vortex's own marker as Syncthing's, so no .stfolder appears among the mods // and the folder stops, rather than looking emptied, if its drive is // missing. Replaced files are kept for a week. A PC receiving deployed mods // only takes them from the source, and stays paused between audited updates. -// HoldNewMod, when set, says whether a new mod folder must start paused -// (its mod manager is open) and remembers to resume it. -var HoldNewMod func(id string) bool - func ModSpec(id, label, path, kind string, source bool) FolderSpec { v := syncthing.StaggeredVersioning() v["params"] = map[string]string{"maxAge": "604800", "cleanInterval": "3600"} diff --git a/internal/meta/modsettings_test.go b/internal/meta/modsettings_test.go index aedbc07..7b5448b 100644 --- a/internal/meta/modsettings_test.go +++ b/internal/meta/modsettings_test.go @@ -79,3 +79,19 @@ func TestModSettingsNewestWins(t *testing.T) { t.Fatal("mod options stayed on without finding mods") } } + +// writtenBySelf records PC device's file, as it is now, as last written by +// that PC, as a reconcile finds out from Syncthing. +func writtenBySelf(t *testing.T, device string) { + t.Helper() + sum, ok := fileSum(device) + if !ok { + t.Fatalf("no file of %s", device) + } + writers.Lock() + defer writers.Unlock() + if writers.m == nil { + writers.m = map[string]writerCheck{} + } + writers.m[device] = writerCheck{sum, true} +} diff --git a/internal/meta/owners.go b/internal/meta/owners.go index fbfad54..2e8860b 100644 --- a/internal/meta/owners.go +++ b/internal/meta/owners.go @@ -322,7 +322,7 @@ func noIntroducers(ctx context.Context, c *syncthing.Client, ds []syncthing.Devi for _, d := range ds { id := d.DeviceID _, introduced := st.Introduced[id] - if !d.Introducer || !(pcs[id] || d.IntroducedBy != "" || st.JoinRequests[id] || st.IgnoredPCs[id] || introduced || ours[id]) { + if !d.Introducer || (!pcs[id] && d.IntroducedBy == "" && !st.JoinRequests[id] && !st.IgnoredPCs[id] && !introduced && !ours[id]) { continue } if err := c.PatchDevice(ctx, id, map[string]any{"introducer": false}); err != nil { @@ -465,7 +465,7 @@ func validDeviceID(id string) bool { if ch != '-' { return false } - } else if !(ch >= 'A' && ch <= 'Z' || ch >= '2' && ch <= '7') { + } else if (ch < 'A' || ch > 'Z') && (ch < '2' || ch > '7') { return false } } diff --git a/internal/meta/owners_test.go b/internal/meta/owners_test.go index 7c8df12..8f7d7d7 100644 --- a/internal/meta/owners_test.go +++ b/internal/meta/owners_test.go @@ -177,22 +177,6 @@ func TestWriteVortexListsLeavesAnUnchangedFileAlone(t *testing.T) { } } -// writtenBySelf records PC device's file, as it is now, as last written by -// that PC, as a reconcile finds out from Syncthing. -func writtenBySelf(t *testing.T, device string) { - t.Helper() - sum, ok := fileSum(device) - if !ok { - t.Fatalf("no file of %s", device) - } - writers.Lock() - defer writers.Unlock() - if writers.m == nil { - writers.m = map[string]writerCheck{} - } - writers.m[device] = writerCheck{sum, true} -} - // fakeSyncthing is a Syncthing that, besides Syncer's PCs, syncs a notes // vault with a server and a laptop, as one paired before Syncer was fixed: // every folder offered to every device. diff --git a/internal/steam/steam.go b/internal/steam/steam.go index 014faa7..c517ee5 100644 --- a/internal/steam/steam.go +++ b/internal/steam/steam.go @@ -421,22 +421,3 @@ func isFile(p string) bool { return err == nil && fi.Mode().IsRegular() } -func clean(p string) string { - p = strings.TrimSpace(p) - if p == "" { - return "" - } - return filepath.Clean(filepath.FromSlash(p)) -} - -// within reports whether child is parent or lies below it (case-insensitive). -func within(parent, child string) bool { - parent, child = strings.ToLower(filepath.Clean(parent)), strings.ToLower(filepath.Clean(child)) - if parent == child { - return true - } - if !strings.HasSuffix(parent, `\`) && !strings.HasSuffix(parent, "/") { - parent += string(filepath.Separator) - } - return strings.HasPrefix(child, parent) -} diff --git a/internal/store/store.go b/internal/store/store.go index 41b4562..91447c7 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -648,7 +648,7 @@ var rename = os.Rename func renameRetry(from, to string) error { var err error for i, wait := 0, 10*time.Millisecond; i < 6; i, wait = i+1, wait*2 { - if err = rename(from, to); err == nil || !(errors.Is(err, windows.ERROR_ACCESS_DENIED) || errors.Is(err, windows.ERROR_SHARING_VIOLATION)) { + if err = rename(from, to); err == nil || (!errors.Is(err, windows.ERROR_ACCESS_DENIED) && !errors.Is(err, windows.ERROR_SHARING_VIOLATION)) { return err } time.Sleep(wait)