Skip to content

Commit

Permalink
Auto merge of #81734 - richkadel:fixfordash, r=pnkfelix
Browse files Browse the repository at this point in the history
Ensures `make` tests run under /bin/dash (if available), like CI, and fixes a Makefile

Note: This cherrypicks #81688 (`@pnkfelix)`

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`.

Also see: [zulip discussion thread](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/how.20to.20run.2Fbless.20src.2Ftest.2Frun-make-fulldeps.2Fcoverage.20.3F)

r? `@pnkfelix`

FYI: `@wesleywiser` `@tmandry`
  • Loading branch information
bors committed Feb 16, 2021
2 parents 42a4673 + cbe6c70 commit 090dac0
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 39 deletions.
3 changes: 1 addition & 2 deletions 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
Expand Down Expand Up @@ -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 \
Expand Down
Expand Up @@ -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 {
Expand Down Expand Up @@ -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?

2 changes: 0 additions & 2 deletions src/test/run-make-fulldeps/coverage/coverage_tools.mk
Expand Up @@ -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)
19 changes: 2 additions & 17 deletions src/test/run-make-fulldeps/coverage/partial_eq.rs
Expand Up @@ -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 {
Expand Down Expand Up @@ -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?
15 changes: 15 additions & 0 deletions src/test/run-make-fulldeps/tools.mk
Expand Up @@ -21,6 +21,21 @@ 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

# 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:
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)`.
RUN_BINFILE = $(TMPDIR)/$(1)
Expand Down

0 comments on commit 090dac0

Please sign in to comment.