Repository: desihub/desiutil

Issue:
Allow desiInstall to install packages with world-read
Currently `desiInstall` removes world-read permissions when installing DESI packages.  Since, in principle, we want all NERSC users to have access to the DESI software stack to work with public data, we need to default to installing world-read permission, with the non-default option to remove world-read if a package (hypothetically) should remain private.

Modulefiles that are installed alongside packages can be world-readable unconditionally.



Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index d73e8fc..af39064 100644
--- a/.gitignore
+++ b/.gitignore
@@ -33,6 +33,8 @@ MANIFEST
 # Other
 .*.swp
 *~
+.vscode
+.env
 
 # Mac OSX
 .DS_Store
diff --git a/doc/desiInstall.rst b/doc/desiInstall.rst
index 8c926f0..c1b28b7 100644
--- a/doc/desiInstall.rst
+++ b/doc/desiInstall.rst
@@ -211,9 +211,10 @@ possible build types that are mutually exclusive.  They are derived in this
 order and the first matching method is used:
 
 py
-    If a setup.py file is detected, :command:`desiInstall` will attempt to execute
-    :command:`pip install .`.  This build type can be suppressed with the
-    command line option ``--compile-c``.
+    If a pyproject.toml or a setup.py file is detected,
+    :command:`desiInstall` will attempt to execute :command:`pip install .`.
+    This build type can be suppressed with the command line option
+    ``--compile-c``.
 make
     If a Makefile is detected, :command:`desiInstall` will attempt to execute
     :command:`make install`.
@@ -274,7 +275,8 @@ Configure Module File
 :command:`desiInstall` will scan :envvar:`WORKING_DIR` to determine the details that need
 to be added to the module file.  The final module file will then be written
 into the DESI module directory at NERSC.  If ``--default`` is specified
-on the command line, an appropriate .version file will be created.
+on the command line, an appropriate .version file will be created. Module
+files are always installed with world-read permissions.
 
 Load Module
 -----------
@@ -342,7 +344,12 @@ The script itself is intended to be a thin wrapper on *e.g.*::
 Fix Permissions
 ---------------
 
-The script :command:`fix_permissions.sh` will be run on :envvar:`INSTALL_DIR`.
+The permissions of :envvar:`INSTALL_DIR` will be recursively set to standard
+values under these circumstances:
+
+1. World-read, unless ``--no-world`` is specified on the command line.
+2. Unwriteable to all, unless a branch install is being performed, in which
+   case user-write is set.
 
 Clean Up
 --------
diff --git a/py/desiutil/iers.py b/py/desiutil/iers.py
index 9d669f9..5ac8908 100644
--- a/py/desiutil/iers.py
+++ b/py/desiutil/iers.py
@@ -116,6 +116,10 @@ def freeze_iers(name='iers_frozen.ecsv', ignore_warnings=True):
     astropy.utils.iers.conf.auto_max_age = None
     astropy.utils.iers.conf.iers_auto_url = 'frozen'
     astropy.utils.iers.conf.iers_auto_url_mirror = 'frozen'
+    if ignore_warnings:
+        astropy.utils.iers.conf.iers_degraded_accuracy = 'ignore'
+    else:
+        astropy.utils.iers.conf.iers_degraded_accuracy = 'warn'
     # Sanity check.
     auto_class = astropy.utils.iers.IERS_Auto.open()
     if auto_class is not iers:
diff --git a/py/desiutil/install.py b/py/desiutil/install.py
index 96b8093..96fef1e 100644
--- a/py/desiutil/install.py
+++ b/py/desiutil/install.py
@@ -10,7 +10,7 @@ This package contains code for installing DESI software products.
 import os
 import sys
 import tarfile
-import re
+import stat
 import shutil
 import requests
 from io import BytesIO
@@ -239,6 +239,9 @@ class DesiInstall(object):
                             help='Print extra information.')
         parser.add_argument('-V', '--version', action='version',
                             version='%(prog)s ' + desiutilVersion)
+        parser.add_argument('-W', '--no-world', action='store_false',
+                            dest='world',
+                            help='Disable world-readable installation.')
         parser.add_argument('product', nargs='?',
                             default='NO PACKAGE',
                             help='Name of product to install.')
@@ -538,7 +541,7 @@ class DesiInstall(object):
             self.log.debug("Forcing build type: make")
             build_type.add('make')
         else:
-            if os.path.exists(os.path.join(self.working_dir, 'setup.py')):
+            if (os.path.exists(os.path.join(self.working_dir, 'pyproject.toml')) or os.path.exists(os.path.join(self.working_dir, 'setup.py'))):
                 self.log.debug("Detected build type: py")
                 build_type.add('py')
             elif os.path.exists(os.path.join(self.working_dir, 'Makefile')):
@@ -966,40 +969,42 @@ class DesiInstall(object):
         return True
 
     def permissions(self):
-        """Fix possible install permission errors.
-
-        Returns
-        -------
-        :class:`int`
-            Status code returned by fix_permissions.sh script.
+        """Set permissions on installed software.
         """
-        command = ['fix_permissions.sh']
-        if self.options.verbose:
-            command.append('-v')
-        if self.options.test:
-            command.append('-t')
-        command.append(self.install_dir)
-        self.log.debug(' '.join(command))
-        proc = Popen(command, universal_newlines=True,
-                     stdout=PIPE, stderr=PIPE)
-        out, err = proc.communicate()
-        status = proc.returncode
-        self.log.debug(out)
+        read_file = stat.S_IRUSR | stat.S_IRGRP
+        if self.options.world:
+            read_file |= stat.S_IROTH
+        read_exec = read_file | stat.S_IXUSR | stat.S_IXGRP
+        if self.options.world:
+            read_exec |= stat.S_IXOTH
+        read_dir = read_exec | stat.S_ISGID
+        if self.is_branch:
+            read_file |= stat.S_IWUSR
+            read_exec |= stat.S_IWUSR
+            read_dir |= stat.S_IWUSR
         #
-        # Remove write permission to avoid accidental changes
+        # Recursively set permissions from the bottom up.
         #
-        if self.is_branch:
-            chmod_mode = 'g-w,o-w'
-        else:
-            chmod_mode = 'a-w'
-        command = ['chmod', '-R', chmod_mode, self.install_dir]
-        self.log.debug(' '.join(command))
-        proc = Popen(command, universal_newlines=True,
-                     stdout=PIPE, stderr=PIPE)
-        out, err = proc.communicate()
-        chmod_status = proc.returncode
-        self.log.debug(out)
-        return status
+        for dirpath, dirnames, filenames in os.walk(self.install_dir, topdown=False):
+            for f in filenames:
+                fname = os.path.join(dirpath, f)
+                if os.path.islink(fname):
+                    continue
+                executable = (stat.S_IMODE(os.stat(fname).st_mode) & stat.S_IXUSR) != 0
+                if executable:
+                    self.log.debug("os.chmod('%s', %s)", fname, read_exec)
+                    os.chmod(fname, read_exec)
+                else:
+                    self.log.debug("os.chmod('%s', %s)", fname, read_exec)
+                    os.chmod(fname, read_file)
+            for d in dirnames:
+                self.log.debug("os.chmod('%s', %s)", os.path.join(dirpath, d), read_dir)
+                os.chmod(os.path.join(dirpath, d), read_dir)
+        #
+        # Finally set permissions on the top directory.
+        #
+        self.log.debug("os.chmod('%s', %s)", self.install_dir, read_dir)
+        os.chmod(self.install_dir, read_dir)
 
     def unlock_permissions(self):
         """Unlock installed directories to allow their removal.
diff --git a/py/desiutil/modules.py b/py/desiutil/modules.py
index 2efe92e..76d1386 100644
--- a/py/desiutil/modules.py
+++ b/py/desiutil/modules.py
@@ -15,10 +15,7 @@ import sys
 from argparse import ArgumentParser
 from shutil import which
 from stat import S_IRUSR, S_IRGRP, S_IROTH
-try:
-    from ConfigParser import SafeConfigParser
-except ImportError:
-    from configparser import ConfigParser as SafeConfigParser
+from configparser import ConfigParser
 from pkg_resources import resource_filename
 from . import __version__ as desiutilVersion
 from .io import unlock_file
@@ -224,14 +221,14 @@ def configure_module(product, version, product_root, working_dir=None, dev=False
         else:
             module_keywords['needs_python'] = ''
     if os.path.exists(os.path.join(working_dir, 'setup.cfg')):
-        conf = SafeConfigParser()
+        conf = ConfigParser()
         conf.read([os.path.join(working_dir, 'setup.cfg')])
         if conf.has_section('entry_points') or conf.has_section('options.entry_points'):
             module_keywords['needs_bin'] = ''
     return module_keywords
 
 
-def process_module(module_file, module_keywords, module_dir, world=True):
+def process_module(module_file, module_keywords, module_dir):
     """Process a Module file.
 
     Parameters
@@ -242,13 +239,15 @@ def process_module(module_file, module_keywords, module_dir, world=True):
         The parameters to use for Module file processing.
     module_dir : :class:`str`
         The directory where the Module file should be installed.
-    world : :class:`bool`, optional
-        Make module files world-readable.
 
     Returns
     -------
     :class:`str`
         The text of the processed Module file.
+
+    Note
+    ----
+    Module files are always installed with world-read permissions.
     """
     if not os.path.isdir(os.path.join(module_dir, module_keywords['name'])):
         os.makedirs(os.path.join(module_dir, module_keywords['name']))
@@ -256,11 +255,11 @@ def process_module(module_file, module_keywords, module_dir, world=True):
                                        module_keywords['version'])
     with open(module_file) as m:
         mod = m.read().format(**module_keywords)
-    _write_module_data(install_module_file, mod, world=world)
+    _write_module_data(install_module_file, mod)
     return mod
 
 
-def default_module(module_keywords, module_dir, world=True):
+def default_module(module_keywords, module_dir):
     """Install or update a .version file to set the default Module.
 
     Parameters
@@ -269,23 +268,25 @@ def default_module(module_keywords, module_dir, world=True):
         The parameters to use for Module file processing.
     module_dir : :class:`str`
         The directory where the Module file should be installed.
-    world : :class:`bool`, optional
-        Make .version files world-readable.
 
     Returns
     -------
     :class:`str`
         The text of the processed .version file.
+
+    Note
+    ----
+    .version files are always installed with world-read permissions.
     """
     dot_template = '#%Module1.0\nset ModulesVersion "{version}"\n'
     install_version_file = os.path.join(module_dir, module_keywords['name'],
                                         '.version')
     dot_version = dot_template.format(**module_keywords)
-    _write_module_data(install_version_file, dot_version, world=world)
+    _write_module_data(install_version_file, dot_version)
     return dot_version
 
 
-def _write_module_data(filename, data, world=True):
+def _write_module_data(filename, data):
     """Write and permission-lock Module file data.  This is intended
     to consolidate some duplicated code.
 
@@ -295,14 +296,10 @@ def _write_module_data(filename, data, world=True):
         The module file to write.
     data : :class:`str`
         The data to be written to `filename`.
-    world : :class:`bool`, optional
-        Make `filename` world-readable.
     """
     with unlock_file(filename, 'w') as f:
         f.write(data)
-    p = S_IRUSR | S_IRGRP
-    if world:
-        p |= S_IROTH
+    p = S_IRUSR | S_IRGRP | S_IROTH
     os.chmod(filename, p)
     return
 
@@ -319,7 +316,6 @@ def main():
                             prog=os.path.basename(sys.argv[0]))
     parser.add_argument('-d', '--default', dest='default', action='store_true', help='Mark this Module as default.')
     parser.add_argument('-m', '--modules', dest='modules', help='Set the Module install directory.')
-    parser.add_argument('-p', '--private', dest='world', action='store_false', help='Do not make module files world-readable.')
     parser.add_argument('-V', '--version', action='version', version='%(prog)s ' + desiutilVersion)
     parser.add_argument('product', help='Name of product.')
     parser.add_argument('product_version', help='Version of product.')
@@ -327,11 +323,11 @@ def main():
 
     if options.modules is None:
         try:
-            self.modules = os.path.join('/global/common/software/desi',
-                                        os.environ['NERSC_HOST'],
-                                        'desiconda',
-                                        'current',
-                                        'modulefiles')
+            options.modules = os.path.join('/global/common/software/desi',
+                                           os.environ['NERSC_HOST'],
+                                           'desiconda',
+                                           'current',
+                                           'modulefiles')
         except KeyError:
             try:
                 options.modules = os.path.join(os.environ['DESI_PRODUCT_ROOT'],
@@ -353,9 +349,9 @@ def main():
         log.warning("Could not find Module file: %s; using default.", module_file)
         module_file = resource_filename('desiutil', 'data/desiutil.module')
 
-    process_module(module_file, module_keywords, options.modules, world=options.world)
+    process_module(module_file, module_keywords, options.modules)
 
     if options.default:
-        default_module(module_keywords, options.modules, world=options.world)
+        default_module(module_keywords, options.modules)
 
     return 0


Test patch:
diff --git a/py/desiutil/test/test_iers.py b/py/desiutil/test/test_iers.py
index e95ac49..5130d3a 100644
--- a/py/desiutil/test/test_iers.py
+++ b/py/desiutil/test/test_iers.py
@@ -117,6 +117,21 @@ class TestIERS(unittest.TestCase):
         self.assertIsNone(astropy.utils.iers.conf.auto_max_age)
         self.assertEqual(astropy.utils.iers.conf.iers_auto_url, 'frozen')
         self.assertEqual(astropy.utils.iers.conf.iers_auto_url_mirror, 'frozen')
+        self.assertEqual(astropy.utils.iers.conf.iers_degraded_accuracy, 'ignore')
+        mock_logger().info.assert_has_calls([call('Freezing IERS table used by astropy time, coordinates.')])
+
+    @patch('desiutil.iers.get_logger')
+    def test_freeze_iers_ignore_warnings(self, mock_logger):
+        """Test freezing from package data/, but allow warnings.
+        """
+        i.freeze_iers(ignore_warnings=False)
+        future = Time('2024-01-01', location=self.location)
+        lst = future.sidereal_time('apparent')
+        self.assertFalse(astropy.utils.iers.conf.auto_download)
+        self.assertIsNone(astropy.utils.iers.conf.auto_max_age)
+        self.assertEqual(astropy.utils.iers.conf.iers_auto_url, 'frozen')
+        self.assertEqual(astropy.utils.iers.conf.iers_auto_url_mirror, 'frozen')
+        self.assertEqual(astropy.utils.iers.conf.iers_degraded_accuracy, 'warn')
         mock_logger().info.assert_has_calls([call('Freezing IERS table used by astropy time, coordinates.')])
 
     @patch('desiutil.iers.get_logger')
diff --git a/py/desiutil/test/test_install.py b/py/desiutil/test/test_install.py
index 6813042..ab401ce 100644
--- a/py/desiutil/test/test_install.py
+++ b/py/desiutil/test/test_install.py
@@ -6,7 +6,7 @@ import sys
 import unittest
 from unittest.mock import patch, call, MagicMock, mock_open
 from os import chdir, environ, getcwd, mkdir, remove, rmdir
-from os.path import abspath, dirname, isdir, join
+from os.path import abspath, basename, isdir, join
 from shutil import rmtree
 from argparse import Namespace
 from tempfile import mkdtemp
@@ -17,6 +17,18 @@ from ..install import DesiInstall, DesiInstallException, dependencies
 from .test_log import NullMemoryHandler
 
 
+def replace_stat(filename):
+    """Mock os.stat().
+    """
+    class st_mode(object):
+        def __init__(self, st_mode):
+            self.st_mode = st_mode
+
+    if basename(filename) == 'executable':
+        return st_mode(33133)
+    return st_mode(33184)
+
+
 class TestInstall(unittest.TestCase):
     """Test desiutil.install.
     """
@@ -87,7 +99,8 @@ class TestInstall(unittest.TestCase):
                 root=None,
                 test=False,
                 username=environ['USER'],
-                verbose=False)
+                verbose=False,
+                world=True)
             options = self.desiInstall.get_options([])
             self.assertEqual(options, default_namespace)
             default_namespace.product = 'product'
@@ -321,7 +334,7 @@ class TestInstall(unittest.TestCase):
         self.assertEqual(self.desiInstall.build_type, set(['plain', 'make']))
         # Create temporary files
         options = self.desiInstall.get_options(['desispec', '1.0.0'])
-        tempfiles = {'Makefile': 'make', 'setup.py': 'py'}
+        tempfiles = {'Makefile': 'make', 'pyproject.toml': 'py', 'setup.py': 'py'}
         for t in tempfiles:
             tempfile = join(self.data_dir, t)
             with open(tempfile, 'w') as tf:
@@ -439,10 +452,36 @@ class TestInstall(unittest.TestCase):
         status = self.desiInstall.start_modules()
         self.assertTrue(callable(self.desiInstall.module))
 
-    def test_module_dependencies(self):
+    @patch('desiutil.install.dependencies')
+    @patch('os.path.exists')
+    def test_module_dependencies(self, mock_exists, mock_dependencies):
         """Test module-loading dependencies.
         """
-        pass
+        mock_dependencies.return_value = ['desiutil/main', 'foobar']
+        mock_exists.return_value = True
+        options = self.desiInstall.get_options(['desispec', '1.9.5'])
+        self.desiInstall.baseproduct = 'desispec'
+        self.desiInstall.working_dir = join(self.data_dir, 'desispec')
+        self.desiInstall.module = MagicMock()
+        self.assertFalse(self.desiInstall.options.test)
+        with patch.dict('os.environ', {'LOADEDMODULES': 'desiutil'}):
+            deps = self.desiInstall.module_dependencies()
+        self.assertListEqual(self.desiInstall.deps, ['desiutil/main', 'foobar'])
+        self.assertEqual(self.desiInstall.module_file, join(self.desiInstall.working_dir, 'etc', 'desispec.module'))
+        self.desiInstall.module.assert_has_calls([call('switch', 'desiutil/main'), call('load', 'foobar')])
+        mock_exists.assert_has_calls([call(join(self.desiInstall.working_dir, 'etc', 'desispec.module'))], any_order=True)
+        mock_dependencies.assert_called_once_with(join(self.desiInstall.working_dir, 'etc', 'desispec.module'))
+
+    def test_module_dependencies_test_mode(self):
+        """Test module-loading dependencies in test mode.
+        """
+        options = self.desiInstall.get_options(['--test', 'desutil', '1.9.5'])
+        self.desiInstall.baseproduct = 'desiutil'
+        self.desiInstall.working_dir = join(self.data_dir, 'desiutil')
+        self.assertTrue(self.desiInstall.options.test)
+        deps = self.desiInstall.module_dependencies()
+        self.assertListEqual(self.desiInstall.deps, [])
+        self.assertLog(-1, 'Test Mode. Skipping loading of dependencies.')
 
     def test_nersc_module_dir(self):
         """Test the nersc_module_dir property.
@@ -574,43 +613,161 @@ exit(main())
         self.assertEqual(str(cm.exception), message)
         self.assertLog(-1, message)
 
-    @patch('desiutil.install.Popen')
-    def test_permissions(self, mock_popen):
-        """Test the permissions stage of the install.
+    @patch('os.stat', replace_stat)
+    @patch('os.walk')
+    @patch('os.chmod')
+    def test_permissions(self, mock_chmod, mock_walk):
+        """Test the permission stage of the install.
+        """
+        options = self.desiInstall.get_options(['desiutil', '1.2.3'])
+        self.assertTrue(self.desiInstall.options.world)
+        self.desiInstall.install_dir = join(self.data_dir, 'desiutil')
+        self.desiInstall.is_branch = False
+        mock_walk.return_value = iter([(join(self.desiInstall.install_dir, 'bin'), [], ['executable', 'README.txt']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), [], ['METADATA', 'LICENSE.rst']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), [], ['__init__.cpython-3.10.pyc', 'module.cpython-3.10.pyc']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), ['__pycache__'], ['__init__.py', 'module.py']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), ['desiutil-1.2.3.dist-info', 'desiutil'], []),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10'), ['site-packages'], []),
+                                       (join(self.desiInstall.install_dir, 'lib'), ['python3.10'], []),
+                                       (self.desiInstall.install_dir, ['bin', 'lib'], [])])
+        self.desiInstall.permissions()
+        mock_walk.assert_called_once_with(self.desiInstall.install_dir, topdown=False)
+        mock_chmod.assert_has_calls([call(join(self.desiInstall.install_dir, 'bin', 'executable'), 0o555),
+                                     call(join(self.desiInstall.install_dir, 'bin', 'README.txt'), 0o444),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'METADATA'), 0o444),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'LICENSE.rst'), 0o444),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', '__init__.cpython-3.10.pyc'), 0o444),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', 'module.cpython-3.10.pyc'), 0o444),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__init__.py'), 0o444),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', 'module.py'), 0o444),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), 0o2555),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), 0o2555),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), 0o2555),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), 0o2555),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10'), 0o2555),
+                                     call(join(self.desiInstall.install_dir, 'bin'), 0o2555),
+                                     call(join(self.desiInstall.install_dir, 'lib'), 0o2555),
+                                     call(self.desiInstall.install_dir, 0o2555)])
+        self.assertLog(-2, "os.chmod('%s', %s)" % (join(self.desiInstall.install_dir, 'lib'), 0o2555))
+        self.assertLog(-1, "os.chmod('%s', %s)" % (self.desiInstall.install_dir, 0o2555))
+
+    @patch('os.stat', replace_stat)
+    @patch('os.walk')
+    @patch('os.chmod')
+    def test_permissions_with_branch(self, mock_chmod, mock_walk):
+        """Test the permission stage of the install with a branch.
         """
         options = self.desiInstall.get_options(['desiutil', 'branches/main'])
+        self.assertTrue(self.desiInstall.options.world)
+        self.desiInstall.install_dir = join(self.data_dir, 'desiutil')
+        self.desiInstall.is_branch = True
+        mock_walk.return_value = iter([(join(self.desiInstall.install_dir, 'bin'), [], ['executable', 'README.txt']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), [], ['METADATA', 'LICENSE.rst']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), [], ['__init__.cpython-3.10.pyc', 'module.cpython-3.10.pyc']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), ['__pycache__'], ['__init__.py', 'module.py']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), ['desiutil-1.2.3.dist-info', 'desiutil'], []),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10'), ['site-packages'], []),
+                                       (join(self.desiInstall.install_dir, 'lib'), ['python3.10'], []),
+                                       (self.desiInstall.install_dir, ['bin', 'lib'], [])])
+        self.desiInstall.permissions()
+        mock_walk.assert_called_once_with(self.desiInstall.install_dir, topdown=False)
+        mock_chmod.assert_has_calls([call(join(self.desiInstall.install_dir, 'bin', 'executable'), 0o755),
+                                     call(join(self.desiInstall.install_dir, 'bin', 'README.txt'), 0o644),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'METADATA'), 0o644),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'LICENSE.rst'), 0o644),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', '__init__.cpython-3.10.pyc'), 0o644),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', 'module.cpython-3.10.pyc'), 0o644),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__init__.py'), 0o644),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', 'module.py'), 0o644),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), 0o2755),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), 0o2755),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), 0o2755),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), 0o2755),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10'), 0o2755),
+                                     call(join(self.desiInstall.install_dir, 'bin'), 0o2755),
+                                     call(join(self.desiInstall.install_dir, 'lib'), 0o2755),
+                                     call(self.desiInstall.install_dir, 0o2755)])
+        self.assertLog(-2, "os.chmod('%s', %s)" % (join(self.desiInstall.install_dir, 'lib'), 0o2755))
+        self.assertLog(-1, "os.chmod('%s', %s)" % (self.desiInstall.install_dir, 0o2755))
+
+    @patch('os.stat', replace_stat)
+    @patch('os.walk')
+    @patch('os.chmod')
+    def test_permissions_without_world(self, mock_chmod, mock_walk):
+        """Test the permission stage of the install, disabling world-read.
+        """
+        options = self.desiInstall.get_options(['--no-world', 'desiutil', '1.2.3'])
+        self.assertFalse(self.desiInstall.options.world)
         self.desiInstall.install_dir = join(self.data_dir, 'desiutil')
         self.desiInstall.is_branch = False
-        mock_proc = mock_popen()
-        mock_proc.returncode = 0
-        mock_proc.communicate.return_value = ('out', 'err')
-        status = self.desiInstall.permissions()
-        self.assertEqual(status, 0)
-        mock_popen.assert_has_calls([call(['fix_permissions.sh', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True),
-                                     call(['chmod', '-R', 'a-w', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True)],
-                                    any_order=True)
-        mock_popen.reset_mock()
-        options = self.desiInstall.get_options(['--test', 'desiutil', 'branches/main'])
-        status = self.desiInstall.permissions()
-        self.assertEqual(status, 0)
-        mock_popen.assert_has_calls([call(['fix_permissions.sh', '-t', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True),
-                                     call(['chmod', '-R', 'a-w', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True)],
-                                    any_order=True)
-        mock_popen.reset_mock()
-        options = self.desiInstall.get_options(['--verbose', 'desiutil', 'branches/main'])
-        status = self.desiInstall.permissions()
-        self.assertEqual(status, 0)
-        mock_popen.assert_has_calls([call(['fix_permissions.sh', '-v', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True),
-                                     call(['chmod', '-R', 'a-w', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True)],
-                                    any_order=True)
-        mock_popen.reset_mock()
-        options = self.desiInstall.get_options(['--verbose', 'desiutil', 'branches/main'])
+        mock_walk.return_value = iter([(join(self.desiInstall.install_dir, 'bin'), [], ['executable', 'README.txt']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), [], ['METADATA', 'LICENSE.rst']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), [], ['__init__.cpython-3.10.pyc', 'module.cpython-3.10.pyc']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), ['__pycache__'], ['__init__.py', 'module.py']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), ['desiutil-1.2.3.dist-info', 'desiutil'], []),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10'), ['site-packages'], []),
+                                       (join(self.desiInstall.install_dir, 'lib'), ['python3.10'], []),
+                                       (self.desiInstall.install_dir, ['bin', 'lib'], [])])
+        self.desiInstall.permissions()
+        mock_walk.assert_called_once_with(self.desiInstall.install_dir, topdown=False)
+        mock_chmod.assert_has_calls([call(join(self.desiInstall.install_dir, 'bin', 'executable'), 0o550),
+                                     call(join(self.desiInstall.install_dir, 'bin', 'README.txt'), 0o440),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'METADATA'), 0o440),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'LICENSE.rst'), 0o440),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', '__init__.cpython-3.10.pyc'), 0o440),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', 'module.cpython-3.10.pyc'), 0o440),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__init__.py'), 0o440),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', 'module.py'), 0o440),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), 0o2550),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), 0o2550),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), 0o2550),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), 0o2550),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10'), 0o2550),
+                                     call(join(self.desiInstall.install_dir, 'bin'), 0o2550),
+                                     call(join(self.desiInstall.install_dir, 'lib'), 0o2550),
+                                     call(self.desiInstall.install_dir, 0o2550)])
+        self.assertLog(-2, "os.chmod('%s', %s)" % (join(self.desiInstall.install_dir, 'lib'), 0o2550))
+        self.assertLog(-1, "os.chmod('%s', %s)" % (self.desiInstall.install_dir, 0o2550))
+
+    @patch('os.stat', replace_stat)
+    @patch('os.walk')
+    @patch('os.chmod')
+    def test_permissions_with_branch_without_world(self, mock_chmod, mock_walk):
+        """Test the permission stage of the install, on a branch, disabling world-read.
+        """
+        options = self.desiInstall.get_options(['--no-world', 'desiutil', 'branches/main'])
+        self.assertFalse(self.desiInstall.options.world)
+        self.desiInstall.install_dir = join(self.data_dir, 'desiutil')
         self.desiInstall.is_branch = True
-        status = self.desiInstall.permissions()
-        self.assertEqual(status, 0)
-        mock_popen.assert_has_calls([call(['fix_permissions.sh', '-v', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True),
-                                     call(['chmod', '-R', 'g-w,o-w', self.desiInstall.install_dir], stderr=-1, stdout=-1, universal_newlines=True)],
-                                    any_order=True)
+        mock_walk.return_value = iter([(join(self.desiInstall.install_dir, 'bin'), [], ['executable', 'README.txt']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), [], ['METADATA', 'LICENSE.rst']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), [], ['__init__.cpython-3.10.pyc', 'module.cpython-3.10.pyc']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), ['__pycache__'], ['__init__.py', 'module.py']),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), ['desiutil-1.2.3.dist-info', 'desiutil'], []),
+                                       (join(self.desiInstall.install_dir, 'lib', 'python3.10'), ['site-packages'], []),
+                                       (join(self.desiInstall.install_dir, 'lib'), ['python3.10'], []),
+                                       (self.desiInstall.install_dir, ['bin', 'lib'], [])])
+        self.desiInstall.permissions()
+        mock_walk.assert_called_once_with(self.desiInstall.install_dir, topdown=False)
+        mock_chmod.assert_has_calls([call(join(self.desiInstall.install_dir, 'bin', 'executable'), 0o750),
+                                     call(join(self.desiInstall.install_dir, 'bin', 'README.txt'), 0o640),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'METADATA'), 0o640),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info', 'LICENSE.rst'), 0o640),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', '__init__.cpython-3.10.pyc'), 0o640),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__', 'module.cpython-3.10.pyc'), 0o640),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__init__.py'), 0o640),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', 'module.py'), 0o640),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil', '__pycache__'), 0o2750),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil-1.2.3.dist-info'), 0o2750),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages', 'desiutil'), 0o2750),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10', 'site-packages'), 0o2750),
+                                     call(join(self.desiInstall.install_dir, 'lib', 'python3.10'), 0o2750),
+                                     call(join(self.desiInstall.install_dir, 'bin'), 0o2750),
+                                     call(join(self.desiInstall.install_dir, 'lib'), 0o2750),
+                                     call(self.desiInstall.install_dir, 0o2750)])
+        self.assertLog(-2, "os.chmod('%s', %s)" % (join(self.desiInstall.install_dir, 'lib'), 0o2750))
+        self.assertLog(-1, "os.chmod('%s', %s)" % (self.desiInstall.install_dir, 0o2750))
 
     @patch('desiutil.install.Popen')
     def test_unlock_permissions(self, mock_popen):
diff --git a/py/desiutil/test/test_modules.py b/py/desiutil/test/test_modules.py
index ec34b85..4844c19 100644
--- a/py/desiutil/test/test_modules.py
+++ b/py/desiutil/test/test_modules.py
@@ -291,6 +291,3 @@ class TestModules(unittest.TestCase):
         _write_module_data(p, 'This is a test.\n')
         self.assertEqual(S_IMODE(stat(p).st_mode), S_IRUSR | S_IRGRP | S_IROTH)
         remove(p)
-        _write_module_data(p, 'This is a test.\n', world=False)
-        self.assertEqual(S_IMODE(stat(p).st_mode), S_IRUSR | S_IRGRP)
-        remove(p)