Conversation
…call.Flock` and updates documentation for Windows users. ### Changes - **File Locking Decoupling:** - Renamed `internal/lock/lock.go` to `internal/lock/lock_unix.go` (added `//go:build !windows`). - Created `internal/lock/lock_windows.go` with a Windows-compatible implementation that relies on file existence and `os.OpenFile` exclusion flags effectively, skipping `syscall.Flock`. - **Documentation:** - Updated `README.md` with a new "Install via Go" option (`go install`). - highlighted Windows support in the installation section. ### CI/CD Existing Windows release workflows in `.goreleaser-linux-windows.yaml` should now succeed since the platform-specific code compilation issues are resolved.
There was a problem hiding this comment.
Pull request overview
This PR aims to fix Windows build failures by splitting the store-lock implementation into platform-specific files and updating installation documentation to better support Windows users.
Changes:
- Added Windows-specific lock implementation under
internal/lock/lock_windows.go. - Added
//go:build !windowsconstraint to the Unix implementation ininternal/lock/lock_unix.go. - Updated
README.mdwith ago installinstallation option and clarified availability of Windows binaries.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/lock/lock_windows.go | Introduces a Windows-targeted lock implementation intended to replace Unix-only syscall.Flock. |
| internal/lock/lock_unix.go | Adds a build constraint so Unix locking code is excluded from Windows builds. |
| README.md | Adds a Go-based install option and highlights pre-built binaries for Windows/macOS/Linux. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path := filepath.Join(storeDir, "LOCK") | ||
| f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0600) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("open lock file: %w", err) | ||
| } |
There was a problem hiding this comment.
Acquire on Windows currently does not provide mutual exclusion: os.OpenFile(path, os.O_CREATE|os.O_RDWR, ...) will succeed even when another process already holds the lock file open, so a second process can also “acquire” the lock. This breaks the intended cross-process store lock semantics (and will fail the existing internal/lock/lock_test.go behavior). Implement an actual non-blocking exclusive lock on Windows (e.g., golang.org/x/sys/windows.LockFileEx with LOCKFILE_EXCLUSIVE_LOCK|LOCKFILE_FAIL_IMMEDIATELY, or CreateFile with share mode 0) and return the same "store is locked" error when the lock cannot be taken; also ensure the file is closed on the lock-failure path.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
| import ( | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "time" | ||
| ) |
There was a problem hiding this comment.
This file is not gofmt-formatted (spaces instead of tabs in the import block and indentation). Please run gofmt on internal/lock/lock_windows.go to match standard Go formatting used elsewhere in the repo.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
1 participant
This PR resolves build errors on Windows caused by Unix-specific
syscall.Flockand updates documentation for Windows users.Changes
File Locking Decoupling:
internal/lock/lock.gotointernal/lock/lock_unix.go(added//go:build !windows).internal/lock/lock_windows.gowith a Windows-compatible implementation that relies on file existence andos.OpenFileexclusion flags effectively, skippingsyscall.Flock.Documentation:
README.mdwith a new "Install via Go" option (go install).CI/CD
Existing Windows release workflows in
.goreleaser-linux-windows.yamlshould now succeed since the platform-specific code compilation issues are resolved.