mirror of
https://github.com/facebook/rocksdb.git
synced 2026-07-07 14:47:40 +08:00
Peter Dillinger
f6fabdb64f
Summary: Local Make builds silently reuse object files even when build parameters (DEBUG_LEVEL, sanitizers, ASSERT_STATUS_CHECKED, RTTI, etc.) change, since all object files share the same paths. This leads to confusing linker errors, sanitizer false negatives, ODR violations, and "phantom" bugs that are easy to miss -- a pitfall for humans and especially for AI agents driving builds non-interactively. This change improves the Make experience on three fronts: 1. Build-parameter change detection in the Makefile. After flags are fully resolved, we hash the effective compile/link inputs (CC|CXX|CFLAGS|CXXFLAGS|LDFLAGS|EXEC_LDFLAGS) into a per-OBJ_DIR .build_signature stamp and compare against the previous build: - Default: stop with a clear error when parameters changed. This is optimized for the expert who might not intend a full rebuild, and for avoiding CI inefficiency. - AUTO_CLEAN=1: automatically run clean-rocks and proceed. - ALLOW_BUILD_PARAMETER_CHANGE=1: skip the check (e.g. intentionally mixing DEBUG_LEVEL=1 and DEBUG_LEVEL=2 objects). The check is skipped for dry runs (-n) and for non-building / self-cleaning targets (clean*, format, check-*, tags, gen-pc, check-progress, watch-log, release, coverage, the asan_*/ubsan_* variants, etc.). make_config.mk is removed only by the top-level `clean` target, not by clean-rocks, so the auto-clean does not delete the make_config.mk already included by the running make. The signature is removed last in clean-rocks so an interrupted clean keeps change detection meaningful. 3. New build_tools/rockstest.sh and build_tools/rocksptest.sh helpers that build and run unit tests in one step. They set AUTO_CLEAN=1 and build with -j<NCORES> (computed like the Makefile). rocksptest.sh runs one or more binaries under the checked-in gtest-parallel (sharing one parallel worker pool); rockstest.sh runs a single binary directly for a small number of cases. Both reject a leading-option first argument; `rockstest.sh install` writes ~/bin/rockstest and ~/bin/rocksptest shims that defer to the in-tree scripts (with a friendly message when run outside a source root). This is a big win for AI-agent workflows: one command instead of a separate `make` then test-run invocation, removing the overhead of monitoring two long commands, while avoiding serial builds and serial test execution. 4. CLAUDE.md guidance updated to recommend AUTO_CLEAN=1 for manual make invocations and the rocks(p)test.sh helpers for running tests Although there might be drive to consolidate around the BUCK build for internal use, I'll note that last I checked it was around ~20 minutes to run all the unit tests under buck and ~2 minutes under `make check`. Bonus: the first revision of this had copyright/license mistakes, and this change corrects similar errors elsewhere, with CLAUDE.md advice updated. Pull Request resolved: https://github.com/facebook/rocksdb/pull/14883 Test Plan: Manual verification of the Makefile change detection: - Confirmed the resolved-flags hash is deterministic across runs for the same goal, and differs between configs (e.g. default/shared vs static_lib, and with/without ASSERT_STATUS_CHECKED). - Unchanged parameters: rebuild does not error. - Changed parameters: errors by default; AUTO_CLEAN=1 runs clean-rocks and proceeds; ALLOW_BUILD_PARAMETER_CHANGE=1 bypasses the check. - Dry runs (make -n) and exempt targets (e.g. `make check-progress`, list_all_tests, gen-pc) do not trip the check and leave the stamp untouched. - Reproduced the ASSERT_STATUS_CHECKED=1 auto-clean path end to end and confirmed it no longer fails with "No rule to make target 'make_config.mk'"; verified `make clean` removes make_config.mk while `clean-rocks` does not, and that the signature is deleted as the final clean-rocks step. Manual verification of the helper scripts: - shellcheck-clean and `bash -n` pass for both scripts. - Usage/guard behavior: missing argument and leading-option first argument are rejected with a clear message. - `rockstest.sh install` (validated against a temp HOME) generates both shims; running a shim outside a source root prints "(Not in a rocksdb source root directory?)" and exits non-zero. - rocksptest.sh argument splitting verified for single binary, multiple binaries, and binaries followed by gtest-parallel/test args. - .gitignore patterns verified with `git check-ignore` for .build_signature (root and jl/jls) and build_tools/__pycache__/. Reviewed By: xingbowang Differential Revision: D109713512 Pulled By: pdillinger fbshipit-source-id: a801d29afa91f53a2dce717db9fe5e5d485cc154
Adding release notes -------------------- When adding release notes for the next release, add a file to one of these directories: unreleased_history/new_features unreleased_history/behavior_changes unreleased_history/public_api_changes unreleased_history/bug_fixes with a unique name that makes sense for your change, preferably using the .md extension for syntax highlighting. There is a script to help, as in $ unreleased_history/add.sh unreleased_history/bug_fixes/crash_in_feature.md or simply $ unreleased_history/add.sh will take you through some prompts. The file should usually contain one line of markdown, and "* " is not required, as it will automatically be inserted later if not included at the start of the first line in the file. Extra newlines or missing trailing newlines will also be corrected. The only times release notes should be added directly to HISTORY are if * A release is being amended or corrected after it is already "cut" but not tagged, which should be rare. * A single commit contains a noteworthy change and a patch release version bump Ordering of entries ------------------- Within each group, entries will be included using ls sort order, so important entries could start their file name with a small three digit number like 100pretty_important.md. The ordering of groups such as new_features vs. public_api_changes is hard-coded in unreleased_history/release.sh Updating HISTORY.md with release notes -------------------------------------- The script unreleased_history/release.sh does this. Run the script before updating version.h to the next development release, so that the script will pick up the version being released. You might want to start with $ DRY_RUN=1 unreleased_history/release.sh | less to check for problems and preview the output. Then run $ unreleased_history/release.sh which will git rm some files and modify HISTORY.md. You still need to commit the changes, or revert with the command reported in the output. Why not update HISTORY.md directly? ----------------------------------- First, it was common to hit unnecessary merge conflicts when adding entries to HISTORY.md, which slowed development. Second, when a PR was opened before a release cut and landed after the release cut, it was easy to add the HISTORY entry to the wrong version's history. This new setup completely fixes both of those issues, with perhaps slightly more initial work to create each entry. There is also now an extra step in using `git blame` to map a release note to its source code implementation, but that is a relatively rare operation.