Change Version formatting properties and functions to return Version objects (#4834)

* Change version.up_to() to return Version() object
* Add unit tests for Version.up_to()
* Fix packages that expected up_to() to return a string
* Ensure that up_to() preserves separator characters
* Use version indexing instead of up_to
* Make all Version formatting properties return Version objects
* Update docs
* Tests need to test string representation

lib/spack/docs/packaging_guide.rst

@@ -490,6 +490,21 @@ version.joined       123
    Python properties don't need parentheses. ``version.dashed`` is correct.
    ``version.dashed()`` is incorrect.
 
+In addition, these version properties can be combined with ``up_to()``.
+For example:
+
+.. code-block:: python
+
+   >>> version = Version('1.2.3')
+   >>> version.up_to(2).dashed
+   Version('1-2')
+   >>> version.underscored.up_to(2)
+   Version('1_2')
+
+
+As you can see, order is not important. Just keep in mind that ``up_to()`` and
+the other version properties return ``Version`` objects, not strings.
+
 If a URL cannot be derived systematically, or there is a special URL for one
 of its versions, you can add an explicit URL for a particular version:
 

lib/spack/spack/test/versions.py

@@ -204,6 +204,8 @@ def test_underscores():
     assert_ver_eq('2_0', '2_0')
     assert_ver_eq('2.0', '2_0')
     assert_ver_eq('2_0', '2.0')
+    assert_ver_eq('2-0', '2_0')
+    assert_ver_eq('2_0', '2-0')
 
 
 def test_rpm_oddities():
@@ -426,20 +428,56 @@ def test_satisfaction_with_lists():
 
 
 def test_formatted_strings():
-    versions = '1.2.3', '1_2_3', '1-2-3'
+    versions = (
+        '1.2.3b', '1_2_3b', '1-2-3b',
+        '1.2-3b', '1.2_3b', '1-2.3b',
+        '1-2_3b', '1_2.3b', '1_2-3b'
+    )
     for item in versions:
         v = Version(item)
-        assert v.dotted == '1.2.3'
-        assert v.dashed == '1-2-3'
-        assert v.underscored == '1_2_3'
-        assert v.joined == '123'
+        assert v.dotted.string == '1.2.3b'
+        assert v.dashed.string == '1-2-3b'
+        assert v.underscored.string == '1_2_3b'
+        assert v.joined.string == '123b'
+
+        assert v.dotted.dashed.string == '1-2-3b'
+        assert v.dotted.underscored.string == '1_2_3b'
+        assert v.dotted.dotted.string == '1.2.3b'
+        assert v.dotted.joined.string == '123b'
+
+
+def test_up_to():
+    v = Version('1.23-4_5b')
+
+    assert v.up_to(1).string == '1'
+    assert v.up_to(2).string == '1.23'
+    assert v.up_to(3).string == '1.23-4'
+    assert v.up_to(4).string == '1.23-4_5'
+    assert v.up_to(5).string == '1.23-4_5b'
+
+    assert v.up_to(-1).string == '1.23-4_5'
+    assert v.up_to(-2).string == '1.23-4'
+    assert v.up_to(-3).string == '1.23'
+    assert v.up_to(-4).string == '1'
+
+    assert v.up_to(2).dotted.string == '1.23'
+    assert v.up_to(2).dashed.string == '1-23'
+    assert v.up_to(2).underscored.string == '1_23'
+    assert v.up_to(2).joined.string == '123'
+
+    assert v.dotted.up_to(2).string == '1.23' == v.up_to(2).dotted.string
+    assert v.dashed.up_to(2).string == '1-23' == v.up_to(2).dashed.string
+    assert v.underscored.up_to(2).string == '1_23'
+    assert v.up_to(2).underscored.string == '1_23'
+
+    assert v.up_to(2).up_to(1).string == '1'
 
 
 def test_repr_and_str():
 
     def check_repr_and_str(vrs):
         a = Version(vrs)
-        assert repr(a) == 'Version(\'' + vrs + '\')'
+        assert repr(a) == "Version('" + vrs + "')"
         b = eval(repr(a))
         assert a == b
         assert str(a) == vrs
@@ -457,17 +495,17 @@ def test_get_item():
     b = a[0:2]
     assert isinstance(b, Version)
     assert b == Version('0.1')
-    assert repr(b) == 'Version(\'0.1\')'
+    assert repr(b) == "Version('0.1')"
     assert str(b) == '0.1'
     b = a[0:3]
     assert isinstance(b, Version)
     assert b == Version('0.1_2')
-    assert repr(b) == 'Version(\'0.1_2\')'
+    assert repr(b) == "Version('0.1_2')"
     assert str(b) == '0.1_2'
     b = a[1:]
     assert isinstance(b, Version)
     assert b == Version('1_2-3')
-    assert repr(b) == 'Version(\'1_2-3\')'
+    assert repr(b) == "Version('1_2-3')"
     assert str(b) == '1_2-3'
     # Raise TypeError on tuples
     with pytest.raises(TypeError):

lib/spack/spack/version.py

@@ -130,30 +130,90 @@ def __init__(self, string):
         self.version = tuple(int_if_int(seg) for seg in segments)
 
         # Store the separators from the original version string as well.
-        # last element of separators is ''
-        self.separators = tuple(re.split(segment_regex, string)[1:-1])
+        self.separators = tuple(re.split(segment_regex, string)[1:])
 
     @property
     def dotted(self):
-        return '.'.join(str(x) for x in self.version)
+        """The dotted representation of the version.
+
+        Example:
+        >>> version = Version('1-2-3b')
+        >>> version.dotted
+        Version('1.2.3b')
+
+        Returns:
+            Version: The version with separator characters replaced by dots
+        """
+        return Version(self.string.replace('-', '.').replace('_', '.'))
 
     @property
     def underscored(self):
-        return '_'.join(str(x) for x in self.version)
+        """The underscored representation of the version.
+
+        Example:
+        >>> version = Version('1.2.3b')
+        >>> version.underscored
+        Version('1_2_3b')
+
+        Returns:
+            Version: The version with separator characters replaced by
+                underscores
+        """
+        return Version(self.string.replace('.', '_').replace('-', '_'))
 
     @property
     def dashed(self):
-        return '-'.join(str(x) for x in self.version)
+        """The dashed representation of the version.
+
+        Example:
+        >>> version = Version('1.2.3b')
+        >>> version.dashed
+        Version('1-2-3b')
+
+        Returns:
+            Version: The version with separator characters replaced by dashes
+        """
+        return Version(self.string.replace('.', '-').replace('_', '-'))
 
     @property
     def joined(self):
-        return ''.join(str(x) for x in self.version)
+        """The joined representation of the version.
+
+        Example:
+        >>> version = Version('1.2.3b')
+        >>> version.joined
+        Version('123b')
+
+        Returns:
+            Version: The version with separator characters removed
+        """
+        return Version(
+            self.string.replace('.', '').replace('-', '').replace('_', ''))
 
     def up_to(self, index):
-        """Return a version string up to the specified component, exclusive.
-           e.g., if this is 10.8.2, self.up_to(2) will return '10.8'.
+        """The version up to the specified component.
+
+        Examples:
+        >>> version = Version('1.23-4b')
+        >>> version.up_to(1)
+        Version('1')
+        >>> version.up_to(2)
+        Version('1.23')
+        >>> version.up_to(3)
+        Version('1.23-4')
+        >>> version.up_to(4)
+        Version('1.23-4b')
+        >>> version.up_to(-1)
+        Version('1.23-4')
+        >>> version.up_to(-2)
+        Version('1.23')
+        >>> version.up_to(-3)
+        Version('1')
+
+        Returns:
+            Version: The first index components of the version
         """
-        return '.'.join(str(x) for x in self[:index])
+        return self[:index]
 
     def lowest(self):
         return self
@@ -204,11 +264,9 @@ def __getitem__(self, idx):
             return self.version[idx]
 
         elif isinstance(idx, slice):
-            # Currently len(self.separators) == len(self.version) - 1
-            extendend_separators = self.separators + ('',)
             string_arg = []
 
-            pairs = zip(self.version[idx], extendend_separators[idx])
+            pairs = zip(self.version[idx], self.separators[idx])
             for token, sep in pairs:
                 string_arg.append(str(token))
                 string_arg.append(str(sep))

var/spack/repos/builtin/packages/lua/package.py

@@ -145,11 +145,11 @@ def setup_environment(self, spack_env, run_env):
 
     @property
     def lua_lib_dir(self):
-        return os.path.join('lib', 'lua', self.version.up_to(2))
+        return os.path.join('lib', 'lua', str(self.version.up_to(2)))
 
     @property
     def lua_share_dir(self):
-        return os.path.join('share', 'lua', self.version.up_to(2))
+        return os.path.join('share', 'lua', str(self.version.up_to(2)))
 
     def setup_dependent_package(self, module, dependent_spec):
         """

var/spack/repos/builtin/packages/magics/package.py

@@ -109,7 +109,7 @@ def install(self, spec, prefix):
         if '+metview' in spec:
             if '+qt' in spec:
                 options.append('-DENABLE_METVIEW=ON')
-                if spec['qt'].version.up_to(1) == '5':
+                if spec['qt'].version[0] == 5:
                     options.append('-DENABLE_QT5=ON')
             else:
                 options.append('-DENABLE_METVIEW_NO_QT=ON')

var/spack/repos/builtin/packages/qt/package.py

@@ -131,9 +131,9 @@ def url_for_version(self, version):
         url = self.list_url
 
         if version >= Version('4.0'):
-            url += version.up_to(2) + '/'
+            url += str(version.up_to(2)) + '/'
         else:
-            url += version.up_to(1) + '/'
+            url += str(version.up_to(1)) + '/'
 
         if version >= Version('4.8'):
             url += str(version) + '/'

var/spack/repos/builtin/packages/sublime-text/package.py

@@ -49,7 +49,7 @@ class SublimeText(Package):
     depends_on('libxau', type='run')
 
     def url_for_version(self, version):
-        if version.up_to(1) == '2':
+        if version[0] == 2:
             return "https://download.sublimetext.com/Sublime%20Text%20{0}%20x64.tar.bz2".format(version)
         else:
             return "https://download.sublimetext.com/sublime_text_3_build_{0}_x64.tar.bz2".format(version)