From 01f5a2a81d02f42d4ea277772d0752e80f8d32d1 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Wed, 3 Feb 2021 16:26:25 -0800 Subject: [PATCH 1/4] Ensures `make` tests run under /bin/dash, like CI, and fixes a Makefile Updates `tools.mk` to explicitly require `SHELL := /bin/dash`, since CI uses `dash` but other environments (including developer local machines) may default to `bash`. Replaces bash-specific shell command in one Makefile with a dash-compatible alternative, and re-enables the affected Makefile test. Removes apparently redundant definition of `UNAME`. --- src/test/run-make-fulldeps/coverage-reports/Makefile | 3 +-- src/test/run-make-fulldeps/coverage/coverage_tools.mk | 2 -- src/test/run-make-fulldeps/tools.mk | 6 ++++++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/test/run-make-fulldeps/coverage-reports/Makefile b/src/test/run-make-fulldeps/coverage-reports/Makefile index a700cf68cd9..31583eaa8fe 100644 --- a/src/test/run-make-fulldeps/coverage-reports/Makefile +++ b/src/test/run-make-fulldeps/coverage-reports/Makefile @@ -1,4 +1,3 @@ -# ignore-test Broken; accidentally silently ignored on Linux CI; FIXME(#81688) # needs-profiler-support # ignore-windows-gnu # min-llvm-version: 11.0 @@ -128,7 +127,7 @@ endif $$( \ for file in $(TMPDIR)/rustdoc-$@/*/rust_out; \ do \ - [[ -x $$file ]] && printf "%s %s " -object $$file; \ + [ -x "$$file" ] && printf "%s %s " -object $$file; \ done \ ) \ 2> "$(TMPDIR)"/show_coverage_stderr.$@.txt \ diff --git a/src/test/run-make-fulldeps/coverage/coverage_tools.mk b/src/test/run-make-fulldeps/coverage/coverage_tools.mk index 11fd824e527..38643aaf902 100644 --- a/src/test/run-make-fulldeps/coverage/coverage_tools.mk +++ b/src/test/run-make-fulldeps/coverage/coverage_tools.mk @@ -12,5 +12,3 @@ # Enabling `-C link-dead-code` is not necessary when compiling with `-Z instrument-coverage`, # due to improvements in the coverage map generation, to add unreachable functions known to Rust. # Therefore, `-C link-dead-code` is no longer automatically enabled. - -UNAME = $(shell uname) diff --git a/src/test/run-make-fulldeps/tools.mk b/src/test/run-make-fulldeps/tools.mk index 634c9ece3f5..29ca30a7cd8 100644 --- a/src/test/run-make-fulldeps/tools.mk +++ b/src/test/run-make-fulldeps/tools.mk @@ -21,6 +21,12 @@ CGREP := "$(S)/src/etc/cat-and-grep.sh" # diff with common flags for multi-platform diffs against text output DIFF := diff -u --strip-trailing-cr +# CI platforms use `/bin/dash`. When compiling in other environments, the +# default may be different (for example, may default to `/bin/bash`), and syntax +# and results could be different. Ensure Makefile `$(shell ...)` invocations +# always run in `dash`. +SHELL := /bin/dash + # This is the name of the binary we will generate and run; use this # e.g. for `$(CC) -o $(RUN_BINFILE)`. RUN_BINFILE = $(TMPDIR)/$(1) From 625803d77ec76ad3ab87b1c1c8db7ac7e213761d Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Fri, 5 Feb 2021 17:50:37 -0800 Subject: [PATCH 2/4] Set SHELL = /bin/dash only if it exists --- src/test/run-make-fulldeps/tools.mk | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/test/run-make-fulldeps/tools.mk b/src/test/run-make-fulldeps/tools.mk index 29ca30a7cd8..83f640e2f40 100644 --- a/src/test/run-make-fulldeps/tools.mk +++ b/src/test/run-make-fulldeps/tools.mk @@ -21,11 +21,18 @@ CGREP := "$(S)/src/etc/cat-and-grep.sh" # diff with common flags for multi-platform diffs against text output DIFF := diff -u --strip-trailing-cr -# CI platforms use `/bin/dash`. When compiling in other environments, the -# default may be different (for example, may default to `/bin/bash`), and syntax -# and results could be different. Ensure Makefile `$(shell ...)` invocations -# always run in `dash`. +# Some of the Rust CI platforms use `/bin/dash` to run `shell` script in +# Makefiles. Other platforms, including many developer platforms, default to +# `/bin/bash`. (In many cases, `make` is actually using `/bin/sh`, but `sh` +# is configured to execute one or the other shell binary). `dash` features +# support only a small subset of `bash` features, so `dash` can be thought of as +# the lowest common denominator, and tests should be validated against `dash` +# whenever possible. Most developer platforms include `/bin/dash`, but to ensure +# tests still work when `/bin/dash`, if not available, this `SHELL` override is +# conditional: +ifneq (,$(wildcard /bin/dash)) SHELL := /bin/dash +endif # This is the name of the binary we will generate and run; use this # e.g. for `$(CC) -o $(RUN_BINFILE)`. From b211acf68358fc109d1ee36fa4a22f3e017dec91 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Thu, 11 Feb 2021 14:11:08 -0800 Subject: [PATCH 3/4] Re-blessed the partial_eq.rs coverage test --- .../expected_show_coverage.partial_eq.txt | 21 +++---------------- .../run-make-fulldeps/coverage/partial_eq.rs | 19 ++--------------- 2 files changed, 5 insertions(+), 35 deletions(-) diff --git a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt index 0fe124f12d9..9cc9a4d47ad 100644 --- a/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt +++ b/src/test/run-make-fulldeps/coverage-reports/expected_show_coverage.partial_eq.txt @@ -2,11 +2,11 @@ 2| |// structure of this test. 3| | 4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] - ^0 ^0 ^0 ^0 ^1 ^0 ^0^0 + ^0 ^0 ^0 ^0 ^1 ^1 ^0^0 5| |pub struct Version { 6| | major: usize, - 7| 1| minor: usize, // Count: 1 - `PartialOrd` compared `minor` values in 3.2.1 vs. 3.3.0 - 8| 0| patch: usize, // Count: 0 - `PartialOrd` was determined by `minor` (2 < 3) + 7| | minor: usize, + 8| | patch: usize, 9| |} 10| | 11| |impl Version { @@ -45,19 +45,4 @@ 44| |`function_source_hash` without a code region, if necessary. 45| | 46| |*/ - 47| | - 48| |// FIXME(#79626): The derived traits get coverage, which is great, but some of the traits appear - 49| |// to get two coverage execution counts at different positions: - 50| |// - 51| |// ```text - 52| |// 4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] - 53| |// ^0 ^0 ^0 ^0 ^1 ^0 ^0^0 - 54| |// ```text - 55| |// - 56| |// `PartialEq`, `PartialOrd`, and `Ord` (and possibly `Eq`, if the trait name was longer than 2 - 57| |// characters) have counts at their first and last characters. - 58| |// - 59| |// Why is this? Why does `PartialOrd` have two values (1 and 0)? This must mean we are checking - 60| |// distinct coverages, so maybe we don't want to eliminate one of them. Should we merge them? - 61| |// If merged, do we lose some information? diff --git a/src/test/run-make-fulldeps/coverage/partial_eq.rs b/src/test/run-make-fulldeps/coverage/partial_eq.rs index 7d265ba66a4..4ceaba9b111 100644 --- a/src/test/run-make-fulldeps/coverage/partial_eq.rs +++ b/src/test/run-make-fulldeps/coverage/partial_eq.rs @@ -4,8 +4,8 @@ #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] pub struct Version { major: usize, - minor: usize, // Count: 1 - `PartialOrd` compared `minor` values in 3.2.1 vs. 3.3.0 - patch: usize, // Count: 0 - `PartialOrd` was determined by `minor` (2 < 3) + minor: usize, + patch: usize, } impl Version { @@ -44,18 +44,3 @@ one expression, which is allowed, but the `function_source_hash` was only passed `function_source_hash` without a code region, if necessary. */ - -// FIXME(#79626): The derived traits get coverage, which is great, but some of the traits appear -// to get two coverage execution counts at different positions: -// -// ```text -// 4| 2|#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)] -// ^0 ^0 ^0 ^0 ^1 ^0 ^0^0 -// ```text -// -// `PartialEq`, `PartialOrd`, and `Ord` (and possibly `Eq`, if the trait name was longer than 2 -// characters) have counts at their first and last characters. -// -// Why is this? Why does `PartialOrd` have two values (1 and 0)? This must mean we are checking -// distinct coverages, so maybe we don't want to eliminate one of them. Should we merge them? -// If merged, do we lose some information? From cbe6c70c687c97a0189cc2ccf2d3071f4ed01174 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Sat, 13 Feb 2021 00:14:50 -0800 Subject: [PATCH 4/4] Disabled SHELL=dash on Windows due to invalid path backslash handling Calling an executable via full path with Windows backslashes fails in dash. --- src/test/run-make-fulldeps/tools.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/run-make-fulldeps/tools.mk b/src/test/run-make-fulldeps/tools.mk index 83f640e2f40..a1a076dd747 100644 --- a/src/test/run-make-fulldeps/tools.mk +++ b/src/test/run-make-fulldeps/tools.mk @@ -30,9 +30,11 @@ DIFF := diff -u --strip-trailing-cr # whenever possible. Most developer platforms include `/bin/dash`, but to ensure # tests still work when `/bin/dash`, if not available, this `SHELL` override is # conditional: +ifndef IS_WINDOWS # dash interprets backslashes in executable paths incorrectly ifneq (,$(wildcard /bin/dash)) SHELL := /bin/dash endif +endif # This is the name of the binary we will generate and run; use this # e.g. for `$(CC) -o $(RUN_BINFILE)`.