Ensures make tests run under /bin/dash (if available), like CI, and fixes a Makefile

[richkadel]

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

r? @pnkfelix

FYI: @wesleywiser @tmandry

[Mark-Simulacrum]

This seems like it would break on Windows? Maybe it's somehow magically accessible though.

[richkadel]

My Windows VM is powered down, so I asked in the zulip thread if anyone else could confirm if /bin/dash is there.

But in the mean time, I'm bringing Windows back up, so I will check.

[richkadel]

@Mark-Simulacrum - I can provide some confidence this will work on Windows in a Rust-configured development environment:

I have a pretty vanilla setup of Windows, set up specifically to test rustc and llvm, using the recommended steps on the rust-lang/rust README.md, and my msys2 setup has /bin/dash. I didn't install it specifically.

[bors]

☔ The latest upstream changes (presumably #81688) made this pull request unmergeable. Please resolve the merge conflicts.

[richkadel]

@pnkfelix - I checked this locally and it works as intended. If /bin/dash is not present, it does not change SHELL at all. If /bin/dash is present, it sets SHELL := /bin/dash, and scripts that are bash compliant but not dash compliant predictably fail.

[pnkfelix]

Lets give this a whirl

[pnkfelix]

@bors r+

[bors]

📌 Commit ca3a714 has been approved by pnkfelix

[JohnTitor]

Failed in rollup: #81983 (comment)
@bors r-

[richkadel]

It looks like, since the coverage tests were recently disabled, some other change came in that SHOULD have forced re-blessing the coverage results, but since the test didn't run, it didn't fail, and coverage results were not updated.

I'll rebase and hopefully get that change, re-bless the output, and if the problem is resolved, hopefully we can get this merged quickly, to avoid this happening again.

Interestingly, whatever change it was seems to (at least partially) clean up a coverage discrepancy, mentioned at the bottom of the test program, with FIXME issue #79626. (I'm still not sure why there are two code regions for derived traits, but at least they are no longer inconsistent.)

But (whether from the same change or an unrelated change), coverage stats no longer appear on the struct fields. I didn't actually expect coverage stats for fields until we created this test, and now I'm not sure if it's appropriate to lose this coverage, or not. But I do think it's acceptable.

I'll need to change the comments in the test source to reflect these new results (assuming I can reproduce them):

--- expected_show_coverage.partial_eq.txt	2021-02-11 03:54:17.271247325 +0000
+++ /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/coverage-reports/coverage-reports/actual_show_coverage.partial_eq.txt	2021-02-11 04:39:04.423096295 +0000
@@ -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, // Count: 1 - `PartialOrd` compared `minor` values in 3.2.1 vs. 3.3.0
+    8|       |    patch: usize, // Count: 0 - `PartialOrd` was determined by `minor` (2 < 3)
     9|       |}
    10|       |
    11|       |impl Version {

[richkadel]

@pnkfelix I re-blessed the tests. I'm not sure what PR introduced the change that resulted in different (and fortunately, better) coverage counts for derived traits, but it was very recent, because my original commits were tested on the latest baseline at that time, and worked then, but don't work now, without today's update.

This is ready to re-submit to the bors queue.

Thanks!
Rich

[richkadel]

r? @pnkfelix

[bors]

📌 Commit cbe6c70 has been approved by pnkfelix

[bors]

⌛ Testing commit cbe6c70 with merge 86d04a71a593d732fab156d5eb95e83664f16e0c...

[bors]

💥 Test timed out

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[richkadel]

Spurious?

test src/unit.rs - unit::() (line 8) ... ok
test src/num/mod.rs - num::i128::to_be_bytes (line 125) ... test src/num/mod.rs - num::i128::to_be_bytes (line 125) has been running for over 60 seconds
test src/num/mod.rs - num::i128::wrapping_rem_euclid (line 132) ... test src/num/mod.rs - num::i128::wrapping_rem_euclid (line 132) has been running for over 60 seconds
Error: The operation was canceled.

[richkadel]

Note all CI platforms succeeded but one, which timed out

[Mark-Simulacrum]

@bors retry - spurious timeout

[bors]

⌛ Testing commit cbe6c70 with merge 916508b3169603a8f0ced72d63fbf9a4288a91e3...

[bors]

💥 Test timed out

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[richkadel]

auto (i686-gnu-nopt, ubuntu-latest-xl):

test [ui] ui/zero-sized/zero-sized-binary-heap-push.rs ... ok
test [ui] ui/zero-sized/zero-sized-linkedlist-push.rs ... ok
test [ui] ui/zero-sized/zero-sized-btreemap-insert.rs ... ok
test [ui] ui/wrong-hashset-issue-42918.rs ... ok

== clock drift check ==
  local time: Mon Feb 15 07:27:19 UTC 2021
Session terminated, terminating shell.../checkout/src/ci/run.sh: line 117:  1864 Terminated              sh -x -c "$SCRIPT"
  network time: Sun, 14 Feb 2021 18:48:58 GMT
== end clock drift check ==
 ...terminated.
Error: The operation was canceled.

Another spurious timeout?

Same builder as last time?

Every other builder completes successfully.

[richkadel]

Yes same builder in both cases

[pnkfelix]

@bors retry - spurious timeout

[bors]

⌛ Testing commit cbe6c70 with merge 090dac0...

[bors]

☀️ Test successful - checks-actions
Approved by: pnkfelix
Pushing 090dac0 to master...