[lit/libcxx] Fix a remaining reference to lit.util.capture() in custom libcxx/Darwin code.

[dlj]

Summary

This reference to lit.util.capture is functionally identical to
subprocess.check_output, so this change switches to call the library routine
directly.

[mzolotukhin]

Looks good to me (see one comment below)!

Thanks,
Michael

utils/libcxx/test/target_info.py

1

Undo this sneaked in change please.

[dlj]

Typop

[dlj]

Really update?

[dlj]

One more time, with feeling. (LOL filesystems)

utils/libcxx/test/target_info.py

1

Done.

[mzolotukhin]

There are still more uses of lit.util.capture. I fixed one of them in r307201 (it broke PGO builds), but there are still some remaining:

./test/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./test/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()
./test/Unit/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./test/Unit/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()
./tools/clang/test/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./tools/clang/test/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()
./tools/clang/test/Unit/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./tools/clang/test/Unit/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()

I wonder why they didn't cause any problems (or why we didn't find the problems). Do we need to change them the same way?

Michael

[dlj]

In D34841#800110, @mzolotukhin wrote:

There are still more uses of lit.util.capture. I fixed one of them in r307201 (it broke PGO builds), but there are still some remaining:

D'oh

/shamecube

./test/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./test/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()
./test/Unit/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./test/Unit/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()
./tools/clang/test/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./tools/clang/test/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()
./tools/clang/test/Unit/lit.cfg:    llvm_src_root = lit.util.capture(['llvm-config', '--src-root']).strip()
./tools/clang/test/Unit/lit.cfg:    llvm_obj_root = lit.util.capture(['llvm-config', '--obj-root']).strip()

I wonder why they didn't cause any problems (or why we didn't find the problems).

So by my reading of those local config files, the calls are in the context of out-of-tree builds or other erroneous cases:

# Otherwise, we haven't loaded the site specific configuration (the user is
# probably trying to run on a test file directly, and either the site
# configuration hasn't been created by the build system, or we are in an
# out-of-tree build situation).

All of my builds are out-of-tree (and CMake generates the files correctly), so I'm guessing the cases you found are some other configuration that triggers the same branch. I'm not sure how that would happen, but I'm pretty confident that it can.

Do we need to change them the same way?

I think that should work. (The Py2/Py3 differences shouldn't matter with llvm-config.) I'm not sure I will be able to test the fix, though, unless I can figure out how to repro.

Do you have any hints for how I might be able to tickle the errors you found?

[mzolotukhin]

Do you have any hints for how I might be able to tickle the errors you found?

I found this issue in our regular 2 stage PGO build (generate-profraw target from tools/clang/utils/perf-training/CMakeLists.txt invokes LIT tests). I think we should just replace all appearances of lit.util.capture with subprocess.check_output - at least, it shouldn't make things worse.

Michael

[dlj]

In D34841#801351, @mzolotukhin wrote:

Do you have any hints for how I might be able to tickle the errors you found?

I found this issue in our regular 2 stage PGO build (generate-profraw target from tools/clang/utils/perf-training/CMakeLists.txt invokes LIT tests). I think we should just replace all appearances of lit.util.capture with subprocess.check_output - at least, it shouldn't make things worse.

Michael

Works for me. Fixed in r307320.

utils/libcxx/test/target_info.py

 #===----------------------------------------------------------------------===//
 #
 #                     The LLVM Compiler Infrastructure
 #
 # This file is dual licensed under the MIT and the University of Illinois Open
 # Source Licenses. See LICENSE.TXT for details.
 #
 #===----------------------------------------------------------------------===//
 
 import importlib
-import lit.util  # pylint: disable=import-error,no-name-in-module
 import locale
 import os
 import platform
 import re
+import subprocess
 import sys
 
 class DefaultTargetInfo(object):
     def __init__(self, full_config):
         self.full_config = full_config
 
     def platform(self):
         return sys.platform.lower().strip()
for loc_id, windows_loc_name in locales:
                                'unsupported.'.format(loc_name))
 
 
 class DarwinLocalTI(DefaultTargetInfo):
     def __init__(self, full_config):
         super(DarwinLocalTI, self).__init__(full_config)
 
     def is_host_macosx(self):
-        name = lit.util.capture(['sw_vers', '-productName']).strip()
+        name = subprocess.check_output(['sw_vers', '-productName']).strip()
         return name == "Mac OS X"
 
     def get_macosx_version(self):
         assert self.is_host_macosx()
-        version = lit.util.capture(['sw_vers', '-productVersion']).strip()
+        version = subprocess.check_output(
+            ['sw_vers', '-productVersion']).strip()
         version = re.sub(r'([0-9]+\.[0-9]+)(\..*)?', r'\1', version)
         return version
 
     def get_sdk_version(self, name):
         assert self.is_host_macosx()
         cmd = ['xcrun', '--sdk', name, '--show-sdk-path']
         try:
-            out = lit.util.capture(cmd).strip()
+            out = subprocess.check_output(cmd).strip()
         except OSError:
             pass
 
         if not out:
             self.full_config.lit_config.fatal(
                     "cannot infer sdk version with: %r" % cmd)
 
         return re.sub(r'.*/[^0-9]+([0-9.]+)\.sdk', r'\1', out)
class DarwinLocalTI(DefaultTargetInfo):
 
     def add_cxx_compile_flags(self, flags):
         if self.full_config.use_deployment:
             _, name, _ = self.full_config.config.deployment
             cmd = ['xcrun', '--sdk', name, '--show-sdk-path']
         else:
             cmd = ['xcrun', '--show-sdk-path']
         try:
-            out = lit.util.capture(cmd).strip()
+            out = subprocess.check_output(cmd).strip()
             res = 0
         except OSError:
             res = -1
         if res == 0 and out:
             sdk_path = out
             self.full_config.lit_config.note('using SDKROOT: %r' % sdk_path)
             flags += ["-isysroot", sdk_path]