From 03c000eb20b4f36428b8c64056bc54de8cd767ac Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Tue, 2 Feb 2021 21:53:30 -0500 Subject: [PATCH 1/2] Use `# min-llvm-version: 11.0` to force a minimum LLVM version, rather than ad-hoc internal solution. In particular: the specific code to define LLVM_VERSION_11_PLUS here was, for some reason, using `$(shell ...)` with bash-specific variable replacement code. On non-bash platforms like dash, that `shell` invocation would fail, and the LLVM_VERSION_11_PLUS check would always fail, the test would always be ignored, and thus be treated as a "success" (in the sense that `--bless` would never do anything). This was causing me a lot of pain. --- src/test/run-make-fulldeps/coverage-llvmir/Makefile | 6 +----- src/test/run-make-fulldeps/coverage-reports/Makefile | 6 +----- src/test/run-make-fulldeps/coverage-spanview/Makefile | 6 +----- src/test/run-make-fulldeps/coverage/coverage_tools.mk | 7 ------- 4 files changed, 3 insertions(+), 22 deletions(-) diff --git a/src/test/run-make-fulldeps/coverage-llvmir/Makefile b/src/test/run-make-fulldeps/coverage-llvmir/Makefile index 54fc3d168645f..7d9121ee2f834 100644 --- a/src/test/run-make-fulldeps/coverage-llvmir/Makefile +++ b/src/test/run-make-fulldeps/coverage-llvmir/Makefile @@ -1,4 +1,5 @@ # needs-profiler-support +# min-llvm-version: 11.0 -include ../coverage/coverage_tools.mk @@ -48,12 +49,7 @@ else -DINSTR_PROF_ORDERFILE='$(DATA_SECTION_PREFIX)__llvm_orderfile' endif -ifeq ($(LLVM_VERSION_11_PLUS),true) all: test_llvm_ir -else -$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.) -all: -endif test_llvm_ir: # Compile the test program with non-experimental coverage instrumentation, and generate LLVM IR diff --git a/src/test/run-make-fulldeps/coverage-reports/Makefile b/src/test/run-make-fulldeps/coverage-reports/Makefile index f98245b4a9944..ebbe065f5235f 100644 --- a/src/test/run-make-fulldeps/coverage-reports/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports/Makefile @@ -1,5 +1,6 @@ # needs-profiler-support # ignore-windows-gnu +# min-llvm-version: 11.0 # FIXME(mati865): MinGW GCC miscompiles compiler-rt profiling library but with Clang it works # properly. Since we only have GCC on the CI ignore the test for now. @@ -67,12 +68,7 @@ ifdef RUSTC_BLESS_TEST DEBUG_FLAG=--debug endif -ifeq ($(LLVM_VERSION_11_PLUS),true) all: $(patsubst $(SOURCEDIR)/lib/%.rs,%,$(wildcard $(SOURCEDIR)/lib/*.rs)) $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs)) -else -$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.) -all: -endif # Ensure there are no `expected` results for tests that may have been removed or renamed .PHONY: clear_expected_if_blessed diff --git a/src/test/run-make-fulldeps/coverage-spanview/Makefile b/src/test/run-make-fulldeps/coverage-spanview/Makefile index 84b5d0e522f8c..2713e7d52ffd3 100644 --- a/src/test/run-make-fulldeps/coverage-spanview/Makefile +++ b/src/test/run-make-fulldeps/coverage-spanview/Makefile @@ -1,4 +1,5 @@ # needs-profiler-support +# min-llvm-version: 11.0 -include ../coverage/coverage_tools.mk @@ -20,12 +21,7 @@ For revisions in Pull Requests (PR): endef export SPANVIEW_HEADER -ifeq ($(LLVM_VERSION_11_PLUS),true) all: $(patsubst $(SOURCEDIR)/lib/%.rs,%,$(wildcard $(SOURCEDIR)/lib/*.rs)) $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs)) -else -$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.) -all: -endif # Ensure there are no `expected` results for tests that may have been removed or renamed .PHONY: clear_expected_if_blessed diff --git a/src/test/run-make-fulldeps/coverage/coverage_tools.mk b/src/test/run-make-fulldeps/coverage/coverage_tools.mk index 4d340d4b1dadd..11fd824e5272f 100644 --- a/src/test/run-make-fulldeps/coverage/coverage_tools.mk +++ b/src/test/run-make-fulldeps/coverage/coverage_tools.mk @@ -14,10 +14,3 @@ # Therefore, `-C link-dead-code` is no longer automatically enabled. UNAME = $(shell uname) - -# Rust option `-Z instrument-coverage` uses LLVM Coverage Mapping Format version 4, -# which requires LLVM 11 or greater. -LLVM_VERSION_11_PLUS := $(shell \ - LLVM_VERSION=$$("$(LLVM_BIN_DIR)"/llvm-config --version) && \ - LLVM_VERSION_MAJOR=$${LLVM_VERSION/.*/} && \ - [ $$LLVM_VERSION_MAJOR -ge 11 ] && echo true || echo false) From 5ce67106688664eb1bfd8abae129e7c548de0351 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 3 Feb 2021 16:02:41 -0500 Subject: [PATCH 2/2] Ignore broken test. (Test was silently ignored on Linux CI prior to parent commit that switched to using `# min-llvm-version`. But the switch made the ignoring stop, exposing other brokenness in the form of bash-dependent syntax in the `$(shell ...)` invocations.) --- src/test/run-make-fulldeps/coverage-reports/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/run-make-fulldeps/coverage-reports/Makefile b/src/test/run-make-fulldeps/coverage-reports/Makefile index ebbe065f5235f..a700cf68cd9da 100644 --- a/src/test/run-make-fulldeps/coverage-reports/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports/Makefile @@ -1,3 +1,4 @@ +# ignore-test Broken; accidentally silently ignored on Linux CI; FIXME(#81688) # needs-profiler-support # ignore-windows-gnu # min-llvm-version: 11.0