diff options
| -rw-r--r-- | etc/spack/defaults/config.yaml | 7 | ||||
| -rw-r--r-- | lib/spack/spack/hooks/sbang.py | 12 | ||||
| -rw-r--r-- | lib/spack/spack/schema/config.py | 3 | ||||
| -rw-r--r-- | lib/spack/spack/store.py | 143 | ||||
| -rw-r--r-- | lib/spack/spack/test/config.py | 72 | ||||
| -rw-r--r-- | lib/spack/spack/test/sbang.py | 7 | ||||
| -rw-r--r-- | lib/spack/spack/util/path.py | 50 |
7 files changed, 212 insertions, 82 deletions
diff --git a/etc/spack/defaults/config.yaml b/etc/spack/defaults/config.yaml index aa4ee2bbc6..6c5e8697b1 100644 --- a/etc/spack/defaults/config.yaml +++ b/etc/spack/defaults/config.yaml @@ -20,6 +20,13 @@ config: root: $spack/opt/spack projections: all: "${ARCHITECTURE}/${COMPILERNAME}-${COMPILERVER}/${PACKAGE}-${VERSION}-${HASH}" + # install_tree can include an optional padded length (int or boolean) + # default is False (do not pad) + # if padded_length is True, Spack will pad as close to the system max path + # length as possible + # if padded_length is an integer, Spack will pad to that many characters, + # assuming it is higher than the length of the install_tree root. + # padded_length: 128 # Locations where templates should be found diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py index d3a6174948..687cf651f4 100644 --- a/lib/spack/spack/hooks/sbang.py +++ b/lib/spack/spack/hooks/sbang.py @@ -16,7 +16,6 @@ import spack.modules import spack.paths import spack.store - #: OS-imposed character limit for shebang line: 127 for Linux; 511 for Mac. #: Different Linux distributions have different limits, but 127 is the #: smallest among all modern versions. @@ -28,7 +27,12 @@ else: def sbang_install_path(): """Location sbang should be installed within Spack's ``install_tree``.""" - return os.path.join(spack.store.layout.root, "bin", "sbang") + sbang_root = str(spack.store.unpadded_root) + install_path = os.path.join(sbang_root, "bin", "sbang") + if len(install_path) > shebang_limit: + raise SbangPathError( + 'Install tree root is too long. Spack cannot patch shebang lines.') + return install_path def sbang_shebang_line(): @@ -164,3 +168,7 @@ def post_install(spec): for directory, _, filenames in os.walk(spec.prefix): filter_shebangs_in_directory(directory, filenames) + + +class SbangPathError(spack.error.SpackError): + """Raised when the install tree root is too long for sbang to work.""" diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py index a21db3d048..7e422eb5b3 100644 --- a/lib/spack/spack/schema/config.py +++ b/lib/spack/spack/schema/config.py @@ -28,6 +28,9 @@ properties = { 'type': 'object', 'properties': union_dicts( {'root': {'type': 'string'}}, + {'padded_length': {'oneOf': [ + {'type': 'integer', 'minimum': 0}, + {'type': 'boolean'}]}}, spack.schema.projections.properties, ), }, diff --git a/lib/spack/spack/store.py b/lib/spack/spack/store.py index 75c3969362..779a5531a4 100644 --- a/lib/spack/spack/store.py +++ b/lib/spack/spack/store.py @@ -24,6 +24,7 @@ configuration. """ import os +import re import six import llnl.util.lang @@ -33,10 +34,100 @@ import spack.paths import spack.config import spack.util.path import spack.database -import spack.directory_layout as dir_layout +import spack.directory_layout + #: default installation root, relative to the Spack install path -default_root = os.path.join(spack.paths.opt_path, 'spack') +default_install_tree_root = os.path.join(spack.paths.opt_path, 'spack') + + +def parse_install_tree(config_dict): + """Parse config settings and return values relevant to the store object. + + Arguments: + config_dict (dict): dictionary of config values, as returned from + spack.config.get('config') + + Returns: + (tuple): triple of the install tree root, the unpadded install tree + root (before padding was applied), and the projections for the + install tree + + Encapsulate backwards compatibility capabilities for install_tree + and deprecated values that are now parsed as part of install_tree. + """ + # The following two configs are equivalent, the first being the old format + # and the second the new format. The new format is also more flexible. + + # config: + # install_tree: /path/to/root$padding:128 + # install_path_scheme: '{name}-{version}' + + # config: + # install_tree: + # root: /path/to/root + # padding: 128 + # projections: + # all: '{name}-{version}' + + install_tree = config_dict.get('install_tree', {}) + + padded_length = False + if isinstance(install_tree, six.string_types): + tty.warn("Using deprecated format for configuring install_tree") + unpadded_root = install_tree + unpadded_root = spack.util.path.canonicalize_path(unpadded_root) + # construct projection from previous values for backwards compatibility + all_projection = config_dict.get( + 'install_path_scheme', + spack.directory_layout.default_projections['all']) + + projections = {'all': all_projection} + else: + unpadded_root = install_tree.get('root', default_install_tree_root) + unpadded_root = spack.util.path.canonicalize_path(unpadded_root) + + padded_length = install_tree.get('padded_length', False) + if padded_length is True: + padded_length = spack.util.path.get_system_path_max() + padded_length -= spack.util.path.SPACK_MAX_INSTALL_PATH_LENGTH + + projections = install_tree.get( + 'projections', spack.directory_layout.default_projections) + + path_scheme = config_dict.get('install_path_scheme', None) + if path_scheme: + tty.warn("Deprecated config value 'install_path_scheme' ignored" + " when using new install_tree syntax") + + # Handle backwards compatibility for padding + old_pad = re.search(r'\$padding(:\d+)?|\${padding(:\d+)?}', unpadded_root) + if old_pad: + if padded_length: + msg = "Ignoring deprecated padding option in install_tree root " + msg += "because new syntax padding is present." + tty.warn(msg) + else: + unpadded_root = unpadded_root.replace(old_pad.group(0), '') + if old_pad.group(1) or old_pad.group(2): + length_group = 2 if '{' in old_pad.group(0) else 1 + padded_length = int(old_pad.group(length_group)[1:]) + else: + padded_length = spack.util.path.get_system_path_max() + padded_length -= spack.util.path.SPACK_MAX_INSTALL_PATH_LENGTH + + unpadded_root = unpadded_root.rstrip(os.path.sep) + + if padded_length: + root = spack.util.path.add_padding(unpadded_root, padded_length) + if len(root) != padded_length: + msg = "Cannot pad %s to %s characters." % (root, padded_length) + msg += " It is already %s characters long" % len(root) + tty.warn(msg) + else: + root = unpadded_root + + return (root, unpadded_root, projections) class Store(object): @@ -52,17 +143,23 @@ class Store(object): Args: root (str): path to the root of the install tree + unpadded_root (str): path to the root of the install tree without + padding; the sbang script has to be installed here to work with + padded roots path_scheme (str): expression according to guidelines in ``spack.util.path`` that describes how to construct a path to a package prefix in this store hash_length (int): length of the hashes used in the directory layout; spec hash suffixes will be truncated to this length """ - def __init__(self, root, projections=None, hash_length=None): + def __init__( + self, root, unpadded_root=None, projections=None, hash_length=None + ): self.root = root + self.unpadded_root = unpadded_root or root self.db = spack.database.Database( root, upstream_dbs=retrieve_upstream_dbs()) - self.layout = dir_layout.YamlDirectoryLayout( + self.layout = spack.directory_layout.YamlDirectoryLayout( root, projections=projections, hash_length=hash_length) def reindex(self): @@ -72,32 +169,13 @@ class Store(object): def _store(): """Get the singleton store instance.""" - install_tree = spack.config.get('config:install_tree', {}) - - if isinstance(install_tree, six.string_types): - tty.warn("Using deprecated format for configuring install_tree") - root = install_tree - - # construct projection from previous values for backwards compatibility - all_projection = spack.config.get( - 'config:install_path_scheme', - dir_layout.default_projections['all']) - - projections = {'all': all_projection} - else: - root = install_tree.get('root', default_root) - root = spack.util.path.canonicalize_path(root) - - projections = install_tree.get( - 'projections', dir_layout.default_projections) - - path_scheme = spack.config.get('config:install_path_scheme', None) - if path_scheme: - tty.warn("Deprecated config value 'install_path_scheme' ignored" - " when using new install_tree syntax") - - return Store(root, projections, - spack.config.get('config:install_hash_length')) + config_dict = spack.config.get('config') + root, unpadded_root, projections = parse_install_tree(config_dict) + hash_length = spack.config.get('config:install_hash_length') + return Store(root=root, + unpadded_root=unpadded_root, + projections=projections, + hash_length=hash_length) #: Singleton store instance @@ -108,6 +186,10 @@ def _store_root(): return store.root +def _store_unpadded_root(): + return store.unpadded_root + + def _store_db(): return store.db @@ -118,6 +200,7 @@ def _store_layout(): # convenience accessors for parts of the singleton store root = llnl.util.lang.LazyReference(_store_root) +unpadded_root = llnl.util.lang.LazyReference(_store_unpadded_root) db = llnl.util.lang.LazyReference(_store_db) layout = llnl.util.lang.LazyReference(_store_layout) diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index b852f4777e..f1081be737 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -358,28 +358,56 @@ def test_substitute_tempdir(mock_low_high_config): ) -def test_substitute_padding(mock_low_high_config): - max_system_path = spack_path.get_system_path_max() - expected_length = (max_system_path - - spack_path.SPACK_MAX_INSTALL_PATH_LENGTH) - - install_path = spack_path.canonicalize_path('/foo/bar/${padding}/baz') - - assert spack_path.SPACK_PATH_PADDING_CHARS in install_path - assert len(install_path) == expected_length - - install_path = spack_path.canonicalize_path('/foo/bar/baz/gah/$padding') - - assert spack_path.SPACK_PATH_PADDING_CHARS in install_path - assert len(install_path) == expected_length - - i_path = spack_path.canonicalize_path('/foo/$padding:10') - i_expect = os.path.join('/foo', spack_path.SPACK_PATH_PADDING_CHARS[:10]) - assert i_path == i_expect - - i_path = spack_path.canonicalize_path('/foo/${padding:20}') - i_expect = os.path.join('/foo', spack_path.SPACK_PATH_PADDING_CHARS[:20]) - assert i_path == i_expect +PAD_STRING = spack.util.path.SPACK_PATH_PADDING_CHARS +MAX_PATH_LEN = spack.util.path.get_system_path_max() +MAX_PADDED_LEN = MAX_PATH_LEN - spack.util.path.SPACK_MAX_INSTALL_PATH_LENGTH +reps = [PAD_STRING for _ in range((MAX_PADDED_LEN // len(PAD_STRING) + 1) + 2)] +full_padded_string = os.path.join( + '/path', os.path.sep.join(reps))[:MAX_PADDED_LEN] + + +@pytest.mark.parametrize('config_settings,expected', [ + ([], [None, None, None]), + ([['config:install_tree:root', '/path']], ['/path', None, None]), + ([['config:install_tree', '/path']], ['/path', None, None]), + ([['config:install_tree:projections', {'all': '{name}'}]], + [None, None, {'all': '{name}'}]), + ([['config:install_path_scheme', '{name}']], + [None, None, {'all': '{name}'}]), + ([['config:install_tree:root', '/path'], + ['config:install_tree:padded_length', 11]], + [os.path.join('/path', PAD_STRING[:5]), '/path', None]), + ([['config:install_tree:root', '/path/$padding:11']], + [os.path.join('/path', PAD_STRING[:5]), '/path', None]), + ([['config:install_tree', '/path/${padding:11}']], + [os.path.join('/path', PAD_STRING[:5]), '/path', None]), + ([['config:install_tree:padded_length', False]], [None, None, None]), + ([['config:install_tree:padded_length', True], + ['config:install_tree:root', '/path']], + [full_padded_string, '/path', None]), + ([['config:install_tree:', '/path$padding']], + [full_padded_string, '/path', None]), + ([['config:install_tree:', '/path/${padding}']], + [full_padded_string, '/path', None]), +]) +def test_parse_install_tree(config_settings, expected, mutable_config): + expected_root = expected[0] or spack.store.default_install_tree_root + expected_unpadded_root = expected[1] or expected_root + expected_proj = expected[2] or spack.directory_layout.default_projections + + # config settings is a list of 2-element lists, [path, value] + # where path is a config path and value is the value to set at that path + # these can be "splatted" in as the arguments to config.set + for config_setting in config_settings: + mutable_config.set(*config_setting) + + config_dict = mutable_config.get('config') + root, unpadded_root, projections = spack.store.parse_install_tree( + config_dict) + + assert root == expected_root + assert unpadded_root == expected_unpadded_root + assert projections == expected_proj def test_read_config(mock_low_high_config, write_config_file): diff --git a/lib/spack/spack/test/sbang.py b/lib/spack/spack/test/sbang.py index ca7d2a6fe6..9307d2bc4b 100644 --- a/lib/spack/spack/test/sbang.py +++ b/lib/spack/spack/test/sbang.py @@ -40,7 +40,8 @@ php_in_text = ("line\n") * 100 + "php\n" + ("line\n" * 100) php_line_patched = "<?php #!/this/" + ('x' * too_long) + "/is/php\n" php_line_patched2 = "?>\n" -last_line = "last!\n" +sbang_line = '#!/bin/sh %s/bin/sbang\n' % spack.store.store.unpadded_root +last_line = "last!\n" @pytest.fixture @@ -191,7 +192,7 @@ def test_shebang_handles_non_writable_files(script_dir, sbang_line): def check_sbang_installation(): sbang_path = sbang.sbang_install_path() sbang_bin_dir = os.path.dirname(sbang_path) - assert sbang_path.startswith(spack.store.layout.root) + assert sbang_path.startswith(spack.store.store.unpadded_root) assert os.path.exists(sbang_path) assert fs.is_exe(sbang_path) @@ -207,7 +208,7 @@ def test_install_sbang(install_mockery): sbang_path = sbang.sbang_install_path() sbang_bin_dir = os.path.dirname(sbang_path) - assert sbang_path.startswith(spack.store.layout.root) + assert sbang_path.startswith(spack.store.store.unpadded_root) assert not os.path.exists(sbang_bin_dir) sbang.install_sbang() diff --git a/lib/spack/spack/util/path.py b/lib/spack/spack/util/path.py index 8bcf598882..182f93c585 100644 --- a/lib/spack/spack/util/path.py +++ b/lib/spack/spack/util/path.py @@ -103,36 +103,36 @@ def _get_padding_string(length): return os.path.sep.join(reps_list) -def _add_computed_padding(path): - """Subtitute in padding of os-specific length. The intent is to leave - SPACK_MAX_INSTALL_PATH_LENGTH characters available for parts of the - path generated by spack. This is to allow for not-completely-known - lengths of things like os/arch, compiler, package name, hash length, - etc. - """ - padding_regex = re.compile(r'(\$[\w\d\:]+\b|\$\{[\w\d\:]+\})') - m = padding_regex.search(path) - if m and m.group(0).strip('${}').startswith('padding'): - padding_part = m.group(0) - len_pad_part = len(m.group(0)) - p_match = re.search(r'\:(\d+)', padding_part) - if p_match: - computed_padding = _get_padding_string(int(p_match.group(1))) - else: - # Take whatever has been computed/substituted so far and add some - # room - path_len = len(path) - len_pad_part + SPACK_MAX_INSTALL_PATH_LENGTH - system_max_path = get_system_path_max() - needed_pad_len = system_max_path - path_len - computed_padding = _get_padding_string(needed_pad_len) - return padding_regex.sub(computed_padding, path) - return path +def add_padding(path, length): + """Add padding subdirectories to path until total is length characters + + Returns the padded path. If path is length - 1 or more characters long, + returns path. If path is length - 1 characters, warns that it is not + padding to length + + Assumes path does not have a trailing path separator""" + padding_length = length - len(path) + if padding_length == 1: + # The only 1 character addition we can make to a path is `/` + # Spack internally runs normpath, so `foo/` will be reduced to `foo` + # Even if we removed this behavior from Spack, the user could normalize + # the path, removing the additional `/`. + # Because we can't expect one character of padding to show up in the + # resulting binaries, we warn the user and do not pad by a single char + tty.warn("Cannot pad path by exactly one character.") + if padding_length <= 0: + return path + + # we subtract 1 from the padding_length to account for the path separator + # coming from os.path.join below + padding = _get_padding_string(padding_length - 1) + + return os.path.join(path, padding) def canonicalize_path(path): """Same as substitute_path_variables, but also take absolute path.""" path = substitute_path_variables(path) path = os.path.abspath(path) - path = _add_computed_padding(path) return path |