microarchitectures: look in /sbin and /usr/sbin for sysctl (#13365)

This PR ensures that on Darwin we always append /sbin and /usr/sbin to PATH, if they are not already present, when looking for sysctl.

* Make sure we look into /sbin and /usr/sbin for sysctl
* Refactor sysctl for better readability
* Remove marker to make test pass

3 files changed, 30 insertions, 22 deletions

diff --git a/lib/spack/llnl/util/cpu/detect.py b/lib/spack/llnl/util/cpu/detect.py
index 7744d5acd2..92b75e9653 100644
--- a/lib/spack/llnl/util/cpu/detect.py
+++ b/lib/spack/llnl/util/cpu/detect.py
@@ -4,6 +4,7 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 import collections
 import functools
+import os
 import platform
 import re
 import subprocess
@@ -75,31 +76,38 @@ def proc_cpuinfo():
     return info
 
 
-def check_output(args):
-    output = subprocess.Popen(args, stdout=subprocess.PIPE).communicate()[0]
+def check_output(args, env):
+    output = subprocess.Popen(
+        args, stdout=subprocess.PIPE, env=env
+    ).communicate()[0]
     return six.text_type(output.decode('utf-8'))
 
 
 @info_dict(operating_system='Darwin')
-def sysctl():
+def sysctl_info_dict():
     """Returns a raw info dictionary parsing the output of sysctl."""
-
-    info = {}
-    info['vendor_id'] = check_output(
-        ['sysctl', '-n', 'machdep.cpu.vendor']
-    ).strip()
-    info['flags'] = check_output(
-        ['sysctl', '-n', 'machdep.cpu.features']
-    ).strip().lower()
-    info['flags'] += ' ' + check_output(
-        ['sysctl', '-n', 'machdep.cpu.leaf7_features']
-    ).strip().lower()
-    info['model'] = check_output(
-        ['sysctl', '-n', 'machdep.cpu.model']
-    ).strip()
-    info['model name'] = check_output(
-        ['sysctl', '-n', 'machdep.cpu.brand_string']
-    ).strip()
+    # Make sure that /sbin and /usr/sbin are in PATH as sysctl is
+    # usually found there
+    child_environment = dict(os.environ.items())
+    search_paths = child_environment.get('PATH', '').split(os.pathsep)
+    for additional_path in ('/sbin', '/usr/sbin'):
+        if additional_path not in search_paths:
+            search_paths.append(additional_path)
+    child_environment['PATH'] = os.pathsep.join(search_paths)
+
+    def sysctl(*args):
+        return check_output(
+            ['sysctl'] + list(args), env=child_environment
+        ).strip()
+
+    flags = (sysctl('-n', 'machdep.cpu.features').lower() + ' '
+             + sysctl('-n', 'machdep.cpu.leaf7_features').lower())
+    info = {
+        'vendor_id': sysctl('-n', 'machdep.cpu.vendor'),
+        'flags': flags,
+        'model': sysctl('-n', 'machdep.cpu.model'),
+        'model name': sysctl('-n', 'machdep.cpu.brand_string')
+    }
 
     # Super hacky way to deal with slight representation differences
     # Would be better to somehow consider these "identical"
diff --git a/lib/spack/spack/test/cmd/compiler_command.py b/lib/spack/spack/test/cmd/compiler_command.py
index e40e49544d..a4ceb63822 100644
--- a/lib/spack/spack/test/cmd/compiler_command.py
+++ b/lib/spack/spack/test/cmd/compiler_command.py
@@ -24,7 +24,7 @@ def no_compilers_yaml(mutable_config, monkeypatch):
             os.remove(compilers_yaml)
 
 
-@pytest.mark.regression('11678')
+@pytest.mark.regression('11678,13138')
 def test_compiler_find_without_paths(no_compilers_yaml, working_env, tmpdir):
     with tmpdir.as_cwd():
         with open('gcc', 'w') as f:
diff --git a/lib/spack/spack/test/llnl/util/cpu.py b/lib/spack/spack/test/llnl/util/cpu.py
index fe1209c8c8..2e03772d19 100644
--- a/lib/spack/spack/test/llnl/util/cpu.py
+++ b/lib/spack/spack/test/llnl/util/cpu.py
@@ -67,7 +67,7 @@ def expected_target(request, monkeypatch):
                 key, value = line.split(':')
                 info[key.strip()] = value.strip()
 
-        def _check_output(args):
+        def _check_output(args, env):
             current_key = args[-1]
             return info[current_key]