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
19 lines
685 B
Python
Executable File
19 lines
685 B
Python
Executable File
#!/usr/bin/env python3
|
|
# Copyright 2017 Google Inc. All rights reserved.
|
|
#
|
|
# Licensed under the Apache License, Version 2.0 (the "License");
|
|
# you may not use this file except in compliance with the License.
|
|
# You may obtain a copy of the License at
|
|
#
|
|
# http://www.apache.org/licenses/LICENSE-2.0
|
|
#
|
|
# Unless required by applicable law or agreed to in writing, software
|
|
# distributed under the License is distributed on an "AS IS" BASIS,
|
|
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
|
# See the License for the specific language governing permissions and
|
|
# limitations under the License.
|
|
import gtest_parallel
|
|
import sys
|
|
|
|
sys.exit(gtest_parallel.main())
|