[ 
https://issues.apache.org/jira/browse/ARROW-2070?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16347650#comment-16347650
 ] 

ASF GitHub Bot commented on ARROW-2070:
---------------------------------------

wesm closed pull request #1540: ARROW-2070: [Python] Fix chdir logic in setup.py
URL: https://github.com/apache/arrow/pull/1540
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/python/setup.py b/python/setup.py
index 076d7e489..cfc771fe8 100644
--- a/python/setup.py
+++ b/python/setup.py
@@ -17,6 +17,7 @@
 # specific language governing permissions and limitations
 # under the License.
 
+import contextlib
 import glob
 import os
 import os.path as osp
@@ -47,6 +48,16 @@
 setup_dir = os.path.abspath(os.path.dirname(__file__))
 
 
[email protected]
+def changed_dir(dirname):
+    oldcwd = os.getcwd()
+    os.chdir(dirname)
+    try:
+        yield
+    finally:
+        os.chdir(oldcwd)
+
+
 class clean(_clean):
 
     def run(self):
@@ -59,6 +70,7 @@ def run(self):
 
 
 class build_ext(_build_ext):
+    _found_names = ()
 
     def build_extensions(self):
         numpy_incl = pkg_resources.resource_filename('numpy', 'core/include')
@@ -129,162 +141,160 @@ def _run_cmake(self):
         # The staging directory for the module being built
         build_temp = pjoin(os.getcwd(), self.build_temp)
         build_lib = os.path.join(os.getcwd(), self.build_lib)
-
-        # Change to the build directory
         saved_cwd = os.getcwd()
+
         if not os.path.isdir(self.build_temp):
             self.mkpath(self.build_temp)
-        os.chdir(self.build_temp)
-
-        # Detect if we built elsewhere
-        if os.path.isfile('CMakeCache.txt'):
-            cachefile = open('CMakeCache.txt', 'r')
-            cachedir = re.search('CMAKE_CACHEFILE_DIR:INTERNAL=(.*)',
-                                 cachefile.read()).group(1)
-            cachefile.close()
-            if (cachedir != build_temp):
-                return
-
-        static_lib_option = ''
-
-        cmake_options = [
-            '-DPYTHON_EXECUTABLE=%s' % sys.executable,
-            static_lib_option,
-        ]
 
-        if self.with_parquet:
-            cmake_options.append('-DPYARROW_BUILD_PARQUET=on')
-        if self.with_static_parquet:
-            cmake_options.append('-DPYARROW_PARQUET_USE_SHARED=off')
-        if not self.with_static_boost:
-            cmake_options.append('-DPYARROW_BOOST_USE_SHARED=on')
-
-        if self.with_plasma:
-            cmake_options.append('-DPYARROW_BUILD_PLASMA=on')
-
-        if self.with_orc:
-            cmake_options.append('-DPYARROW_BUILD_ORC=on')
-
-        if len(self.cmake_cxxflags) > 0:
-            cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
-                                 .format(self.cmake_cxxflags))
-
-        if self.bundle_arrow_cpp:
-            cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
-            # ARROW-1090: work around CMake rough edges
-            if 'ARROW_HOME' in os.environ and sys.platform != 'win32':
-                pkg_config = pjoin(os.environ['ARROW_HOME'], 'lib',
-                                   'pkgconfig')
-                os.environ['PKG_CONFIG_PATH'] = pkg_config
-                del os.environ['ARROW_HOME']
-
-        cmake_options.append('-DCMAKE_BUILD_TYPE={0}'
-                             .format(self.build_type.lower()))
-
-        extra_cmake_args = shlex.split(self.extra_cmake_args)
-        if sys.platform != 'win32':
-            cmake_command = (['cmake'] + extra_cmake_args +
-                             cmake_options + [source])
-
-            print("-- Runnning cmake for pyarrow")
-            self.spawn(cmake_command)
-            print("-- Finished cmake for pyarrow")
-            args = ['make']
-            if os.environ.get('PYARROW_BUILD_VERBOSE', '0') == '1':
-                args.append('VERBOSE=1')
-
-            if 'PYARROW_PARALLEL' in os.environ:
-                args.append('-j{0}'.format(os.environ['PYARROW_PARALLEL']))
-            print("-- Running cmake --build for pyarrow")
-            self.spawn(args)
-            print("-- Finished cmake --build for pyarrow")
-        else:
-            cmake_generator = 'Visual Studio 14 2015 Win64'
-            if not is_64_bit:
-                raise RuntimeError('Not supported on 32-bit Windows')
-
-            # Generate the build files
-            cmake_command = (['cmake'] + extra_cmake_args +
-                             cmake_options +
-                             [source, '-G', cmake_generator])
-            if "-G" in self.extra_cmake_args:
-                cmake_command = cmake_command[:-2]
-
-            print("-- Runnning cmake for pyarrow")
-            self.spawn(cmake_command)
-            print("-- Finished cmake for pyarrow")
-            # Do the build
-            print("-- Running cmake --build for pyarrow")
-            self.spawn(['cmake', '--build', '.', '--config', self.build_type])
-            print("-- Finished cmake --build for pyarrow")
-
-        if self.inplace:
-            # a bit hacky
-            build_lib = saved_cwd
-
-        # Move the libraries to the place expected by the Python
-        # build
-
-        try:
-            os.makedirs(pjoin(build_lib, 'pyarrow'))
-        except OSError:
-            pass
+        # Change to the build directory
+        with changed_dir(self.build_temp):
+            # Detect if we built elsewhere
+            if os.path.isfile('CMakeCache.txt'):
+                cachefile = open('CMakeCache.txt', 'r')
+                cachedir = re.search('CMAKE_CACHEFILE_DIR:INTERNAL=(.*)',
+                                     cachefile.read()).group(1)
+                cachefile.close()
+                if (cachedir != build_temp):
+                    return
+
+            static_lib_option = ''
+
+            cmake_options = [
+                '-DPYTHON_EXECUTABLE=%s' % sys.executable,
+                static_lib_option,
+            ]
+
+            if self.with_parquet:
+                cmake_options.append('-DPYARROW_BUILD_PARQUET=on')
+            if self.with_static_parquet:
+                cmake_options.append('-DPYARROW_PARQUET_USE_SHARED=off')
+            if not self.with_static_boost:
+                cmake_options.append('-DPYARROW_BOOST_USE_SHARED=on')
 
-        if sys.platform == 'win32':
-            build_prefix = ''
-        else:
-            build_prefix = self.build_type
+            if self.with_plasma:
+                cmake_options.append('-DPYARROW_BUILD_PLASMA=on')
+
+            if self.with_orc:
+                cmake_options.append('-DPYARROW_BUILD_ORC=on')
+
+            if len(self.cmake_cxxflags) > 0:
+                cmake_options.append('-DPYARROW_CXXFLAGS="{0}"'
+                                     .format(self.cmake_cxxflags))
+
+            if self.bundle_arrow_cpp:
+                cmake_options.append('-DPYARROW_BUNDLE_ARROW_CPP=ON')
+                # ARROW-1090: work around CMake rough edges
+                if 'ARROW_HOME' in os.environ and sys.platform != 'win32':
+                    pkg_config = pjoin(os.environ['ARROW_HOME'], 'lib',
+                                       'pkgconfig')
+                    os.environ['PKG_CONFIG_PATH'] = pkg_config
+                    del os.environ['ARROW_HOME']
+
+            cmake_options.append('-DCMAKE_BUILD_TYPE={0}'
+                                 .format(self.build_type.lower()))
+
+            extra_cmake_args = shlex.split(self.extra_cmake_args)
+            if sys.platform != 'win32':
+                cmake_command = (['cmake'] + extra_cmake_args +
+                                 cmake_options + [source])
+
+                print("-- Runnning cmake for pyarrow")
+                self.spawn(cmake_command)
+                print("-- Finished cmake for pyarrow")
+                args = ['make']
+                if os.environ.get('PYARROW_BUILD_VERBOSE', '0') == '1':
+                    args.append('VERBOSE=1')
+
+                if 'PYARROW_PARALLEL' in os.environ:
+                    args.append('-j{0}'.format(os.environ['PYARROW_PARALLEL']))
+                print("-- Running cmake --build for pyarrow")
+                self.spawn(args)
+                print("-- Finished cmake --build for pyarrow")
+            else:
+                cmake_generator = 'Visual Studio 14 2015 Win64'
+                if not is_64_bit:
+                    raise RuntimeError('Not supported on 32-bit Windows')
+
+                # Generate the build files
+                cmake_command = (['cmake'] + extra_cmake_args +
+                                 cmake_options +
+                                 [source, '-G', cmake_generator])
+                if "-G" in self.extra_cmake_args:
+                    cmake_command = cmake_command[:-2]
+
+                print("-- Runnning cmake for pyarrow")
+                self.spawn(cmake_command)
+                print("-- Finished cmake for pyarrow")
+                # Do the build
+                print("-- Running cmake --build for pyarrow")
+                self.spawn(['cmake', '--build', '.', '--config', 
self.build_type])
+                print("-- Finished cmake --build for pyarrow")
+
+            if self.inplace:
+                # a bit hacky
+                build_lib = saved_cwd
+
+            # Move the libraries to the place expected by the Python
+            # build
+
+            try:
+                os.makedirs(pjoin(build_lib, 'pyarrow'))
+            except OSError:
+                pass
 
-        if self.bundle_arrow_cpp:
-            print(pjoin(build_lib, 'pyarrow'))
-            move_shared_libs(build_prefix, build_lib, "arrow")
-            move_shared_libs(build_prefix, build_lib, "arrow_python")
+            if sys.platform == 'win32':
+                build_prefix = ''
+            else:
+                build_prefix = self.build_type
+
+            if self.bundle_arrow_cpp:
+                print(pjoin(build_lib, 'pyarrow'))
+                move_shared_libs(build_prefix, build_lib, "arrow")
+                move_shared_libs(build_prefix, build_lib, "arrow_python")
+                if self.with_plasma:
+                    move_shared_libs(build_prefix, build_lib, "plasma")
+                if self.with_parquet and not self.with_static_parquet:
+                    move_shared_libs(build_prefix, build_lib, "parquet")
+
+            print('Bundling includes: ' + pjoin(build_prefix, 'include'))
+            if os.path.exists(pjoin(build_lib, 'pyarrow', 'include')):
+                shutil.rmtree(pjoin(build_lib, 'pyarrow', 'include'))
+            shutil.move(pjoin(build_prefix, 'include'),
+                        pjoin(build_lib, 'pyarrow'))
+
+            # Move the built C-extension to the place expected by the Python 
build
+            self._found_names = []
+            for name in self.CYTHON_MODULE_NAMES:
+                built_path = self.get_ext_built(name)
+                if not os.path.exists(built_path):
+                    print(built_path)
+                    if self._failure_permitted(name):
+                        print('Cython module {0} failure 
permitted'.format(name))
+                        continue
+                    raise RuntimeError('pyarrow C-extension failed to build:',
+                                       os.path.abspath(built_path))
+
+                ext_path = pjoin(build_lib, self._get_cmake_ext_path(name))
+                if os.path.exists(ext_path):
+                    os.remove(ext_path)
+                self.mkpath(os.path.dirname(ext_path))
+                print('Moving built C-extension', built_path,
+                      'to build path', ext_path)
+                shutil.move(self.get_ext_built(name), ext_path)
+                self._found_names.append(name)
+
+                if os.path.exists(self.get_ext_built_api_header(name)):
+                    shutil.move(self.get_ext_built_api_header(name),
+                                pjoin(os.path.dirname(ext_path), name + 
'_api.h'))
+
+            # Move the plasma store
             if self.with_plasma:
-                move_shared_libs(build_prefix, build_lib, "plasma")
-            if self.with_parquet and not self.with_static_parquet:
-                move_shared_libs(build_prefix, build_lib, "parquet")
-
-        print('Bundling includes: ' + pjoin(build_prefix, 'include'))
-        if os.path.exists(pjoin(build_lib, 'pyarrow', 'include')):
-            shutil.rmtree(pjoin(build_lib, 'pyarrow', 'include'))
-        shutil.move(pjoin(build_prefix, 'include'),
-                    pjoin(build_lib, 'pyarrow'))
-
-        # Move the built C-extension to the place expected by the Python build
-        self._found_names = []
-        for name in self.CYTHON_MODULE_NAMES:
-            built_path = self.get_ext_built(name)
-            if not os.path.exists(built_path):
-                print(built_path)
-                if self._failure_permitted(name):
-                    print('Cython module {0} failure permitted'.format(name))
-                    continue
-                raise RuntimeError('pyarrow C-extension failed to build:',
-                                   os.path.abspath(built_path))
-
-            ext_path = pjoin(build_lib, self._get_cmake_ext_path(name))
-            if os.path.exists(ext_path):
-                os.remove(ext_path)
-            self.mkpath(os.path.dirname(ext_path))
-            print('Moving built C-extension', built_path,
-                  'to build path', ext_path)
-            shutil.move(self.get_ext_built(name), ext_path)
-            self._found_names.append(name)
-
-            if os.path.exists(self.get_ext_built_api_header(name)):
-                shutil.move(self.get_ext_built_api_header(name),
-                            pjoin(os.path.dirname(ext_path), name + '_api.h'))
-
-        # Move the plasma store
-        if self.with_plasma:
-            build_py = self.get_finalized_command('build_py')
-            source = os.path.join(self.build_type, "plasma_store")
-            target = os.path.join(build_lib,
-                                  build_py.get_package_dir('pyarrow'),
-                                  "plasma_store")
-            shutil.move(source, target)
-
-        os.chdir(saved_cwd)
+                build_py = self.get_finalized_command('build_py')
+                source = os.path.join(self.build_type, "plasma_store")
+                target = os.path.join(build_lib,
+                                      build_py.get_package_dir('pyarrow'),
+                                      "plasma_store")
+                shutil.move(source, target)
 
     def _failure_permitted(self, name):
         if name == '_parquet' and not self.with_parquet:


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> [Python] chdir logic in setup.py buggy
> --------------------------------------
>
>                 Key: ARROW-2070
>                 URL: https://issues.apache.org/jira/browse/ARROW-2070
>             Project: Apache Arrow
>          Issue Type: Bug
>          Components: Python
>    Affects Versions: 0.8.0
>            Reporter: Antoine Pitrou
>            Assignee: Antoine Pitrou
>            Priority: Minor
>              Labels: pull-request-available
>             Fix For: 0.9.0
>
>
> In some conditions, {{setup.py}} may change the current directory and omit 
> restoring the previous one, which can fail some operations.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to