SC-18: hard-link detection/preservation wired into pipeline

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
 

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")

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})
 }

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 {

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

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)

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 {