Add a concurrency lock for download/install against the same destination

combineDirTrees' merge logic assumes it's the only thing moving files
into a given target at a time; two `vintner download`/`install` runs
racing against the same --dest could otherwise interleave os.Rename
calls and corrupt the tree instead of erroring cleanly. Take an
exclusive, non-blocking flock(2) on the destination for the duration
of each run, so a second invocation fails immediately with a clear
message instead of silently colliding with the first.

cmd/vintner/download.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/Cheviiot/vintner/internal/download"
 	"github.com/Cheviiot/vintner/internal/i18n"
+	"github.com/Cheviiot/vintner/internal/lock"
 )
 
 func runDownload(args []string) int {
@@ -177,6 +178,12 @@ func runDownload(args []string) int {
 		fmt.Fprintln(os.Stderr, "vintner download:", err)
 		return 1
 	}
+	unlock, err := lock.Acquire(destAbs)
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "vintner download:", err)
+		return 1
+	}
+	defer unlock()
 
 	unpack := destAbs
 	if !*onlyUnpack {

internal/install/install.go

@@ -15,6 +15,7 @@ import (
 	"strings"
 
 	"github.com/Cheviiot/vintner/assets"
+	"github.com/Cheviiot/vintner/internal/lock"
 	"github.com/Cheviiot/vintner/internal/wineenv"
 )
 
@@ -34,6 +35,12 @@ func Install(dest, selfBinary string) error {
 		return fmt.Errorf("destination %q is not a directory", dest)
 	}
 
+	unlock, err := lock.Acquire(dest)
+	if err != nil {
+		return err
+	}
+	defer unlock()
+
 	// Targets are relative so the whole installed tree stays relocatable -
 	// moving or renaming dest doesn't break these symlinks the way an
 	// absolute target baked in at install time would.

internal/lock/lock.go

@@ -0,0 +1,49 @@
+// Package lock guards a destination directory against two `vintner
+// download`/`install` runs mutating it at the same time.
+package lock
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"syscall"
+)
+
+// FileName is the advisory lock file `download` and `install` both take
+// out against their (usually shared) destination directory before
+// touching anything in it - concurrent download+install, or two
+// downloads, against the same dest could otherwise interleave badly:
+// combineDirTrees' merge logic assumes it's the only thing moving files
+// into a given target at a time, and two `os.Rename` calls racing for the
+// same destination path is exactly the kind of thing that corrupts a tree
+// instead of erroring cleanly.
+const FileName = ".vintner.lock"
+
+// Acquire takes an exclusive, non-blocking lock on dest (creating dest if
+// it doesn't exist yet) and returns a func to release it, which the caller
+// must defer. If another vintner process already holds the lock, returns
+// an error immediately instead of blocking - there's no reason a second
+// invocation should silently queue up and wait for the first to finish
+// touching the same directory; the caller should simply not have started
+// it yet. Uses flock(2), so a crashed holder's lock is released
+// automatically by the kernel when its file descriptor closes - never
+// needs manual cleanup, unlike a plain "does a file exist" lock
+// convention would.
+func Acquire(dest string) (unlock func(), err error) {
+	if err := os.MkdirAll(dest, 0o755); err != nil {
+		return nil, err
+	}
+	path := filepath.Join(dest, FileName)
+	f, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o644)
+	if err != nil {
+		return nil, err
+	}
+	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB); err != nil {
+		f.Close()
+		return nil, fmt.Errorf("another vintner download/install is already running against %s", dest)
+	}
+	return func() {
+		syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
+		f.Close()
+	}, nil
+}

internal/lock/lock_test.go

@@ -0,0 +1,124 @@
+package lock
+
+import (
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"testing"
+	"time"
+)
+
+func TestAcquireAndRelease(t *testing.T) {
+	dest := t.TempDir()
+
+	unlock, err := Acquire(dest)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if _, err := os.Stat(filepath.Join(dest, FileName)); err != nil {
+		t.Errorf("expected the lock file to exist while held: %v", err)
+	}
+	unlock()
+
+	// Released - a second Acquire against the same dest must now succeed.
+	unlock2, err := Acquire(dest)
+	if err != nil {
+		t.Fatalf("Acquire after release failed: %v", err)
+	}
+	unlock2()
+}
+
+func TestAcquireCreatesDestIfMissing(t *testing.T) {
+	dest := filepath.Join(t.TempDir(), "does", "not", "exist", "yet")
+	unlock, err := Acquire(dest)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+	if fi, err := os.Stat(dest); err != nil || !fi.IsDir() {
+		t.Errorf("expected Acquire to create %s, stat err: %v", dest, err)
+	}
+}
+
+func TestAcquireFailsWhileAlreadyHeld(t *testing.T) {
+	dest := t.TempDir()
+
+	unlock, err := Acquire(dest)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+
+	if _, err := Acquire(dest); err == nil {
+		t.Fatal("expected a second Acquire against the same dest, while the first is still held, to fail")
+	}
+}
+
+func TestAcquireSucceedsAfterHolderReleases(t *testing.T) {
+	dest := t.TempDir()
+
+	unlock1, err := Acquire(dest)
+	if err != nil {
+		t.Fatal(err)
+	}
+	unlock1()
+
+	unlock2, err := Acquire(dest)
+	if err != nil {
+		t.Fatalf("Acquire should succeed once the first holder released: %v", err)
+	}
+	unlock2()
+}
+
+// TestAcquireFailsAcrossRealProcesses is the real end-to-end check: flock
+// is per-open-file-description, not per-process or per-thread, so a lock
+// held by *this* test process via one fd could in principle still be
+// re-acquirable by another fd in the same process depending on the
+// platform's exact semantics. Spawning this test binary as a genuinely
+// separate child process (via the standard TestMain re-exec trick) and
+// having it hold the lock while the parent tries to acquire it is what
+// actually proves two independent `vintner download`/`install` processes
+// contend correctly, not just two Go-level calls in one process.
+func TestAcquireFailsAcrossRealProcesses(t *testing.T) {
+	if os.Getenv("VINTNER_LOCK_TEST_HOLD") != "" {
+		unlock, err := Acquire(os.Getenv("VINTNER_LOCK_TEST_HOLD"))
+		if err != nil {
+			os.Exit(2)
+		}
+		defer unlock()
+		// Signal readiness, then wait to be killed by the parent. A plain
+		// `select {}` here would have zero other goroutines able to ever
+		// wake it, which Go's runtime provably detects as a deadlock and
+		// crashes on ("fatal error: all goroutines are asleep") - a real
+		// timer avoids that.
+		os.Stdout.WriteString("locked\n")
+		time.Sleep(time.Minute)
+	}
+
+	dest := t.TempDir()
+	exe, err := os.Executable()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	cmd := exec.Command(exe, "-test.run=TestAcquireFailsAcrossRealProcesses")
+	cmd.Env = append(os.Environ(), "VINTNER_LOCK_TEST_HOLD="+dest)
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	defer cmd.Process.Kill()
+
+	buf := make([]byte, len("locked\n"))
+	if _, err := io.ReadFull(stdout, buf); err != nil || string(buf) != "locked\n" {
+		t.Fatalf("child process didn't report holding the lock: %v (%q)", err, buf)
+	}
+
+	if _, err := Acquire(dest); err == nil {
+		t.Fatal("expected Acquire to fail while a separate process holds the lock")
+	}
+}