[Bug Fix (and docs too)] : Do not select @develop version by default (#1933)

* This fixes a bug in concretization.  Before the recent change to the
algorithm, the intent was that the @develop version, although
"greater" than numberic versions, is never preferred BY DEFAULT over
numeric versions.

To test this... suppose you have a package with no `preferred=True` in
it, and nothing in `packages.yaml`, but with a `develop` version.  For
the sake of this example, I've hacked my `python/package.py` to work
this way.

Without bugfix (WRONG: user should never get develop by default):

```
  python@develop%clang@7.3.0-apple~tk~ucs4 arch=darwin-elcapitan-x86_64
      ...
```

With bugfix (RIGHT: largest numeric version selected):

```
  python@3.5.2%clang@7.3.0-apple~tk~ucs4 arch=darwin-elcapitan-x86_64
      ...
```

* Documented version selection in concretization algo.

* Fix typos

* flake8

lib/spack/docs/packaging_guide.rst

@@ -431,7 +431,7 @@ how to compare and sort numeric versions.
 Some Spack versions involve slight extensions of numeric syntax; for
 example, ``py-sphinx-rtd-theme@0.1.10a0``.  In this case, numbers are
 always considered to be "newer" than letters.  This is for consistency
-with `RPM <https://bugzilla.redhat.com/show_bug.cgi?id=50977>`.
+with `RPM <https://bugzilla.redhat.com/show_bug.cgi?id=50977>`_.
 
 Spack versions may also be arbitrary non-numeric strings; any string
 here will suffice; for example, ``@develop``, ``@master``, ``@local``.
@@ -458,6 +458,32 @@ The logic behind this sort order is two-fold:
    ``develop`` version to satisfy dependencies like ``depends_on(abc,
    when="@x.y.z:")``
 
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+Concretization Version Selection
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+When concretizing, many versions might match a user-supplied spec.
+For example, the spec ``python`` matches all available versions of the
+package ``python``.  Similarly, ``python@3:`` matches all versions of
+Python3.  Given a set of versions that match a spec, Spack
+concretization uses the following priorities to decide which one to
+use:
+
+#. If the user provided a list of versions in ``packages.yaml``, the
+   first matching version in that list will be used.
+
+#. If one or more versions is specified as ``preferred=True``, in
+   either ``packages.yaml`` or ``package.py``, the largest matching
+   version will be used.  ("Latest" is defined by the sort order
+   above).
+
+#. If no preferences in particular are specified in the package or in
+   ``packages.yaml``, then the largest matching non-develop version
+   will be used.  By avoiding ``@develop``, this prevents users from
+   accidentally installing a ``@develop`` version.
+
+#. If all else fails and ``@develop`` is the only matching version, it
+   will be used.
 
 ^^^^^^^^^^^^^
 Date Versions

lib/spack/spack/concretize.py

@@ -180,16 +180,32 @@ def concretize_version(self, spec):
             v for v in pkg.versions
             if any(v.satisfies(sv) for sv in spec.versions)]
 
+        # The keys below show the order of precedence of factors used
+        # to select a version when concretizing.  The item with
+        # the "largest" key will be selected.
+        #
+        # NOTE: When COMPARING VERSIONS, the '@develop' version is always
+        #       larger than other versions.  BUT when CONCRETIZING,
+        #       the largest NON-develop version is selected by
+        #       default.
         keys = [(
+            # ------- Special direction from the user
             # Respect order listed in packages.yaml
             yaml_index.get(v, -1),
+
             # The preferred=True flag (packages or packages.yaml or both?)
             pkg.versions.get(Version(v)).get('preferred', False),
-            # @develop special case
+
-            v.isdevelop(),
+            # ------- Regular case: use latest non-develop version by default.
-            # Numeric versions more preferred than non-numeric
+            # Avoid @develop version, which would otherwise be the "largest"
-            v.isnumeric(),
+            # in straight version comparisons
+            not v.isdevelop(),
+
             # Compare the version itself
+            # This includes the logic:
+            #    a) develop > everything (disabled by "not v.isdevelop() above)
+            #    b) numeric > non-numeric
+            #    c) Numeric or string comparison
             v) for v in unsorted_versions]
         keys.sort(reverse=True)