Configurable chunking algorithms

[unlightable]

Implements what is described in #441 and some more

[folbricht]

A few issues found while reviewing; the fastcdc one (silently dropping min bytes when the boundary is exactly at min size) looks like the most serious.

---

Generated by Claude Code

[unlightable]

Fixed stuff pointed at above and some few issues found along the way. Linter should be happy too, I think.

[folbricht]

Thank you. You can regenerate the docs with:

go run ./cmd/desync gendocs docs/cli

And to run the linter locally, you can use:

golangci-lint run ./...

Plus

go vet ./...

[folbricht]

addChunkerOptions also registers -m/--chunk-size on extract, but nothing reads it here. FileSeed.RegenerateIndex takes min/avg/max from the seed's own index, which is the right behavior. So desync extract --chunk-size … is accepted and silently ignored.

I'd only register the two flags that are actually used:

flags.StringVar(&opt.cmdChunkerOptions.name, "chunker", chunkers.DefaultChunkerName,
	fmt.Sprintf("chunking algorithm used when regenerating seed indexes, pick from %v", chunkers.RegisteredNames()))
flags.StringVar(&opt.cmdChunkerOptions.options, "chunker-options", "",
	"additional chunker-specific options used when regenerating seed indexes")

It might also be worth checking the chunker name up front in runExtract. Right now an invalid --chunker only fails partway through extraction, when a seed needs regenerating:

if chunkers.FindChunkerByName(opt.cmdChunkerOptions.name) == nil {
	return fmt.Errorf("unknown chunker '%s'", opt.cmdChunkerOptions.name)
}

The CLI docs need regenerating after this either way (go run ./cmd/desync gendocs docs/cli). That's the step currently failing in the Lint job.

---

Generated by Claude Code

[folbricht]

I'm not a fan of how the new chunker is propagated everywhere via arguments. It breaks the interfaces. Given that there's only ever one algorithm anyway, how about using the hashing alg as a precedent and making this exported global with a global flag. That avoids chaining it through all over the place. IndexFromFile, Seed.RegenerateIndex, SeedSequencer.*, AssembleOptions go back to how they were before, and extract no longer needs its own addChunkerOptions (the ignored -m flag goes away).

[unlightable]

I'm not a fan of how the new chunker is propagated everywhere via arguments. It breaks the interfaces. Given that there's only ever one algorithm anyway, how about using the hashing alg as a precedent and making this exported global with a global flag. That avoids chaining it through all over the place. IndexFromFile, Seed.RegenerateIndex, SeedSequencer.*, AssembleOptions go back to how they were before, and extract no longer needs its own addChunkerOptions (the ignored -m flag goes away).

I'm not a fan of long argument chains either, and although global state is a solution, it does have a price of hidden dependencies that make testing/reusing stuff harder.
I would prefer it being obvious, that index (re-)generation depends on a chunker (and a digest algorithm btw). It forces you to provide it and avoid silently using "incorrect" one, e.g. how I missed it in extract command.

But it's your choice in the end. Let me do another pass over how and where they are explicit, and if you still dislike how it turns out - I'll extract it into global.

[folbricht]

This signature still has chunkerName and chunkerOptions, but nothing passes or reads them, and the Seed interface went back to the 4-argument form in the last commit. It compiles only because selfSeed is never used as a Seed, so it quietly stops implementing the interface. Looks like a leftover from the earlier approach. Suggest reverting:

func (s *selfSeed) RegenerateIndex(ctx context.Context, n int, attempt int, seedNumber int) error {

---

Generated by Claude Code

[folbricht]

Minor cleanup: since this relies on the segment sizes being exact, io.ReadFull expresses it directly and lets you drop the readSizeLeft loop. A short read then comes back as io.ErrUnexpectedEOF with the segment index attached, rather than a bare io.EOF:

segSize := c.segmentSizes[c.nextSegmentIdx]
buf := c.mergedSegments[c.mergedSegmentsSize : c.mergedSegmentsSize+segSize]
if _, err := io.ReadFull(c.r, buf); err != nil {
	return fmt.Errorf("segment %d: %w", c.nextSegmentIdx, err)
}
c.mergedSegmentsSize += segSize

---

Generated by Claude Code

[folbricht]

uint32(kbValue * 1024) silently wraps for values of 4194304 KB and up (4194304 becomes 0, which then falls back to the default), and negative input turns into a huge number. The later min(..., Max) clamp hides the negative case but not the wrap. Suggest parsing as unsigned and bounds-checking:

kb, err := strconv.ParseUint(kv[1], 10, 32)
if err != nil || kb > math.MaxUint32/1024 {
	return options, fmt.Errorf("invalid smallSegmentBreakpoint '%s'", kv[1])
}
options.smallSegmentBreakpoint = uint32(kb * 1024)

---

Generated by Claude Code

[folbricht]

Suggestion for a generic invariant test over every registered chunker: chunks must be contiguous (each start equals the end of the previous chunk), no larger than max, and concatenate back to the input. The FastCDC min-boundary bug fixed earlier in this PR dropped min bytes from the index; the new min-boundary test covers that one path, but this would catch the whole class of bug, including in chunkers added later. I ran it against the PR's first revision and it fails on fastcdc with a 256-byte gap; it passes on the current head. Segmentaware could get the same check with a generated CSV segment file.

func TestChunkersContiguous(t *testing.T) {
	data := make([]byte, 4<<20)
	rand.New(rand.NewSource(1)).Read(data)
	for _, name := range RegisteredNames() {
		if name == "segmentaware" {
			continue // needs a segment source; cover separately
		}
		t.Run(name, func(t *testing.T) {
			c, err := NewChunker(name, bytes.NewReader(data),
				ChunkerParams{Min: 256, Avg: 1024, Max: 4096})
			require.NoError(t, err)
			var pos uint64
			var out []byte
			for {
				start, b, err := c.Next()
				require.NoError(t, err)
				if len(b) == 0 {
					break
				}
				require.Equal(t, pos, start, "gap or overlap")
				require.LessOrEqual(t, len(b), 4096)
				pos += uint64(len(b))
				out = append(out, b...)
			}
			require.Equal(t, data, out)
		})
	}
}

---

Generated by Claude Code

[folbricht]

I don't have much time to look too closely this week, will be back next week though.

[unlightable]

No pressure.