From 6893303c9e441ff1f456304f1dc53e62d964c323 Mon Sep 17 00:00:00 2001 From: Benjamin Alan Weaver Date: Wed, 17 Aug 2022 16:44:48 -0700 Subject: [PATCH 1/6] add method for compiling code in a branch --- py/desiutil/install.py | 23 +++++++++++++++++++++ py/desiutil/test/test_install.py | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/py/desiutil/install.py b/py/desiutil/install.py index 504a5bf..b3f5010 100644 --- a/py/desiutil/install.py +++ b/py/desiutil/install.py @@ -909,6 +909,28 @@ def get_extra(self): raise DesiInstallException(message) return + def compile_branch(self): + """Certain packages need C/C++ code compiled even for a branch install. + """ + if self.is_branch: + compile_script = os.path.join(self.install_dir, 'etc', + '{0}_compile.sh'.format(self.baseproduct)) + if os.path.exists(compile_script): + self.log.debug("Detected compile script: %s.", compile_script) + if self.options.test: + self.log.debug('Test Mode. Skipping compile script.') + else: + proc = Popen([compile_script, sys.executable], universal_newlines=True, + stdout=PIPE, stderr=PIPE) + out, err = proc.communicate() + status = proc.returncode + self.log.debug(out) + if status != 0 and len(err) > 0: + message = "Error compiling code: {0}".format(err) + self.log.critical(message) + raise DesiInstallException(message) + return + def verify_bootstrap(self): """Make sure that desiutil/desiInstall was installed with an explicit Python executable path. @@ -1026,6 +1048,7 @@ def run(self): # pragma: no cover self.prepare_environment() self.install() self.get_extra() + self.compile_branch() self.verify_bootstrap() self.permissions() except DesiInstallException: diff --git a/py/desiutil/test/test_install.py b/py/desiutil/test/test_install.py index 73c18f7..2c99d05 100644 --- a/py/desiutil/test/test_install.py +++ b/py/desiutil/test/test_install.py @@ -2,6 +2,7 @@ # -*- coding: utf-8 -*- """Test desiutil.install. """ +import sys import unittest from unittest.mock import patch, call, MagicMock, mock_open from os import chdir, environ, getcwd, mkdir, remove, rmdir @@ -507,6 +508,39 @@ def test_get_extra(self, mock_popen, mock_exists): self.assertLog(-1, message) self.assertEqual(str(cm.exception), message) + @patch('os.path.exists') + @patch('desiutil.install.Popen') + def test_compile_branch(self, mock_popen, mock_exists): + """Test compiling code in certain cases. + """ + options = self.desiInstall.get_options(['fiberassign', 'branches/main']) + self.desiInstall.baseproduct = 'fiberassign' + self.desiInstall.is_branch = True + self.desiInstall.install_dir = join(self.data_dir, 'fiberassign') + mock_exists.return_value = True + mock_proc = mock_popen() + mock_proc.returncode = 0 + mock_proc.communicate.return_value = ('out', 'err') + self.desiInstall.compile_branch() + # mock_exists.assert_called_once_with(join(self.desiInstall.install_dir, 'etc', 'fiberassign_compile.sh'), + # sys.executable) + mock_popen.assert_has_calls([call([join(self.desiInstall.install_dir, 'etc', 'fiberassign_compile.sh'), sys.executable], + stderr=-1, stdout=-1, universal_newlines=True)], any_order=True) + mock_popen.reset_mock() + self.desiInstall.options.test = True + self.desiInstall.compile_branch() + self.assertLog(-1, 'Test Mode. Skipping compile script.') + mock_popen.reset_mock() + self.desiInstall.options.test = False + mock_proc = mock_popen() + mock_proc.returncode = 1 + mock_proc.communicate.return_value = ('out', 'err') + with self.assertRaises(DesiInstallException) as cm: + self.desiInstall.compile_branch() + message = "Error compiling code: err" + self.assertLog(-1, message) + self.assertEqual(str(cm.exception), message) + def test_verify_bootstrap(self): """Test proper installation of the desiInstall executable. """ From 4ccdccb81f503dffc65f7db13bcae21d1fbbf983 Mon Sep 17 00:00:00 2001 From: Benjamin Alan Weaver Date: Wed, 17 Aug 2022 17:14:32 -0700 Subject: [PATCH 2/6] add documentation --- doc/changes.rst | 2 ++ doc/desiInstall.rst | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/doc/changes.rst b/doc/changes.rst index 8697970..15c1410 100644 --- a/doc/changes.rst +++ b/doc/changes.rst @@ -8,9 +8,11 @@ Change Log * :command:`desiInstall` uses desihub location of simqso fork (commit e963344_). * Allow :command:`desiInstall` to remove permission-locked directories; suppress certain :command:`pip` warnings (PR `#185`_). +* Allow :command:`desiInstall` to compile code in certain branch installs (PR `#188`_). .. _e963344: https://github.com/desihub/desiutil/commit/e963344cd072255174187d2bd6da72d085745abd .. _`#185`: https://github.com/desihub/desiutil/pull/185 +.. _`#188`: https://github.com/desihub/desiutil/pull/188 3.2.5 (2022-01-20) ------------------ diff --git a/doc/desiInstall.rst b/doc/desiInstall.rst index e433192..3200bd8 100644 --- a/doc/desiInstall.rst +++ b/doc/desiInstall.rst @@ -322,6 +322,20 @@ not bundled with the code. The script should download data *directly* to with :command:`desiInstall` and unit tests. Note that here are other, better ways to install and manipulate data that is bundled *with* a Python package. +Compile in Branch Installs +-------------------------- + +In a few cases (fiberassign_, specex_) code needs to be compiled even when +installing a branch. If :command:`desiInstall` detects a branch install *and* +the script ``etc/product_compile.sh``, :command:`desiInstall` will run this +script, supplying the Python executable path as a single command-line arguement. +The script itself is intended to be a thin wrapper on *e.g.*:: + + python setup.py build_ext --inplace + +.. _fiberassign: https://github.com/desihub/fiberassign +.. _specex: https://github.com/desihub/specex + Fix Permissions --------------- From 6a4f584af7bd6b95f6b8ab7e84cf129becdf94f6 Mon Sep 17 00:00:00 2001 From: Benjamin Alan Weaver Date: Thu, 18 Aug 2022 11:36:27 -0700 Subject: [PATCH 3/6] update change log --- doc/changes.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/changes.rst b/doc/changes.rst index 15c1410..93581d9 100644 --- a/doc/changes.rst +++ b/doc/changes.rst @@ -9,10 +9,13 @@ Change Log * Allow :command:`desiInstall` to remove permission-locked directories; suppress certain :command:`pip` warnings (PR `#185`_). * Allow :command:`desiInstall` to compile code in certain branch installs (PR `#188`_). +* Add `gpu_specter`_ to known packages (PR `#189`_). .. _e963344: https://github.com/desihub/desiutil/commit/e963344cd072255174187d2bd6da72d085745abd .. _`#185`: https://github.com/desihub/desiutil/pull/185 .. _`#188`: https://github.com/desihub/desiutil/pull/188 +.. _`#189`: https://github.com/desihub/desiutil/pull/189 +.. _`gpu_specter`: https://github.com/desihub/gpu_specter 3.2.5 (2022-01-20) ------------------ From fe0d5c775b27eda069bb3beb4a390019464745ed Mon Sep 17 00:00:00 2001 From: Benjamin Alan Weaver Date: Thu, 18 Aug 2022 15:08:32 -0700 Subject: [PATCH 4/6] change dir internally to desiInstall --- py/desiutil/install.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/py/desiutil/install.py b/py/desiutil/install.py index bae97e2..b4df026 100644 --- a/py/desiutil/install.py +++ b/py/desiutil/install.py @@ -921,11 +921,16 @@ def compile_branch(self): if self.options.test: self.log.debug('Test Mode. Skipping compile script.') else: + current_dir = os.getcwd() + self.log.debug("os.chdir('%s')", self.install_dir) + os.chdir(self.install_dir) proc = Popen([compile_script, sys.executable], universal_newlines=True, stdout=PIPE, stderr=PIPE) out, err = proc.communicate() status = proc.returncode self.log.debug(out) + self.log.debug("os.chdir('%s')", current_dir) + os.chdir(original_dir) if status != 0 and len(err) > 0: message = "Error compiling code: {0}".format(err) self.log.critical(message) From f2f74d9d7d6cd60ec17ec0eb6a4f7c9322300f36 Mon Sep 17 00:00:00 2001 From: Benjamin Alan Weaver Date: Thu, 18 Aug 2022 15:09:52 -0700 Subject: [PATCH 5/6] update documentation --- doc/desiInstall.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/desiInstall.rst b/doc/desiInstall.rst index 3200bd8..8c926f0 100644 --- a/doc/desiInstall.rst +++ b/doc/desiInstall.rst @@ -327,11 +327,14 @@ Compile in Branch Installs In a few cases (fiberassign_, specex_) code needs to be compiled even when installing a branch. If :command:`desiInstall` detects a branch install *and* -the script ``etc/product_compile.sh``, :command:`desiInstall` will run this -script, supplying the Python executable path as a single command-line arguement. +the script ``etc/product_compile.sh`` exists, :command:`desiInstall` will run this +script, supplying the Python executable path as a single command-line argument. The script itself is intended to be a thin wrapper on *e.g.*:: - python setup.py build_ext --inplace + #!/bin/bash + py=$1 + ${py} setup.py build_ext --inplace + .. _fiberassign: https://github.com/desihub/fiberassign .. _specex: https://github.com/desihub/specex From bc34890f586185020dfe27c5810c467ea99c228e Mon Sep 17 00:00:00 2001 From: Benjamin Alan Weaver Date: Thu, 18 Aug 2022 15:18:16 -0700 Subject: [PATCH 6/6] fix tests --- py/desiutil/install.py | 2 +- py/desiutil/test/test_install.py | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/py/desiutil/install.py b/py/desiutil/install.py index b4df026..7dfc4cf 100644 --- a/py/desiutil/install.py +++ b/py/desiutil/install.py @@ -930,7 +930,7 @@ def compile_branch(self): status = proc.returncode self.log.debug(out) self.log.debug("os.chdir('%s')", current_dir) - os.chdir(original_dir) + os.chdir(current_dir) if status != 0 and len(err) > 0: message = "Error compiling code: {0}".format(err) self.log.critical(message) diff --git a/py/desiutil/test/test_install.py b/py/desiutil/test/test_install.py index 2c99d05..a12433f 100644 --- a/py/desiutil/test/test_install.py +++ b/py/desiutil/test/test_install.py @@ -508,11 +508,13 @@ def test_get_extra(self, mock_popen, mock_exists): self.assertLog(-1, message) self.assertEqual(str(cm.exception), message) + @patch('os.chdir') @patch('os.path.exists') @patch('desiutil.install.Popen') - def test_compile_branch(self, mock_popen, mock_exists): + def test_compile_branch(self, mock_popen, mock_exists, mock_chdir): """Test compiling code in certain cases. """ + current_dir = getcwd() options = self.desiInstall.get_options(['fiberassign', 'branches/main']) self.desiInstall.baseproduct = 'fiberassign' self.desiInstall.is_branch = True @@ -522,8 +524,9 @@ def test_compile_branch(self, mock_popen, mock_exists): mock_proc.returncode = 0 mock_proc.communicate.return_value = ('out', 'err') self.desiInstall.compile_branch() - # mock_exists.assert_called_once_with(join(self.desiInstall.install_dir, 'etc', 'fiberassign_compile.sh'), - # sys.executable) + mock_chdir.assert_has_calls([call(self.desiInstall.install_dir), + call(current_dir)]) + mock_exists.assert_has_calls([call(join(self.desiInstall.install_dir, 'etc', 'fiberassign_compile.sh'))]) mock_popen.assert_has_calls([call([join(self.desiInstall.install_dir, 'etc', 'fiberassign_compile.sh'), sys.executable], stderr=-1, stdout=-1, universal_newlines=True)], any_order=True) mock_popen.reset_mock()