Skip to content

Commit 9e5d58f

Browse files
committed
Auto merge of #81688 - pnkfelix:fix-llvm-version-check-in-run-make-tests, r=simulacrum
Use `# min-llvm-version: 11.0` to force a minimum LLVM version 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). * Note in particular that GNU Make treats the SHELL variable as a very special case: it does not inherit the value of SHELL from the user's environment. Except on Windows. See more explanation in the [GNU Make docs](https://www.gnu.org/software/make/manual/html_node/Choosing-the-Shell.html). * The effect of this is that these tests end up using `/bin/sh` (except on Windows) for their `$(shell ...)` invocations, and thus we see differing behaviors depending on whether your `/bin/sh` links to `/bin/dash` or to `/bin/bash`. This was causing me a lot of pain.
2 parents 6a388dc + 5ce6710 commit 9e5d58f

File tree

4 files changed

+4
-22
lines changed

4 files changed

+4
-22
lines changed

src/test/run-make-fulldeps/coverage-llvmir/Makefile

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# needs-profiler-support
2+
# min-llvm-version: 11.0
23

34
-include ../coverage/coverage_tools.mk
45

@@ -48,12 +49,7 @@ else
4849
-DINSTR_PROF_ORDERFILE='$(DATA_SECTION_PREFIX)__llvm_orderfile'
4950
endif
5051

51-
ifeq ($(LLVM_VERSION_11_PLUS),true)
5252
all: test_llvm_ir
53-
else
54-
$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.)
55-
all:
56-
endif
5753

5854
test_llvm_ir:
5955
# Compile the test program with non-experimental coverage instrumentation, and generate LLVM IR

src/test/run-make-fulldeps/coverage-reports/Makefile

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
# ignore-test Broken; accidentally silently ignored on Linux CI; FIXME(#81688)
12
# needs-profiler-support
23
# ignore-windows-gnu
4+
# min-llvm-version: 11.0
35

46
# FIXME(mati865): MinGW GCC miscompiles compiler-rt profiling library but with Clang it works
57
# properly. Since we only have GCC on the CI ignore the test for now.
@@ -67,12 +69,7 @@ ifdef RUSTC_BLESS_TEST
6769
DEBUG_FLAG=--debug
6870
endif
6971

70-
ifeq ($(LLVM_VERSION_11_PLUS),true)
7172
all: $(patsubst $(SOURCEDIR)/lib/%.rs,%,$(wildcard $(SOURCEDIR)/lib/*.rs)) $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs))
72-
else
73-
$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.)
74-
all:
75-
endif
7673

7774
# Ensure there are no `expected` results for tests that may have been removed or renamed
7875
.PHONY: clear_expected_if_blessed

src/test/run-make-fulldeps/coverage-spanview/Makefile

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
# needs-profiler-support
2+
# min-llvm-version: 11.0
23

34
-include ../coverage/coverage_tools.mk
45

@@ -20,12 +21,7 @@ For revisions in Pull Requests (PR):
2021
endef
2122
export SPANVIEW_HEADER
2223

23-
ifeq ($(LLVM_VERSION_11_PLUS),true)
2424
all: $(patsubst $(SOURCEDIR)/lib/%.rs,%,$(wildcard $(SOURCEDIR)/lib/*.rs)) $(patsubst $(SOURCEDIR)/%.rs,%,$(wildcard $(SOURCEDIR)/*.rs))
25-
else
26-
$(info Rust option `-Z instrument-coverage` requires LLVM 11 or higher. Test skipped.)
27-
all:
28-
endif
2925

3026
# Ensure there are no `expected` results for tests that may have been removed or renamed
3127
.PHONY: clear_expected_if_blessed

src/test/run-make-fulldeps/coverage/coverage_tools.mk

-7
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,3 @@
1414
# Therefore, `-C link-dead-code` is no longer automatically enabled.
1515

1616
UNAME = $(shell uname)
17-
18-
# Rust option `-Z instrument-coverage` uses LLVM Coverage Mapping Format version 4,
19-
# which requires LLVM 11 or greater.
20-
LLVM_VERSION_11_PLUS := $(shell \
21-
LLVM_VERSION=$$("$(LLVM_BIN_DIR)"/llvm-config --version) && \
22-
LLVM_VERSION_MAJOR=$${LLVM_VERSION/.*/} && \
23-
[ $$LLVM_VERSION_MAJOR -ge 11 ] && echo true || echo false)

0 commit comments

Comments
 (0)