[PR] fix(rmqctl): tolerate working-tree CRLF in the catalog verify gate [rocketmq-dashboard]

["zjncs (via GitHub)" <gi...@apache.org>]

zjncs opened a new pull request, #5618:
URL: https://github.com/apache/rocketmq-dashboard/pull/5618

   Closes #5617
   
   ## Problem
   
   `cataloggen -check` compared the on-disk generated catalog byte-for-byte. `go/format` always emits LF, but a Git for Windows checkout with the **installer-default `core.autocrlf=true`** (and no `.gitattributes` in the repo) smudges the file to CRLF — so the freshness gate is permanently red on default Windows checkouts and `catalog-generate` needlessly rewrites the file with flipped EOLs. Demonstrated on unmodified master: `-check` exits 1 with "is stale" while the working-tree file and the generated output are **byte-identical after EOL normalization** (42516 bytes/770 CRLF lines vs 41746 bytes/LF). The digest is EOL-independent; only the verify comparison is broken.
   
   ## Fix
   
   Strip CR from CRLF pairs on both sides before comparing:
   
   ```go
   if !bytes.Equal(normalizeEOL(actual), normalizeEOL(expected)) { ... }
   ```
   
   ## Verification
   
   - New `verify_test.go` with two cases: **`TestCheckToleratesCRLFWorkingTree`** (generates, rewrites the output with CRLF to simulate the git smudge, runs `-check`, expects nil) — **FAILS on master** ("is stale"), **PASSES with this change**; **`TestCheckStillDetectsRealStaleness`** (corrupts a version token, expects the stale error) — passes both before and after, proving the tolerance is not vacuous
   - Mutation check: stashing only the `main.go` change makes the tolerance test fail again; restoring it passes (`-count=1` throughout)
   - Regression: full `./internal/catalog/...` suite green
   
   ## Collision note
   
   Same file as our open #5614 (single-file shadowing fix) but a different function — #5614 touches the read branch at ~302-322, this change touches `verify` at ~824-833; disjoint regions, trivial rebase either way. No foreign PR touches the file.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] fix(rmqctl): tolerate working-tree CRLF in the catalog verify gate [rocketmq-dashboard]

["zjncs (via GitHub)" <gi...@apache.org>]

zjncs commented on PR #5618:
URL: https://github.com/apache/rocketmq-dashboard/pull/5618#issuecomment-6076680044

   Hi @lizhimins, thanks for the detailed review. I've switched to the `.gitattributes` approach as suggested, rebased onto rocketmq-studio c99b9ad5 and retargeted the PR. Point by point:
   
   **Switched to `.gitattributes`.** Added a root `.gitattributes` containing:
   
   ```
   *.go text eol=lf
   ```
   
   The committed Go catalog (`rmqctl/internal/catalog/catalog_gen.go`) is the only generated artifact git ever checks out — `docs/generated/` (the markdown and SDK JSON outputs) is in `.gitignore`, so no entries are needed for those. This pins the working-tree copy to LF even on a `core.autocrlf=true` checkout, which fixes the verify gate and the `make catalog-generate` rewrite in one place, and since `verify` stays a byte-exact `bytes.Equal`, a catalog that was actually committed with CRLF still fails `-check` — the freshness check is not weakened.
   
   **`verify` restored to byte-exact.** `generate/main.go` is back to the plain `bytes.Equal` comparison (plus a doc comment recording why the gate stays EOL-strict and pointing at the `.gitattributes` pin).
   
   **Trailing newlines restored** on `main.go` and `verify_test.go`; `gofmt -l .` is clean, so `make fmt` (ci.yml) passes.
   
   **Tests updated to the new semantics** in `verify_test.go`:
   - `TestCheckRejectsLineEndingDrift` — a CRLF copy of an otherwise current catalog must fail `-check` (the inverse of the old tolerance test; a CRLF-committed catalog can no longer pass undetected).
   - `TestCheckStillDetectsRealStaleness` — genuinely different content is still rejected.
   - `TestGitAttributesPinsGoFilesToLF` — guards the `*.go text eol=lf` pin and fails if the entry is dropped or changed. Since the rmqctl CI job runs on ubuntu-latest, a removed rule would otherwise never show up in CI.
   
   `go test ./...` is green. One note: existing Windows checkouts need a one-time refresh of the affected files after pulling this (e.g. `git checkout -- rmqctl/internal/catalog/catalog_gen.go`); fresh clones are unaffected.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org