diff --git a/README.md b/README.md index fe75adc..cc3719b 100644 --- a/README.md +++ b/README.md @@ -12,8 +12,11 @@ requested attributes along the way. See [End-to-End Sync Pipeline](#end-to-end-sync-pipeline) below for exactly how the pieces connect and, just as importantly, what's still explicitly out of scope (compression, progress reporting, `--dry-run`, partial/ -append transfers, batch mode, full `--delete`, hard links, and device/ -special files) - this is real, working sync, not yet full feature parity. +append transfers, batch mode, full `--delete`, and device/special files) +- this is real, working sync, not yet full feature parity. Hard links +*are* now preserved, opt-in via `-H`/`--hard-links` exactly like real +rsync's own flag (see +[File Attribute Preservation](#file-attribute-preservation) below). `grsync --daemon` also now speaks a real subset of the rsync daemon protocol - `rsyncd.conf` parsing, the `@RSYNCD` greeting/handshake, module @@ -69,6 +72,7 @@ argument is always the destination. | `--owner` | `-o` | preserve owner (implied by `--archive`; requires appropriate privileges) | | `--group` | `-g` | preserve group (implied by `--archive`; requires appropriate privileges) | | `--links` | `-l` | recreate symlinks as symlinks (implied by `--archive`) | +| `--hard-links` | `-H` | preserve hard links between files in the source (**NOT** implied by `--archive`, matching real rsync's own `-a`) | | `--daemon` | | run as an rsync-protocol daemon, serving modules from `--config` (see [rsync Daemon Mode](#rsync-daemon-mode)) | | `--config PATH` | | path to the `rsyncd.conf` to serve (required with `--daemon`) | | `--port PORT` | | TCP port to listen on in `--daemon` mode (default `873`, matching rsync) | @@ -175,6 +179,44 @@ capture. Call `sync.HardLinksSupported()` to tell "this platform can't detect hard links" apart from "this tree just has none," rather than guessing from an empty result. + + **Wired into the actual sync pipeline, opt-in via `-H`/`--hard-links`** + - exactly matching real rsync's own flag, including that `--archive` + does *not* imply it (real rsync's `-a` is `-rlptgoD`, no `H`; see + `effectiveAttrOptions` in `internal/cli/sync.go`). Only when requested, + `pipeline.Sender` runs `sync.DetectHardLinks` over the filtered file + list (skipping the call entirely, not just discarding its result, when + `sync.HardLinksSupported()` is false - no point paying for a per-entry + `Lstat` pass that would always come back empty either way) and attaches + the grouping to the same `FrameFileList` message the entries themselves + travel in, rather than a separate round trip. `pipeline.Receiver` + writes each group's first member through the normal signature/delta + path and recreates every other member with `sync.ApplyHardLinks` + (`os.Link`) instead of re-transferring bytes that are identical by + definition - in a pass that runs after every regular file is written + but *before* the deferred directory-attributes pass, since `os.Link` + touches its parent directory's mtime the same way creating any other + file does. Detection only has to succeed on the *sending* side: + `os.Link` itself works on Windows too, so a Linux/macOS sender pushing + to a Windows destination still produces real hard links there, even + though a Windows source's own links can't be detected in the first + place. + + Tested end to end with `TestSenderReceiver_HardLinks` in + `internal/pipeline/pipeline_test.go` (proving the destination files are + genuinely the same file, not independent copies with matching content + - and that this degrades to correct independent copies, not an error, + where `HardLinksSupported()` is false), + `TestSenderReceiver_HardLinksNotPreservedWithoutOptIn` (the direct + proof that omitting `-H` really does leave files unlinked, not just + documented as opt-in while actually running unconditionally), + `TestReceiver_AppliesHardLinksFromReceivedGroups` (proving `Receiver` + never asks for a signature for a group's secondary member, regardless + of what the current platform can detect), and - at the real CLI command + level, in `internal/cli/sync_test.go` - + `TestE2E_HardLinksPreservedWithFlag`/`TestE2E_ArchiveAloneDoesNotImplyHardLinks`, + the latter being the specific regression test for `--archive` never + silently turning this on. - **Device/special files** (`sync.ApplySpecialFile`): deliberately scoped down. Named pipes (FIFOs) are fully created via `Mkfifo`, since that needs no elevated privilege. Sockets and character/block devices are @@ -301,13 +343,11 @@ ticket): compression, progress reporting, real `--dry-run` (still the flag-echoing placeholder, to avoid silently performing a real sync when a dry run was requested), partial/append transfers, batch mode, pulling from a remote source (only local-source syncs are supported - push, not -pull), and - carried over from SC-8 - hard links and device/special -files. `sync.DetectHardLinks`/`sync.ApplyHardLinks`/`sync.ApplySpecialFile` -exist and are tested, but nothing in `pipeline.Receiver` calls them yet; -wiring them in is a reasonable, low-risk follow-up (hard links -particularly, since unlike device files it needs no elevated privilege) -but was left out of this already-large integration ticket rather than -expanding its scope further. +pull), and device/special files. `sync.ApplySpecialFile` exists and is +tested, but nothing in `pipeline.Receiver` calls it yet - unlike hard +links (see [File Attribute Preservation](#file-attribute-preservation) +above, now wired in), this needs elevated privilege to test meaningfully +and was left out rather than expanding this integration further. ## rsync Daemon Mode diff --git a/internal/cli/root.go b/internal/cli/root.go index 5f6010b..6aa1bd8 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -57,6 +57,7 @@ type options struct { owner bool group bool links bool + hardLinks bool filterRules []FilterRule rsh string server bool @@ -169,6 +170,8 @@ func NewRootCmd() *cobra.Command { flags.BoolVarP(&opts.owner, "owner", "o", false, "preserve owner (implied by --archive; requires appropriate privileges)") flags.BoolVarP(&opts.group, "group", "g", false, "preserve group (implied by --archive; requires appropriate privileges)") flags.BoolVarP(&opts.links, "links", "l", false, "recreate symlinks as symlinks (implied by --archive)") + flags.BoolVarP(&opts.hardLinks, "hard-links", "H", false, + "preserve hard links between files in the source (NOT implied by --archive, matching real rsync's own -a)") flags.StringVarP(&opts.rsh, "rsh", "e", "", "specify the remote shell to use, e.g. \"ssh -p 2222 -i key.pem\" (default: ssh); "+ "the sole way to customize port/identity/proxy for remote transport, matching rsync") diff --git a/internal/cli/rsync_url.go b/internal/cli/rsync_url.go index a7cc531..64fe872 100644 --- a/internal/cli/rsync_url.go +++ b/internal/cli/rsync_url.go @@ -31,8 +31,11 @@ const dialDaemonTimeout = 10 * time.Second // daemon.DialClient, which runs the real handshake/authentication and // then the same pipeline.Sender every other destination uses - this // function's only job is to get from a URL to a net.Conn and supply the -// credentials, not to know anything about the transfer itself. -func syncToRsyncDaemon(src string, u daemon.URL, password daemon.PasswordFunc, walkOpts sync.WalkOptions, rules []sync.Rule) error { +// credentials, not to know anything about the transfer itself. hardLinks +// is the only AttrOptions field DialClient's Sender-side (DirectionPut) +// call actually consults, but it's threaded through as a full +// sync.AttrOptions to match DialClient's own signature. +func syncToRsyncDaemon(src string, u daemon.URL, password daemon.PasswordFunc, walkOpts sync.WalkOptions, rules []sync.Rule, hardLinks bool) error { port := u.Port if port == 0 { port = daemon.DefaultPort @@ -46,5 +49,5 @@ func syncToRsyncDaemon(src string, u daemon.URL, password daemon.PasswordFunc, w defer func() { _ = nc.Close() }() user := resolveUser(u.User) - return daemon.DialClient(nc, u.Module, user, password, daemon.DirectionPut, src, rules, walkOpts, sync.AttrOptions{}) + return daemon.DialClient(nc, u.Module, user, password, daemon.DirectionPut, src, rules, walkOpts, sync.AttrOptions{HardLinks: hardLinks}) } diff --git a/internal/cli/sync.go b/internal/cli/sync.go index 8b1b433..5dfc4a5 100644 --- a/internal/cli/sync.go +++ b/internal/cli/sync.go @@ -33,15 +33,19 @@ func effectiveWalkOptions(opts *options) sync.WalkOptions { // effectiveAttrOptions computes sync.AttrOptions from opts: --archive // implies perms/times/owner/group/links, matching real rsync's -a // (-rlptgoD, minus the r which effectiveWalkOptions handles, and minus -// devices/specials - see the README's note on why hard links and device -// files are deferred rather than wired up here). +// devices/specials - see the README's note on why device files are +// deferred rather than wired up here). HardLinks is deliberately NOT +// included in that implication: real rsync's own -a does not imply -H +// either (-a is exactly -rlptgoD, no H), so --archive alone must not +// turn hard-link detection on. func effectiveAttrOptions(opts *options) sync.AttrOptions { return sync.AttrOptions{ - Perms: opts.archive || opts.perms, - Times: opts.archive || opts.times, - Owner: opts.archive || opts.owner, - Group: opts.archive || opts.group, - Links: opts.archive || opts.links, + Perms: opts.archive || opts.perms, + Times: opts.archive || opts.times, + Owner: opts.archive || opts.owner, + Group: opts.archive || opts.group, + Links: opts.archive || opts.links, + HardLinks: opts.hardLinks, } } @@ -122,11 +126,11 @@ func runSync(cmd *cobra.Command, sources []string, destination string, opts *opt for _, src := range sources { switch { case isRsyncDaemon: - if err := syncToRsyncDaemon(src, rsyncURL, password, walkOpts, rules); err != nil { + if err := syncToRsyncDaemon(src, rsyncURL, password, walkOpts, rules, attrOpts.HardLinks); err != nil { return fmt.Errorf("syncing %q to %q: %w", src, destination, err) } case isRemote: - if err := syncToRemote(opts.rsh, src, remote, walkOpts, rules); err != nil { + if err := syncToRemote(opts.rsh, src, remote, walkOpts, rules, attrOpts.HardLinks); err != nil { return fmt.Errorf("syncing %q to %q: %w", src, destination, err) } default: @@ -153,7 +157,7 @@ func syncLocal(src, dest string, walkOpts sync.WalkOptions, rules []sync.Rule, a receiver := pipeReadWriter{Reader: receiverReadsFromSender, Writer: receiverWritesToSender} senderErrCh := make(chan error, 1) - go func() { senderErrCh <- pipeline.Sender(sender, src, walkOpts, rules) }() + go func() { senderErrCh <- pipeline.Sender(sender, src, walkOpts, rules, attrOpts.HardLinks) }() receiverErr := pipeline.Receiver(receiver, dest, attrOpts) senderErr := <-senderErrCh @@ -167,7 +171,7 @@ func syncLocal(src, dest string, walkOpts sync.WalkOptions, rules []sync.Rule, a // syncToRemote spawns `grsync --server DEST` on the remote host via SSH // (or whatever --rsh overrides it to), performs the handshake, then runs // the sender side of the pipeline against that connection. -func syncToRemote(rsh, src string, remote transport.RemotePath, walkOpts sync.WalkOptions, rules []sync.Rule) error { +func syncToRemote(rsh, src string, remote transport.RemotePath, walkOpts sync.WalkOptions, rules []sync.Rule, hardLinks bool) error { session, err := transport.Dial(rsh, remote.User, remote.Host, []string{"grsync", "--server", remote.Path}) if err != nil { return fmt.Errorf("connecting to %s: %w", remote.Host, err) @@ -178,7 +182,7 @@ func syncToRemote(rsh, src string, remote transport.RemotePath, walkOpts sync.Wa return fmt.Errorf("handshake with %s failed: %w", remote.Host, err) } - sendErr := pipeline.Sender(session, src, walkOpts, rules) + sendErr := pipeline.Sender(session, src, walkOpts, rules, hardLinks) closeErr := session.Close() if sendErr != nil { diff --git a/internal/cli/sync_test.go b/internal/cli/sync_test.go index a999c92..29921f6 100644 --- a/internal/cli/sync_test.go +++ b/internal/cli/sync_test.go @@ -40,6 +40,76 @@ func TestE2E_LocalToLocal(t *testing.T) { assertTreesMatch(t, src, dst, symlinksSupported) } +// TestE2E_HardLinksPreservedWithFlag drives the real CLI command with +// -H/--hard-links and confirms two hard-linked source files arrive at +// the destination still hard-linked to each other (os.SameFile), not +// independent copies that merely have matching content. +func TestE2E_HardLinksPreservedWithFlag(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + + mustWriteFile(t, filepath.Join(src, "original.txt"), "shared content") + if err := os.Link(filepath.Join(src, "original.txt"), filepath.Join(src, "linked.txt")); err != nil { + t.Skipf("hard link creation unsupported in this environment: %v", err) + } + if runtime.GOOS == "windows" { + t.Skip("sync.HardLinksSupported() is false on Windows - grsync's own detection can't observe the link created above, so this platform always produces independent copies regardless of -H; see TestSenderReceiver_HardLinks for the graceful-degradation proof") + } + + cmd := NewRootCmd() + cmd.SetArgs([]string{"-a", "-H", src, dst}) + cmd.SetOut(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute returned error: %v", err) + } + + originalInfo, err := os.Stat(filepath.Join(dst, "original.txt")) + if err != nil { + t.Fatalf("Stat: %v", err) + } + linkedInfo, err := os.Stat(filepath.Join(dst, "linked.txt")) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if !os.SameFile(originalInfo, linkedInfo) { + t.Errorf("destination files are independent, want them hard-linked (-H was given)") + } +} + +// TestE2E_ArchiveAloneDoesNotImplyHardLinks is the critical correctness +// check behind -H being opt-in: --archive alone (no -H) must NOT +// preserve hard links, exactly matching real rsync's own -a (-rlptgoD, +// no H). Without this, "-H defaults to off" would be true in name only +// if --archive silently turned it on anyway. +func TestE2E_ArchiveAloneDoesNotImplyHardLinks(t *testing.T) { + src := t.TempDir() + dst := t.TempDir() + + mustWriteFile(t, filepath.Join(src, "original.txt"), "shared content") + if err := os.Link(filepath.Join(src, "original.txt"), filepath.Join(src, "linked.txt")); err != nil { + t.Skipf("hard link creation unsupported in this environment: %v", err) + } + + cmd := NewRootCmd() + cmd.SetArgs([]string{"-a", src, dst}) // -a, deliberately no -H + cmd.SetOut(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute returned error: %v", err) + } + + originalInfo, err := os.Stat(filepath.Join(dst, "original.txt")) + if err != nil { + t.Fatalf("Stat: %v", err) + } + linkedInfo, err := os.Stat(filepath.Join(dst, "linked.txt")) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if os.SameFile(originalInfo, linkedInfo) { + t.Errorf("destination files are hard-linked despite -H not being given - --archive must not imply -H, matching real rsync's own -a (-rlptgoD, no H)") + } +} + // assertTreesMatch walks both roots and compares every entry: path, // directory-ness, permission bits (platform-aware, see wantPermCLI), // modification time, and - for regular files - content, and for diff --git a/internal/daemon/session.go b/internal/daemon/session.go index 752353a..5640366 100644 --- a/internal/daemon/session.go +++ b/internal/daemon/session.go @@ -70,13 +70,14 @@ func moduleRules(m Module) ([]sync.Rule, error) { return sync.CompileRules(raw) } -// moduleAttrOptions is what a daemon module preserves on an upload. -// Fixed, rather than client-controlled, since the module (not the -// client) owns what its own rsyncd.conf-configured directory considers -// worth preserving - a client can't currently ask a module to preserve -// more or less than this. +// moduleAttrOptions is what a daemon module preserves - on an upload +// (applied by the receiving Receiver) and, via its HardLinks field, on a +// download too (consulted by the sending Sender). Fixed, rather than +// client-controlled, since the module (not the client) owns what its own +// rsyncd.conf-configured directory considers worth preserving - a client +// can't currently ask a module to preserve more or less than this. func moduleAttrOptions() sync.AttrOptions { - return sync.AttrOptions{Perms: true, Times: true, Owner: true, Group: true, Links: true} + return sync.AttrOptions{Perms: true, Times: true, Owner: true, Group: true, Links: true, HardLinks: true} } // ServeModule runs one authenticated client's session against the @@ -115,7 +116,7 @@ func ServeModule(c *conn, m Module) error { if err != nil { return fmt.Errorf("compiling module %q exclude rules: %w", m.Name, err) } - if err := pipeline.Sender(c, m.Path, sync.WalkOptions{Recursive: true}, rules); err != nil { + if err := pipeline.Sender(c, m.Path, sync.WalkOptions{Recursive: true}, rules, moduleAttrOptions().HardLinks); err != nil { return err } return waitForTransferDone(c) @@ -133,9 +134,12 @@ func ServeModule(c *conn, m Module) error { // requested direction, waits for the server's ack (see ServeModule) and // fails without touching the pipeline at all if it's an @ERROR instead, // then runs the matching pipeline side against localPath. rules and -// walkOpts only matter for DirectionPut (they govern what the client's -// own Sender walk includes); attrOpts only matters for DirectionGet (what -// the client's own Receiver preserves). +// walkOpts govern what the client's own Sender walk includes on a +// DirectionPut; attrOpts governs what the client's own Receiver preserves +// on a DirectionGet, and its HardLinks field also governs whether a +// DirectionPut's Sender detects hard links at all - the same field +// serves both directions since it's one "does the client want hard links +// preserved" decision either way. func DialModule(c *conn, direction Direction, localPath string, rules []sync.Rule, walkOpts sync.WalkOptions, attrOpts sync.AttrOptions) error { if err := writeLine(c.w, string(direction)); err != nil { return fmt.Errorf("sending direction: %w", err) @@ -156,7 +160,7 @@ func DialModule(c *conn, direction Direction, localPath string, rules []sync.Rul } return writeLine(c.w, transferDone) case DirectionPut: - if err := pipeline.Sender(c, localPath, walkOpts, rules); err != nil { + if err := pipeline.Sender(c, localPath, walkOpts, rules, attrOpts.HardLinks); err != nil { return err } return waitForTransferDone(c) diff --git a/internal/pipeline/messages.go b/internal/pipeline/messages.go index 4f683e7..5d72d3d 100644 --- a/internal/pipeline/messages.go +++ b/internal/pipeline/messages.go @@ -126,24 +126,36 @@ func readTypedFrame(rw io.Reader, want transport.FrameType, what string) (transp return f, nil } -func sendFileList(w io.Writer, entries []sync.FileEntry) error { - payload, err := encodeGob(entries) +// fileListMessage is FrameFileList's payload: the filtered file list plus +// which entries are hard-linked to each other on the source, so that +// grouping travels with the list itself rather than needing a separate +// round trip - HardLinkGroups is empty exactly when there's nothing to +// say about hard links, either because the source tree has none or +// because the sending platform can't detect them (sync.HardLinksSupported() +// is false). +type fileListMessage struct { + Entries []sync.FileEntry + HardLinkGroups []sync.HardLinkGroup +} + +func sendFileList(w io.Writer, entries []sync.FileEntry, groups []sync.HardLinkGroup) error { + payload, err := encodeGob(fileListMessage{Entries: entries, HardLinkGroups: groups}) if err != nil { return fmt.Errorf("encoding file list: %w", err) } return transport.WriteFrame(w, transport.Frame{Type: transport.FrameFileList, Payload: payload}) } -func recvFileList(r io.Reader) ([]sync.FileEntry, error) { +func recvFileList(r io.Reader) ([]sync.FileEntry, []sync.HardLinkGroup, error) { f, err := readTypedFrame(r, transport.FrameFileList, "file list") if err != nil { - return nil, err + return nil, nil, err } - var entries []sync.FileEntry - if err := decodeGob(f.Payload, &entries); err != nil { - return nil, fmt.Errorf("decoding file list: %w", err) + var msg fileListMessage + if err := decodeGob(f.Payload, &msg); err != nil { + return nil, nil, fmt.Errorf("decoding file list: %w", err) } - return entries, nil + return msg.Entries, msg.HardLinkGroups, nil } func sendSignature(w io.Writer, path string, sig sync.Signature) error { diff --git a/internal/pipeline/messages_test.go b/internal/pipeline/messages_test.go index b218d3e..3fae962 100644 --- a/internal/pipeline/messages_test.go +++ b/internal/pipeline/messages_test.go @@ -35,11 +35,13 @@ func TestFileListRoundTrip(t *testing.T) { }, } + wantGroups := []sync.HardLinkGroup{{"dir/a.txt", "dir/b.txt"}} + var buf bytes.Buffer - if err := sendFileList(&buf, want); err != nil { + if err := sendFileList(&buf, want, wantGroups); err != nil { t.Fatalf("sendFileList returned error: %v", err) } - got, err := recvFileList(&buf) + got, gotGroups, err := recvFileList(&buf) if err != nil { t.Fatalf("recvFileList returned error: %v", err) } @@ -59,6 +61,20 @@ func TestFileListRoundTrip(t *testing.T) { t.Errorf("entry %d = %+v, want %+v", i, got[i], want[i]) } } + + if len(gotGroups) != len(wantGroups) { + t.Fatalf("got %d hard-link groups, want %d", len(gotGroups), len(wantGroups)) + } + for i := range wantGroups { + if len(gotGroups[i]) != len(wantGroups[i]) { + t.Fatalf("group %d = %v, want %v", i, gotGroups[i], wantGroups[i]) + } + for j := range wantGroups[i] { + if gotGroups[i][j] != wantGroups[i][j] { + t.Errorf("group %d member %d = %q, want %q", i, j, gotGroups[i][j], wantGroups[i][j]) + } + } + } } func TestSignatureRoundTrip(t *testing.T) { diff --git a/internal/pipeline/pipeline_test.go b/internal/pipeline/pipeline_test.go index e869b7e..39d7203 100644 --- a/internal/pipeline/pipeline_test.go +++ b/internal/pipeline/pipeline_test.go @@ -1,6 +1,7 @@ package pipeline import ( + "fmt" "io" "os" "path/filepath" @@ -25,7 +26,7 @@ func runSenderReceiver(t *testing.T, src, dest string, walkOpts sync.WalkOptions receiver := pipeReadWriter{Reader: receiverReadsFromSender, Writer: receiverWritesToSender} senderErrCh := make(chan error, 1) - go func() { senderErrCh <- Sender(sender, src, walkOpts, rules) }() + go func() { senderErrCh <- Sender(sender, src, walkOpts, rules, attrOpts.HardLinks) }() receiverErrCh := make(chan error, 1) go func() { receiverErrCh <- Receiver(receiver, dest, attrOpts) }() @@ -186,6 +187,254 @@ func TestSenderReceiver_DirectoryAttributesSurviveChildCreation(t *testing.T) { } } +// TestSenderReceiver_HardLinks is SC-18's end-to-end proof, with +// AttrOptions.HardLinks explicitly requested (the -H/--hard-links +// opt-in - see TestSenderReceiver_HardLinksNotPreservedWithoutOptIn for +// the default-off case): two hard-linked source files must arrive at the +// destination still hard-linked to each other (the same underlying file, +// not two independent copies that merely happen to have identical +// content), and an unrelated third file must NOT be linked to either of +// them. Where this platform can't detect hard links at all (Windows - +// sync.HardLinksSupported()), the same sync must still succeed and +// produce byte-correct content - as independent copies, not an error - +// exactly the graceful-degradation behavior sync.DetectHardLinks itself +// already documents. +// +// os.SameFile is used for the linked/unlinked check rather than +// platform-specific stat code in the test itself: it reports genuine +// same-file identity on every platform Go supports, including Windows +// (via the Win32 file index), even though grsync's own hard-link +// *detection* can't see that identity there - so this test can assert +// the real, portable ground truth regardless of what grsync itself was +// able to observe. +func TestSenderReceiver_HardLinks(t *testing.T) { + srcRoot := t.TempDir() + destRoot := t.TempDir() + + mustWriteFile(t, filepath.Join(srcRoot, "original.txt"), "shared content") + mustWriteFile(t, filepath.Join(srcRoot, "unrelated.txt"), "different content, must stay independent") + if err := os.Link(filepath.Join(srcRoot, "original.txt"), filepath.Join(srcRoot, "linked.txt")); err != nil { + t.Skipf("hard link creation unsupported in this environment: %v", err) + } + + runSenderReceiver(t, srcRoot, destRoot, + sync.WalkOptions{Recursive: true}, nil, sync.AttrOptions{Perms: true, Times: true, HardLinks: true}) + + assertSameContent(t, filepath.Join(srcRoot, "original.txt"), filepath.Join(destRoot, "original.txt")) + assertSameContent(t, filepath.Join(srcRoot, "linked.txt"), filepath.Join(destRoot, "linked.txt")) + assertSameContent(t, filepath.Join(srcRoot, "unrelated.txt"), filepath.Join(destRoot, "unrelated.txt")) + + destOriginal := filepath.Join(destRoot, "original.txt") + destLinked := filepath.Join(destRoot, "linked.txt") + destUnrelated := filepath.Join(destRoot, "unrelated.txt") + + originalInfo, err := os.Stat(destOriginal) + if err != nil { + t.Fatalf("Stat %q: %v", destOriginal, err) + } + linkedInfo, err := os.Stat(destLinked) + if err != nil { + t.Fatalf("Stat %q: %v", destLinked, err) + } + unrelatedInfo, err := os.Stat(destUnrelated) + if err != nil { + t.Fatalf("Stat %q: %v", destUnrelated, err) + } + + if os.SameFile(originalInfo, unrelatedInfo) { + t.Fatalf("%q and %q are the same file, want independent - unrelated.txt must never be linked to the hard-link group", destOriginal, destUnrelated) + } + + if !sync.HardLinksSupported() { + if os.SameFile(originalInfo, linkedInfo) { + t.Fatalf("%q and %q are the same file on a platform where HardLinksSupported() is false - "+ + "expected independent copies (graceful degradation), not a link this platform can't have detected", destOriginal, destLinked) + } + return + } + + if !os.SameFile(originalInfo, linkedInfo) { + t.Fatalf("%q and %q are independent files at the destination, want them hard-linked (same underlying file) like they are at the source", destOriginal, destLinked) + } + + // Prove it directly, not just via os.SameFile: a write through one + // path must be visible through the other, the same proof + // internal/sync's own TestApplyHardLinks uses. + if err := os.WriteFile(destLinked, []byte("changed via the linked path"), 0o644); err != nil { + t.Fatalf("WriteFile(%q): %v", destLinked, err) + } + got, err := os.ReadFile(destOriginal) + if err != nil { + t.Fatalf("ReadFile(%q): %v", destOriginal, err) + } + if string(got) != "changed via the linked path" { + t.Errorf("%q content = %q after writing through %q, want the change to be visible (they should be the same file)", destOriginal, got, destLinked) + } +} + +// TestSenderReceiver_HardLinksNotPreservedWithoutOptIn is +// TestSenderReceiver_HardLinks's counterpart for the default case: +// without AttrOptions.HardLinks set, hard-linked source files must sync +// as independent copies - correct content, but not linked - even on a +// platform that could have detected and preserved the relationship. This +// is the direct proof that hard-link detection is genuinely opt-in +// (matching real rsync's own -H/--hard-links, not implied by any other +// flag), not just documented as opt-in while actually running +// unconditionally. +func TestSenderReceiver_HardLinksNotPreservedWithoutOptIn(t *testing.T) { + srcRoot := t.TempDir() + destRoot := t.TempDir() + + mustWriteFile(t, filepath.Join(srcRoot, "original.txt"), "shared content") + if err := os.Link(filepath.Join(srcRoot, "original.txt"), filepath.Join(srcRoot, "linked.txt")); err != nil { + t.Skipf("hard link creation unsupported in this environment: %v", err) + } + + runSenderReceiver(t, srcRoot, destRoot, + sync.WalkOptions{Recursive: true}, nil, sync.AttrOptions{Perms: true, Times: true}) // HardLinks left false + + assertSameContent(t, filepath.Join(srcRoot, "original.txt"), filepath.Join(destRoot, "original.txt")) + assertSameContent(t, filepath.Join(srcRoot, "linked.txt"), filepath.Join(destRoot, "linked.txt")) + + originalInfo, err := os.Stat(filepath.Join(destRoot, "original.txt")) + if err != nil { + t.Fatalf("Stat: %v", err) + } + linkedInfo, err := os.Stat(filepath.Join(destRoot, "linked.txt")) + if err != nil { + t.Fatalf("Stat: %v", err) + } + if os.SameFile(originalInfo, linkedInfo) { + t.Errorf("destination files are hard-linked despite AttrOptions.HardLinks being false - " + + "hard-link detection must be opt-in, not run unconditionally") + } +} + +// TestReceiver_AppliesHardLinksFromReceivedGroups exercises Receiver's +// hard-link handling directly against a hand-built file list, the same +// way TestReceiver_ConnectionDropsMidTransfer drives Receiver against a +// raw peer goroutine rather than a real Sender. This matters on a +// platform where sync.HardLinksSupported() is false (Windows): Receiver's +// own linking logic - skip a group's secondary members in the main loop, +// link them once their primary is written - has nothing to do with +// whether *this* platform's Sender could have detected the grouping, so +// it can and should be verified directly, standing in for whatever +// Sender a real Linux/macOS peer would be running. +// +// The peer goroutine responds to a signature request for "original.txt" +// only; if Receiver incorrectly asked for a signature for "linked.txt" +// too (i.e. failed to skip it as a secondary member), nothing would ever +// answer that second request and the test would time out rather than +// silently pass. +func TestReceiver_AppliesHardLinksFromReceivedGroups(t *testing.T) { + destRoot := t.TempDir() + + peerReadsFromReceiver, receiverWritesToPeer := io.Pipe() + receiverReadsFromPeer, peerWritesToReceiver := io.Pipe() + receiver := pipeReadWriter{Reader: receiverReadsFromPeer, Writer: receiverWritesToPeer} + + // Named so the alphabetical sort order sync.DetectHardLinks itself + // would produce is obvious at a glance: "aaa-primary.txt" sorts + // first, so it's group[0] - the member Receiver writes normally - + // and "zzz-secondary.txt" is the one that must be skipped and linked + // instead. + const content = "shared content, sent once for the whole group" + entries := []sync.FileEntry{ + {Path: "aaa-primary.txt", Mode: 0o644, Size: int64(len(content))}, + {Path: "unrelated.txt", Mode: 0o644, Size: 5}, + {Path: "zzz-secondary.txt", Mode: 0o644, Size: int64(len(content))}, + } + groups := []sync.HardLinkGroup{{"aaa-primary.txt", "zzz-secondary.txt"}} + + peerErrCh := make(chan error, 1) + go func() { + if err := sendFileList(peerWritesToReceiver, entries, groups); err != nil { + peerErrCh <- fmt.Errorf("sending file list: %w", err) + return + } + + sigMsg, err := recvSignature(peerReadsFromReceiver) + if err != nil { + peerErrCh <- fmt.Errorf("receiving signature: %w", err) + return + } + if sigMsg.Path != "aaa-primary.txt" { + peerErrCh <- fmt.Errorf("signature requested for %q, want only \"aaa-primary.txt\" - "+ + "\"zzz-secondary.txt\" (a hard-link group's secondary member) must never reach the signature/delta exchange", sigMsg.Path) + return + } + ops := sync.GenerateDelta(sigMsg.Sig, []byte(content)) + if err := sendDelta(peerWritesToReceiver, "aaa-primary.txt", ops); err != nil { + peerErrCh <- fmt.Errorf("sending delta: %w", err) + return + } + + sigMsg, err = recvSignature(peerReadsFromReceiver) + if err != nil { + peerErrCh <- fmt.Errorf("receiving signature: %w", err) + return + } + if sigMsg.Path != "unrelated.txt" { + peerErrCh <- fmt.Errorf("signature requested for %q, want \"unrelated.txt\"", sigMsg.Path) + return + } + ops = sync.GenerateDelta(sigMsg.Sig, []byte("xxxxx")) + if err := sendDelta(peerWritesToReceiver, "unrelated.txt", ops); err != nil { + peerErrCh <- fmt.Errorf("sending delta: %w", err) + return + } + + peerErrCh <- nil + }() + + receiverErrCh := make(chan error, 1) + go func() { receiverErrCh <- Receiver(receiver, destRoot, sync.AttrOptions{}) }() + + select { + case err := <-receiverErrCh: + if err != nil { + t.Fatalf("Receiver returned error: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("Receiver did not complete within 10s - likely blocked waiting for a signature request for the secondary hard-link member that should have been skipped") + } + if err := <-peerErrCh; err != nil { + t.Fatalf("peer goroutine: %v", err) + } + + destPrimary := filepath.Join(destRoot, "aaa-primary.txt") + destSecondary := filepath.Join(destRoot, "zzz-secondary.txt") + destUnrelated := filepath.Join(destRoot, "unrelated.txt") + + primaryInfo, err := os.Stat(destPrimary) + if err != nil { + t.Fatalf("Stat %q: %v", destPrimary, err) + } + secondaryInfo, err := os.Stat(destSecondary) + if err != nil { + t.Fatalf("Stat %q: %v", destSecondary, err) + } + unrelatedInfo, err := os.Stat(destUnrelated) + if err != nil { + t.Fatalf("Stat %q: %v", destUnrelated, err) + } + + if !os.SameFile(primaryInfo, secondaryInfo) { + t.Errorf("%q and %q are independent files, want them hard-linked", destPrimary, destSecondary) + } + if os.SameFile(primaryInfo, unrelatedInfo) { + t.Errorf("%q and %q are the same file, want independent", destPrimary, destUnrelated) + } + + got, err := os.ReadFile(destSecondary) + if err != nil { + t.Fatalf("ReadFile(%q): %v", destSecondary, err) + } + if string(got) != content { + t.Errorf("%q content = %q, want %q", destSecondary, got, content) + } +} + // TestSender_ConnectionDropsMidTransfer confirms a dropped connection // produces a prompt, clear error - not a hang and not a silently // swallowed failure. The "receiver" here reads the file list (so Sender @@ -210,7 +459,7 @@ func TestSender_ConnectionDropsMidTransfer(t *testing.T) { }() errCh := make(chan error, 1) - go func() { errCh <- Sender(sender, srcRoot, sync.WalkOptions{Recursive: true}, nil) }() + go func() { errCh <- Sender(sender, srcRoot, sync.WalkOptions{Recursive: true}, nil, false) }() select { case err := <-errCh: @@ -234,7 +483,7 @@ func TestReceiver_ConnectionDropsMidTransfer(t *testing.T) { receiver := pipeReadWriter{Reader: receiverReadsFromPeer, Writer: receiverWritesToPeer} go func() { - _ = sendFileList(peerWritesToReceiver, []sync.FileEntry{{Path: "file.txt", Mode: 0o644}}) + _ = sendFileList(peerWritesToReceiver, []sync.FileEntry{{Path: "file.txt", Mode: 0o644}}, nil) // Read (and discard) the signature Receiver sends back for // file.txt, then vanish - closing both pipe halves without ever // sending a delta. diff --git a/internal/pipeline/receiver.go b/internal/pipeline/receiver.go index 34ab9f5..903e6d9 100644 --- a/internal/pipeline/receiver.go +++ b/internal/pipeline/receiver.go @@ -11,11 +11,15 @@ import ( ) // Receiver runs the receiving side of a sync over rw: receives the -// sender's file list, then for each entry either creates it directly -// (directories, symlinks - both have everything they need already inside -// the FileEntry, no round trip required) or exchanges a signature/delta -// with the sender (regular files) to reconstruct its bytes, applying -// attributes per opts along the way. +// sender's file list (with its hard-link grouping), then for each entry +// either creates it directly (directories, symlinks - both have +// everything they need already inside the FileEntry, no round trip +// required), exchanges a signature/delta with the sender (a regular file +// that is either unlinked or the first-seen member of a hard-link +// group), or - for every other member of a hard-link group - is skipped +// here entirely and instead recreated as a real hard link once its +// group's first member has been fully written, in the dedicated pass +// below. Attributes are applied per opts along the way. // // A destination file not mentioned in the sender's list is never touched // at all: Receiver only ever acts on paths that appear in the received @@ -23,11 +27,22 @@ import ( // reconcile against it, so nothing here can delete or corrupt an // unrelated file. (Full --delete semantics are explicitly out of scope.) func Receiver(rw io.ReadWriter, dest string, opts sync.AttrOptions) error { - entries, err := recvFileList(rw) + entries, groups, err := recvFileList(rw) if err != nil { return fmt.Errorf("receiving file list: %w", err) } + // secondary marks every hard-link group member except the first: + // group[0] is written normally, like any other regular file, and + // every other member is linked to it in the dedicated pass below + // instead of being written out a second time. + secondary := make(map[string]bool) + for _, group := range groups { + for _, path := range group[1:] { + secondary[path] = true + } + } + // Directory attributes are deferred to a final pass below, applied // deepest-first, rather than immediately when each directory is // created: applying them immediately would have the filesystem @@ -37,7 +52,10 @@ func Receiver(rw io.ReadWriter, dest string, opts sync.AttrOptions) error { // at all. Walk's own sort guarantees a parent directory's entry // always precedes its children's in entries, so collecting them here // in list order and processing that collection in reverse gives - // children-before-parents for free, without a second sort. + // children-before-parents for free, without a second sort. The + // hard-link pass runs before this one for the same reason: os.Link + // creates a new directory entry too, and doing that after a + // directory's final mtime was already set would bump it right back. var dirEntries []sync.FileEntry for _, entry := range entries { @@ -59,6 +77,9 @@ func Receiver(rw io.ReadWriter, dest string, opts sync.AttrOptions) error { return fmt.Errorf("creating symlink %q: %w", entry.Path, err) } continue + + case secondary[entry.Path]: + continue } if err := receiveRegularFile(rw, destPath, entry, opts); err != nil { @@ -66,6 +87,12 @@ func Receiver(rw io.ReadWriter, dest string, opts sync.AttrOptions) error { } } + for _, group := range groups { + if err := sync.ApplyHardLinks(dest, group); err != nil { + return fmt.Errorf("linking hard-link group starting at %q: %w", group[0], err) + } + } + for i := len(dirEntries) - 1; i >= 0; i-- { entry := dirEntries[i] destPath := filepath.Join(dest, filepath.FromSlash(entry.Path)) diff --git a/internal/pipeline/sender.go b/internal/pipeline/sender.go index 3991ceb..6329d00 100644 --- a/internal/pipeline/sender.go +++ b/internal/pipeline/sender.go @@ -11,28 +11,61 @@ import ( ) // Sender runs the sending side of a sync over rw: walks and filters src, -// sends the resulting file list, then for each regular-file entry -// receives the receiver's signature, computes a delta against the -// current source bytes, and sends it back. +// detects which entries are hard-linked to each other (only if +// hardLinks is true - see below), sends the resulting file list (with +// that grouping attached), then for each regular-file entry that isn't a +// secondary member of a hard-link group receives the receiver's +// signature, computes a delta against the current source bytes, and +// sends it back. // // Directories and symlinks are deliberately not part of this exchange at // all: a directory has no byte content to diff, and a symlink's entire // "content" is its LinkTarget, which already travels inside the FileEntry -// in the file list itself. Only regular files need a signature/delta -// round trip. -func Sender(rw io.ReadWriter, src string, walkOpts sync.WalkOptions, rules []sync.Rule) error { +// in the file list itself. A hard-linked group's second-and-later members +// are skipped the same way, for a different reason: their data is +// byte-identical to the group's first member by definition (they're the +// same inode), so exchanging a signature/delta for them would just +// re-transfer bytes the receiver is about to get for free via +// sync.ApplyHardLinks instead. +// +// hardLinks mirrors real rsync's own -H/--hard-links flag: off by +// default, and deliberately not implied by --archive (real rsync's -a is +// -rlptgoD, no H) - detecting hard links means an extra Lstat per entry, +// a cost real rsync doesn't spend unless asked to, so grsync doesn't +// either. +func Sender(rw io.ReadWriter, src string, walkOpts sync.WalkOptions, rules []sync.Rule, hardLinks bool) error { entries, err := sync.Walk(src, walkOpts) if err != nil { return fmt.Errorf("walking %q: %w", src, err) } entries = sync.FilterEntries(entries, rules) - if err := sendFileList(rw, entries); err != nil { + // DetectHardLinks is skipped entirely, not just left to return no + // groups, in two cases: hardLinks wasn't requested, or the platform + // can't detect them at all (DetectHardLinks would always return + // nothing there anyway, but skipping the call also skips the + // Lstat-per-entry cost that would otherwise buy nothing). + var groups []sync.HardLinkGroup + if hardLinks && sync.HardLinksSupported() { + groups, err = sync.DetectHardLinks(src, entries) + if err != nil { + return fmt.Errorf("detecting hard links in %q: %w", src, err) + } + } + + if err := sendFileList(rw, entries, groups); err != nil { return fmt.Errorf("sending file list: %w", err) } + secondary := make(map[string]bool) + for _, group := range groups { + for _, path := range group[1:] { + secondary[path] = true + } + } + for _, entry := range entries { - if entry.IsDir || entry.Mode&fs.ModeSymlink != 0 { + if entry.IsDir || entry.Mode&fs.ModeSymlink != 0 || secondary[entry.Path] { continue } diff --git a/internal/pipeline/ssh_test.go b/internal/pipeline/ssh_test.go index 8ddd865..398a3bf 100644 --- a/internal/pipeline/ssh_test.go +++ b/internal/pipeline/ssh_test.go @@ -79,7 +79,7 @@ func TestSSHLocalhost_SyncRoundTrip(t *testing.T) { sendErrCh := make(chan error, 1) go func() { - sendErrCh <- Sender(session, src, sync.WalkOptions{Recursive: true}, nil) + sendErrCh <- Sender(session, src, sync.WalkOptions{Recursive: true}, nil, false) }() select {