Makefile fix and speed up 'clean'

[pdillinger]

Summary:

• Fix Makefile default target (was ordered after a folly target)
• Improved the speed of make clean by using just one find and by pruning "hidden" .* and third-party directories that should not be modified anyway.
• Reduce excessive output from make clean

Test Plan: manual

[meta-codesync]

@pdillinger has imported this pull request. If you are a Meta employee, you can view this in D105972958.

[github-actions]

✅ clang-tidy: No findings on changed lines

Completed in 0.0s.

[github-actions]

🟡 Codex Code Review

Auto-triggered after CI passed — reviewing commit 913debf

---

❌ Codex review failed before producing findings.

WARNING: proceeding, even though we could not update PATH: Refusing to create helper binaries under temporary dir "/tmp" (codex_home: AbsolutePathBuf("/tmp/codex-home"))
error: the argument '--base <BRANCH>' cannot be used with '[PROMPT]'

Usage: codex exec review --commit <SHA> --base <BRANCH> --title <TITLE> --model <MODEL> --config <key=value> --dangerously-bypass-approvals-and-sandbox --output-last-message <FILE> [PROMPT]

For more information, try '--help'.

---

ℹ️ About this response

Generated by Codex CLI.
Review methodology: claude_md/code_review.md

Limitations:

• Codex may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /codex-review [context] — Request a code review
• /codex-query <question> — Ask about the PR or codebase

[github-actions]

✅ Claude Code Review

Auto-triggered after CI passed — reviewing commit 913debf

---

Summary

Solid Makefile maintenance PR that fixes a real default-target bug, improves cross-platform consistency, and speeds up make clean. One medium-severity functional gap in the fallback path and a few minor items.

High-severity findings (0):

No high-severity findings.

Full review (click to expand)

Findings

🟡 MEDIUM

M1. Fallback path omits .gcda/.gcno cleanup — Makefile:1295

• Issue: The else branch of the new clean-rocks target only runs $(FIND) . -name "*.[oda]" -exec rm -f {} \;, but the original code also ran a second find for *.gcda and *.gcno coverage files. On systems where -regextype awk is not supported (macOS with BSD find), gcov files will accumulate and not be cleaned.
• Root cause: When consolidating the two find commands into one regex-based command, the fallback was not updated to preserve full parity.
• Suggested fix: Add the second find to the fallback:

  else \
      $(FIND) . -name "*.[oda]" -exec rm -f {} \; ; \
      $(FIND) . -type f \( -name "*.gcda" -o -name "*.gcno" \) -exec rm -f {} \; ; \
  fi

M2. One remaining bare find not converted to $(FIND) — Makefile:1105

• Issue: The check target at line 1105 still uses bare find to clean up stale test directories: find $$test_tmpdir_parent -maxdepth 1 -name 'rocksdb.*' .... This will fail on OpenBSD where find is BSD find and gfind should be used.
• Root cause: The PR converted find to $(FIND) in check_0, valgrind_check_0, and clean-ext-libraries-bin, but missed this instance in the check target.
• Suggested fix: Change find $$test_tmpdir_parent to $(FIND) $$test_tmpdir_parent.

🟢 LOW / NIT

L1. Consider using -exec ... + instead of -exec ... \; — Makefile:1293

• Issue: The find command uses -exec rm -f {} \; which forks rm once per file. Using -exec rm -f {} + batches arguments, reducing fork/exec overhead.
• Suggested fix: Change \; to + in both the primary and fallback find commands.

L2. Duplicate default: all rules — Makefile:329,768

• Issue: After this PR, default: all appears at both ~line 329 and line 768. Harmless in GNU Make but the one at 768 is now redundant.
• Suggested fix: Consider removing the default: all at line 768 in a follow-up.

L3. Minor: echo messages don't use quotes — Makefile:1288-1290

• Issue: @echo Removing link targets works but quoting would be safer against special characters.
• Suggested fix: @echo 'Removing link targets'

Cross-Component Analysis

Context	Affected?	Assessment
Linux (GNU find)	Yes	Full regex path works correctly. Genuine speedup.
macOS (BSD find)	Yes	Fallback triggered. Missing gcov cleanup (M1).
OpenBSD (gfind)	Yes	GNU find via gfind. Full regex path works.
CI (GitHub Actions)	Yes	Linux-based. No issues. $(FIND) always defined via make_config.mk.
make (no args)	Fixed	Default target now correctly resolves to all instead of checkout_folly.
Java builds	Yes	rm -rf consolidation is safe and equivalent.

Assumption stress-test:

• "prune pattern is safe" — Verified. .git and third-party/ contain no build artifacts that need cleaning. Claim holds.
• "regex correctly matches file extensions" — Verified. GNU find's full-path matching prevents false positives like .old. Claim holds.
• "first rule fix" — Verified. common.mk defines no rules. First rule will be default: all. Claim holds.

Positive Observations

• The default target bug fix is correct and well-placed.
• The $(FIND) consistency improvements are good cross-platform hygiene.
• The .git directory pruning alone accounts for the majority of the speedup.
• The java/Makefile consolidation is clean and obviously correct.

---

ℹ️ About this response

Generated by Claude Code.
Review methodology: claude_md/code_review.md

Limitations:

• Claude may miss context from files not in the diff
• Large PRs may be truncated
• Always apply human judgment to AI suggestions

Commands:

• /claude-review [context] — Request a code review
• /claude-query <question> — Ask about the PR or codebase

[meta-codesync]

@pdillinger merged this pull request in e82af29.