From 3d740cfb385aa0750bf02f295479f0f46660e14e Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu, 29 Nov 2018 15:29:34 -0800 Subject: [PATCH 1/4] move platform-specific testing to its own subdirectory of python tasks for 1:1:1 --- .../pants_test/backend/python/tasks/BUILD | 106 +----------------- .../tasks/platform_specific_behavior/BUILD | 30 +++++ .../test_build_local_python_distributions.py | 0 .../test_ctypes.py | 0 .../test_ctypes_integration.py | 0 .../test_python_distribution_integration.py | 0 6 files changed, 32 insertions(+), 104 deletions(-) create mode 100644 tests/python/pants_test/backend/python/tasks/platform_specific_behavior/BUILD rename tests/python/pants_test/backend/python/tasks/{ => platform_specific_behavior}/test_build_local_python_distributions.py (100%) rename tests/python/pants_test/backend/python/tasks/{ => platform_specific_behavior}/test_ctypes.py (100%) rename tests/python/pants_test/backend/python/tasks/{ => platform_specific_behavior}/test_ctypes_integration.py (100%) rename tests/python/pants_test/backend/python/tasks/{ => platform_specific_behavior}/test_python_distribution_integration.py (100%) diff --git a/tests/python/pants_test/backend/python/tasks/BUILD b/tests/python/pants_test/backend/python/tasks/BUILD index b41b47b4169..5c34fa61e49 100644 --- a/tests/python/pants_test/backend/python/tasks/BUILD +++ b/tests/python/pants_test/backend/python/tasks/BUILD @@ -21,85 +21,8 @@ python_library( ] ) -python_native_code_test_files = [ - 'test_build_local_python_distributions.py', - 'test_python_distribution_integration.py', - 'test_ctypes.py', - 'test_ctypes_integration.py', -] - -python_tests( - name='python_native_code_testing_3', - sources=[ - python_native_code_test_files[0], - ], - dependencies=[ - '3rdparty/python:future', - '3rdparty/python/twitter/commons:twitter.common.collections', - 'src/python/pants/backend/native', - 'src/python/pants/backend/native/targets', - 'src/python/pants/backend/python:plugin', - 'src/python/pants/backend/python/targets', - 'src/python/pants/backend/python/tasks', - 'src/python/pants/util:contextutil', - 'src/python/pants/util:process_handler', - 'tests/python/pants_test:int-test', - 'tests/python/pants_test/backend/python/tasks/util', - 'tests/python/pants_test/engine:scheduler_test_base', - 'tests/python/pants_test/testutils:py2_compat', - ], - tags={'platform_specific_behavior', 'integration'}, - timeout=2400, -) - -python_tests( - name='python_native_code_testing_2', - sources=[ - python_native_code_test_files[1], - ], - dependencies=[ - '3rdparty/python:future', - '3rdparty/python/twitter/commons:twitter.common.collections', - 'src/python/pants/backend/native', - 'src/python/pants/backend/native/targets', - 'src/python/pants/backend/python:plugin', - 'src/python/pants/backend/python/targets', - 'src/python/pants/backend/python/tasks', - 'src/python/pants/util:contextutil', - 'src/python/pants/util:process_handler', - 'tests/python/pants_test:int-test', - 'tests/python/pants_test/backend/python/tasks/util', - 'tests/python/pants_test/engine:scheduler_test_base', - 'tests/python/pants_test/testutils:py2_compat', - ], - tags={'platform_specific_behavior', 'integration'}, - timeout=2400, -) - python_tests( - name='python_native_code_testing_1', - sources=python_native_code_test_files[2:], - dependencies=[ - '3rdparty/python:future', - '3rdparty/python/twitter/commons:twitter.common.collections', - 'src/python/pants/backend/native', - 'src/python/pants/backend/native/targets', - 'src/python/pants/backend/python:plugin', - 'src/python/pants/backend/python/targets', - 'src/python/pants/backend/python/tasks', - 'src/python/pants/util:contextutil', - 'src/python/pants/util:process_handler', - 'tests/python/pants_test:int-test', - 'tests/python/pants_test/backend/python/tasks/util', - 'tests/python/pants_test/engine:scheduler_test_base', - 'tests/python/pants_test/testutils:py2_compat', - ], - tags={'platform_specific_behavior', 'integration'}, - timeout=2400, -) - -python_tests( - sources=globs('test_*.py', exclude=([globs('*_integration.py')] + python_native_code_test_files)), + sources=globs('test_*.py', exclude=[globs('*_integration.py')]), dependencies=[ '3rdparty/python/twitter/commons:twitter.common.collections', '3rdparty/python/twitter/commons:twitter.common.dirutil', @@ -137,7 +60,7 @@ python_tests( python_tests( name='integration', - sources=globs('*_integration.py', exclude=python_native_code_test_files), + sources=globs('*_integration.py'), dependencies=[ '3rdparty/python:pex', 'src/python/pants/base:build_environment', @@ -152,28 +75,3 @@ python_tests( tags={'integration'}, timeout=2400 ) - -python_tests( - name='isort_run_integration', - sources=['test_isort_run_integration.py'], - dependencies=[ - 'tests/python/pants_test:int-test', - 'tests/python/pants_test/backend/python/tasks:python_task_test_base', - ], - tags={'integration'}, -) - -python_tests( - name='isort_run', - sources=['test_isort_run.py'], - dependencies=[ - '3rdparty/python:future', - 'src/python/pants/util:dirutil', - 'tests/python/pants_test/backend/python/tasks:python_task_test_base', - 'src/python/pants/util:process_handler', - 'tests/python/pants_test:test_base', - 'tests/python/pants_test/subsystem:subsystem_utils', - 'tests/python/pants_test:task_test_base', - ], - timeout=300 -) diff --git a/tests/python/pants_test/backend/python/tasks/platform_specific_behavior/BUILD b/tests/python/pants_test/backend/python/tasks/platform_specific_behavior/BUILD new file mode 100644 index 00000000000..df1edc8d1d9 --- /dev/null +++ b/tests/python/pants_test/backend/python/tasks/platform_specific_behavior/BUILD @@ -0,0 +1,30 @@ +python_tests( + sources=globs('test_*.py', exclude=[globs('*_integration.py')]), + dependencies = [ + '3rdparty/python:future', + '3rdparty/python/twitter/commons:twitter.common.collections', + 'src/python/pants/backend/native/targets', + 'src/python/pants/backend/python/targets', + 'src/python/pants/util:contextutil', + 'tests/python/pants_test/backend/python/tasks/util', + ], + tags={'platform_specific_behavior'}, +) + +python_tests( + name='integration', + sources=globs('*_integration.py'), + dependencies=[ + '3rdparty/python:future', + 'src/python/pants/backend/native/config', + 'src/python/pants/util:collections', + 'src/python/pants/util:contextutil', + 'src/python/pants/util:dirutil', + 'src/python/pants/util:process_handler', + 'tests/python/pants_test:int-test', + 'tests/python/pants_test/backend/python/tasks:python_task_test_base', + 'tests/python/pants_test/testutils:py2_compat', + ], + tags={'platform_specific_behavior', 'integration'}, + timeout=2400, +) diff --git a/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py b/tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_build_local_python_distributions.py similarity index 100% rename from tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py rename to tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_build_local_python_distributions.py diff --git a/tests/python/pants_test/backend/python/tasks/test_ctypes.py b/tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_ctypes.py similarity index 100% rename from tests/python/pants_test/backend/python/tasks/test_ctypes.py rename to tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_ctypes.py diff --git a/tests/python/pants_test/backend/python/tasks/test_ctypes_integration.py b/tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_ctypes_integration.py similarity index 100% rename from tests/python/pants_test/backend/python/tasks/test_ctypes_integration.py rename to tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_ctypes_integration.py diff --git a/tests/python/pants_test/backend/python/tasks/test_python_distribution_integration.py b/tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_python_distribution_integration.py similarity index 100% rename from tests/python/pants_test/backend/python/tasks/test_python_distribution_integration.py rename to tests/python/pants_test/backend/python/tasks/platform_specific_behavior/test_python_distribution_integration.py From ccb998f2e0cebbb4cc486da3b5a834b8e3048e52 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Thu, 29 Nov 2018 18:50:58 -0800 Subject: [PATCH 2/4] i don't think these strings ever needed to be binary at all --- .../example/python_distribution/hello/fasthello/setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/src/python/example/python_distribution/hello/fasthello/setup.py b/examples/src/python/example/python_distribution/hello/fasthello/setup.py index 99f996f3e5d..9f6f9553e2a 100644 --- a/examples/src/python/example/python_distribution/hello/fasthello/setup.py +++ b/examples/src/python/example/python_distribution/hello/fasthello/setup.py @@ -8,8 +8,8 @@ from distutils.core import Extension -c_module = Extension(b'c_greet', sources=[b'c_greet.c']) -cpp_module = Extension(b'cpp_greet', sources=[b'cpp_greet.cpp']) +c_module = Extension('c_greet', sources=['c_greet.c']) +cpp_module = Extension('cpp_greet', sources=['cpp_greet.cpp']) setup( name='fasthello', From c3b2a10626ce1ee2b1fd77f8b4f42abc15f45254 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Tue, 4 Dec 2018 22:02:35 -0800 Subject: [PATCH 3/4] add CC and CXX to the distutils native build environment --- .../python/subsystems/python_native_code.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/python/pants/backend/python/subsystems/python_native_code.py b/src/python/pants/backend/python/subsystems/python_native_code.py index f8becc87f04..44caa4c1d4a 100644 --- a/src/python/pants/backend/python/subsystems/python_native_code.py +++ b/src/python/pants/backend/python/subsystems/python_native_code.py @@ -209,13 +209,18 @@ def as_environment(self): c_linker.path_entries + cpp_compiler.path_entries + cpp_linker.path_entries) - # TODO(#6273): We prepend our toolchain to the PATH instead of overwriting it -- we need - # better control of the distutils compilation environment if we want to actually isolate the - # PATH (distutils does lots of sneaky things). - ret['PATH'] = create_path_env_var(all_path_entries, env=os.environ.copy(), prepend=True) - - # GCC will output smart quotes in a variety of situations (leading to decoding errors - # downstream) unless we set this environment variable. - ret['LC_ALL'] = 'C' + + ret.update({ + 'CC': c_compiler.exe_filename, + 'CXX': cpp_compiler.exe_filename, + # TODO(#6273): We prepend our toolchain to the PATH instead of overwriting it -- we need + # better control of the distutils compilation environment if we want to actually isolate the + # PATH (distutils does lots of sneaky things). + 'PATH': create_path_env_var(all_path_entries, env=os.environ.copy(), prepend=True), + # GCC will output smart quotes in a variety of situations (leading to decoding errors + # downstream because pex prints directly to stderr) unless we set this environment variable, + # so compile errors are impossible to see otherwise. + 'LC_ALL': 'C', + }) return ret From 022ed8cb677d1711a2af189192202442e12b4134 Mon Sep 17 00:00:00 2001 From: Danny McClanahan <1305167+cosmicexplorer@users.noreply.github.com> Date: Fri, 7 Dec 2018 13:13:48 -0800 Subject: [PATCH 4/4] add the c++ compiler and linker's env to distutils --- .../pants/backend/python/subsystems/python_native_code.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/python/pants/backend/python/subsystems/python_native_code.py b/src/python/pants/backend/python/subsystems/python_native_code.py index 44caa4c1d4a..431c9db00c2 100644 --- a/src/python/pants/backend/python/subsystems/python_native_code.py +++ b/src/python/pants/backend/python/subsystems/python_native_code.py @@ -210,9 +210,13 @@ def as_environment(self): cpp_compiler.path_entries + cpp_linker.path_entries) + # TODO(#6273): we are constructing an informally-composed environment for consumption by + # distutils in order to set CC, CXX, LDSHARED, LIBRARY_PATH, and (DY)LD_LIBRARY_PATH so the + # compilers will run. + ret.update(cpp_linker.as_invocation_environment_dict) + ret.update(cpp_compiler.as_invocation_environment_dict) + ret.update({ - 'CC': c_compiler.exe_filename, - 'CXX': cpp_compiler.exe_filename, # TODO(#6273): We prepend our toolchain to the PATH instead of overwriting it -- we need # better control of the distutils compilation environment if we want to actually isolate the # PATH (distutils does lots of sneaky things).