Update package spec files to allow build option defaults using / nota…

…tion
spkenv · Mar 22, 2021 · 09c3aad · 09c3aad
1 parent 956882c
commit 09c3aad
Show file tree

Hide file tree

Showing 13 changed files with 74 additions and 72 deletions.
diff --git a/.spdev.yaml b/.spdev.yaml
@@ -3,6 +3,10 @@ release_notes: |
   * Update to latest spfs for improved logging and bugfixes
   * Update logging to use stderr instead of stdio, making it
     much easier to interact with processes launched through spk
+  * Enable the use of '/' notation for default values in build options
+    * This aligns the syntax across the whole file, creating better consistency
+    * The old 'default' field will still be supported for the time being, but
+      removed from the documentation and examples
 
 toolchain:
   - kind: Shell

diff --git a/docs/reference.md b/docs/reference.md
@@ -67,8 +67,7 @@ Variable options represents some arbitrary configuration parameter to the build.
 
 | Field       | Type        | Description                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | ----------- | ----------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| var         | _str_       | The name of the option                                                                                                                                                                                                                                                                                                                                                                                                            |
-| default     | _str_       | The default value for this option if not otherwise specified                                                                                                                                                                                                                                                                                                                                                                      |
+| var         | _str_       | The name of the option, with optional default value (eg `my_option` or `my_option/default_value`)                                                                                                                                                                                                                                                                                                                                                                                                            |
 | choices     | _List[str]_ | An optional set of possible values for this variable                                                                                                                                                                                                                                                                                                                                                                              |
 | inheritance | _str_       | Defines how this option is inherited by downstream packages. `Weak` is the default behaviour and does not influence downstream packages directly. `Strong` propagates this build option into every package that has this one in it's build environment while also adding an install requirement for this option. `StrongForBuildOnly` can be used to propagate this requirement as a build option but not an install requirement. |
 | static      | _str_       | Defines an unchangeable value for this variable - this is usually reserved for use by the system and is set when a package build is published to save the value of the variable at build time                                                                                                                                                                                                                                     |
@@ -79,8 +78,7 @@ Package options define a package that is required at build time.
 
 | Field            | Type                                    | Description                                                                                                                                                                                    |
 | ---------------- | --------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| pkg              | _str_                                   | The name of the package that is required                                                                                                                                                       |
-| default          | _str_                                   | The default requested version for this option if not otherwise specified                                                                                                                       |
+| pkg              | _str_                                   | The name of the package that is required, with optional default value (eg just `package_name`, or `package_name/1.4`)                                                                                                                                                       |
 | prereleasePolicy | _[PreReleasePolicy](#prereleasepolicy)_ | Defines how pre-release versions should be handled when resolving this request                                                                                                                 |
 | static           | _str_                                   | Defines an unchangeable value for this variable - this is usually reserved for use by the system and is set when a package build is published to save the version of the package at build time |
 

diff --git a/docs/spec.md b/docs/spec.md
@@ -18,7 +18,7 @@ pkg: my-package/1.0.0
 
 The optional `compat` field of a package specifies the compatibility between versions of this package. The compat field takes a version number, with each digit replaced by one or more characters denoting compatibility (`a` for api compatibility, `b` for binary compatbility and `x` for no compatibility). Multiple characters can be put together if necessary: `x.ab`.
 
-If not specified, the default value for this field is: `x.a.b`.
+If not specified, the default value for this field is: `x.a.b`. This means that at build time and on the command line, when API compatibility is needed, any minor version of this package can be considered compatible (eg `my-package/1.0.0` could resolve any `my-package/1.*`). When resolving dependencies however, when binary compatibility is needed, only the patch version is considered (eg `my-package/1.0.0` could resolve any `my-package/1.0.*`).
 
 ```yaml
 pkg: my-package/1.0.0
@@ -105,11 +105,9 @@ Build options are considered inputs to the build process. There are two types of
 ```yaml
 build:
   options:
-    - var: debug
-      default: off
+    - var: debug/off
       choices: [on, off]
-    - pkg: cmake
-      default: 3.16
+    - pkg: cmake/3.16
 ```
 
 All options that are declared in your package should be used in the build script, otherwise they are not relevant build options and your package may need rebuilding unnecessarily.
@@ -338,7 +336,7 @@ You can specfiy additional requirements for any defined test. These requirements
 ```yaml
 build:
   options:
-    - { pkg: python, default: 3 }
+    - pkg: python/3
 
 tests:
   - stage: install

diff --git a/examples/cmake/example.spk.yaml b/examples/cmake/example.spk.yaml
@@ -5,7 +5,7 @@ build:
   options:
   - var: os
   - var: arch
-  - {var: debug, default: off}
+  - var: debug/off
   - pkg: gcc
 
   variants:

diff --git a/examples/cmake/postrelease.yaml b/examples/cmake/postrelease.yaml
@@ -4,8 +4,7 @@ build:
   options:
     - var: os
     - var: arch
-    - var: debug
-      default: on
+    - var: debug/on
     - pkg: gcc
 
   variants:

diff --git a/examples/cmake/prerelease.spk.yaml b/examples/cmake/prerelease.spk.yaml
@@ -1,24 +1,22 @@
 pkg: cmake-example/0.2.0-pre.0
 
 build:
-
   options:
-  - var: os
-  - var: arch
-  - var: debug
-    default: on
-  - pkg: gcc
+    - var: os
+    - var: arch
+    - var: debug/on
+    - pkg: gcc
 
   variants:
-  - {gcc: 6.3, debug: on}
-  - {gcc: 4.8, debug: on}
+    - { gcc: 6.3, debug: on }
+    - { gcc: 4.8, debug: on }
 
   script:
-  - mkdir -p build
-  - cd build
-  - CONFIG=Release
-  - if [[ "${SPK_OPT_debug}" == "on" ]]; then CONFIG=Debug; fi
-  - cmake ..
+    - mkdir -p build
+    - cd build
+    - CONFIG=Release
+    - if [[ "${SPK_OPT_debug}" == "on" ]]; then CONFIG=Debug; fi
+    - cmake ..
       -DCMAKE_PREFIX_PATH=/spfs
       -DCMAKE_INSTALL_PREFIX=/spfs
-  - cmake --build . --target install
+    - cmake --build . --target install
diff --git a/self.spk.yaml b/self.spk.yaml
@@ -3,27 +3,25 @@ pkg: spk/0.1.0
 build:
   options:
     - var: arch
-    - var: os
-      choices: [linux]
-    - pkg: python
-      static: 3.7
+    - { var: os, choices: [linux] }
+    - { pkg: python, static: 3.7 }
     - pkg: pytest
     - pkg: mypy
     - { pkg: black, prereleasePolicy: IncludeAll }
-    - { pkg: nuitka, default: =0.6.7 }
+    - pkg: nuitka/=0.6.7
     - pkg: pytest-mypy
     - pkg: pytest-cov
-    - { pkg: spfs, default: 0.20.10 }
-    - { pkg: ruamel-yaml, default: 0.16.10 }
-    - { pkg: colorama, default: 0.4.3 }
-    - { pkg: typing-extensions, default: 3.7 }
-    - { pkg: simplejson, default: 3.17 }
-    - { pkg: semver, default: 2.9 }
-    - { pkg: structlog, default: 19.2 }
-    - { pkg: spops, default: 0.1.11 }
-    - { pkg: sortedcontainers, default: 2.1 }
-    - { pkg: pytz, default: ">=2020" }
-    - { pkg: cerberus, default: 1.3 }
+    - pkg: spfs/0.20.10
+    - pkg: ruamel-yaml/0.16.10
+    - pkg: colorama/0.4.3
+    - pkg: typing-extensions/3.7
+    - pkg: simplejson/3.17
+    - pkg: semver/2.9
+    - pkg: structlog/19.2
+    - pkg: spops/0.1.11
+    - pkg: sortedcontainers/2.1
+    - pkg: pytz/>=2020
+    - pkg: cerberus/1.3
   script:
     - export PYTHONDONTWRITEBYTECODE=1
     - /spfs/bin/python -m pip install --no-deps .

diff --git a/spk/api/_build_spec.py b/spk/api/_build_spec.py
@@ -5,7 +5,7 @@
 
 from ._request import Request, PkgRequest, parse_ident_range, PreReleasePolicy
 from ._option_map import OptionMap
-from ._name import validate_name
+from ._version import Version
 from ._compat import Compatibility, COMPATIBLE, CompatRule
 
 
@@ -281,10 +281,11 @@ def validate(self, value: Optional[str]) -> Compatibility:
 
     def to_dict(self) -> Dict[str, Any]:
 
-        spec: Dict[str, Any] = {"var": self.var}
+        var = self.var
         if self.default:
-            spec["default"] = self.default
+            var += "/" + self.default
 
+        spec: Dict[str, Any] = {"var": var}
         if self.choices:
             spec["choices"] = list(self.choices)
 
@@ -305,8 +306,14 @@ def from_dict(data: Dict[str, Any]) -> "VarOpt":
         except KeyError:
             raise ValueError("missing required key for VarOpt: var")
 
-        opt = VarOpt(var)
-        opt.default = str(data.pop("default", ""))
+        if "default" in data:
+            # the default field is deprecated, but we support it for existing packages
+            var += "/" + str(data.pop("default", ""))
+        if "/" not in var:
+            var += "/"
+
+        var, default = var.split("/", 1)
+        opt = VarOpt(var, default=default)
         opt.choices = set(str(c) for c in data.pop("choices", []))
         opt.set_value(str(data.pop("static", "")))
 
@@ -389,9 +396,11 @@ def to_request(self, given_value: str = None) -> Request:
 
     def to_dict(self) -> Dict[str, Any]:
 
-        spec = {"pkg": self.pkg}
+        pkg = self.pkg
         if self.default:
-            spec["default"] = self.default
+            pkg += "/" + self.default
+
+        spec = {"pkg": pkg}
         base_value = super(PkgOpt, self).get_value()
         if base_value:
             spec["static"] = base_value
@@ -407,14 +416,13 @@ def from_dict(data: Dict[str, Any]) -> "PkgOpt":
         except KeyError:
             raise ValueError("missing required key for PkgOpt: pkg")
 
-        if "/" in pkg:
-            raise ValueError(
-                "Build option for package cannot have version number, use 'default' field instead"
-            )
-        pkg = validate_name(pkg)
-
-        default = str(data.pop("default", ""))
-        opt = PkgOpt(pkg, default=default)
+        if "default" in data:
+            # the default field is deprecated but we support it for existing packages
+            pkg += "/" + data.pop("default")
+        pkg = parse_ident_range(pkg)
+        opt = PkgOpt(
+            pkg.name, default=str(pkg.version) if pkg.version != Version() else ""
+        )
 
         if "prereleasePolicy" in data:
             name = data.pop("prereleasePolicy")

diff --git a/spk/api/_build_spec_test.py b/spk/api/_build_spec_test.py
@@ -47,7 +47,7 @@ def test_variants_must_be_unique() -> None:
         # two variants end up resolving to the same set of options
         BuildSpec.from_dict(
             {
-                "options": [{"var": "my-opt", "default": "any-value"}],
+                "options": [{"var": "my-opt/any-value"}],
                 "variants": [{"my-opt": "any-value"}, {}],
             },
         )

diff --git a/spk/build/_binary_test.py b/spk/build/_binary_test.py
@@ -86,7 +86,7 @@ def test_build_package_pinning(tmprepo: storage.SpFSRepository) -> None:
                 "script": [
                     "touch /spfs/top-file",
                 ],
-                "options": [{"pkg": "dep", "default": "1.0.0"}],
+                "options": [{"pkg": "dep/1.0.0"}],
             },
             "install": {"requirements": [{"pkg": "dep", "fromBuildEnv": "~x.x"}]},
         }
@@ -136,7 +136,7 @@ def test_build_var_pinning(tmprepo: storage.SpFSRepository) -> None:
             "pkg": "dep/1.0.0",
             "build": {
                 "script": "touch /spfs/dep-file",
-                "options": [{"var": "depvar", "default": "depvalue"}],
+                "options": [{"var": "depvar/depvalue"}],
             },
         }
     )
@@ -148,8 +148,8 @@ def test_build_var_pinning(tmprepo: storage.SpFSRepository) -> None:
                     "touch /spfs/top-file",
                 ],
                 "options": [
-                    {"pkg": "dep", "default": "1.0.0"},
-                    {"var": "topvar", "default": "topvalue"},
+                    {"pkg": "dep/1.0.0"},
+                    {"var": "topvar/topvalue"},
                 ],
             },
             "install": {
@@ -246,9 +246,7 @@ def test_build_package_requirement_propagation(tmprepo: storage.SpFSRepository)
             "pkg": "base/1.0.0",
             "sources": [],
             "build": {
-                "options": [
-                    {"var": "inherited", "default": "val", "inheritance": "Strong"}
-                ],
+                "options": [{"var": "inherited/val", "inheritance": "Strong"}],
                 "script": "echo building...",
             },
         }

diff --git a/spk/cli/_cmd_new.py b/spk/cli/_cmd_new.py
@@ -37,8 +37,7 @@ def register(
     # pkg options request packages that need to be present
     # in the build environment. You can specify a version number
     # here as the default for when the option is not otherise specified
-    - pkg: python
-      default: 3
+    - pkg: python/3
 
   # variants declares the default set of variants to build and publish
   # using the spk build and make-* commands

diff --git a/spk/solve/_solver_test.py b/spk/solve/_solver_test.py
@@ -475,7 +475,7 @@ def test_solver_option_compatibility(solver: Union[Solver, legacy.Solver]) -> No
             "build": {
                 # favoritize 2.7, otherwise an option of python=2 doesn't actually
                 # exclude python 3 from being resolved
-                "options": [{"pkg": "python", "default": "~2.7"}],
+                "options": [{"pkg": "python/~2.7"}],
                 "variants": [{"python": "3.7"}, {"python": "2.7"}],
             },
         }
@@ -517,8 +517,8 @@ def test_solver_option_injection() -> None:
             "build": {
                 "options": [
                     {"pkg": "python"},
-                    {"var": "python.abi", "default": "cp27mu"},
-                    {"var": "debug", "default": "on"},
+                    {"var": "python.abi/cp27mu"},
+                    {"var": "debug/on"},
                     {"var": "special"},
                 ],
             },
@@ -527,7 +527,7 @@ def test_solver_option_injection() -> None:
     pybuild = make_build(
         {
             "pkg": "python/2.7.5",
-            "build": {"options": [{"var": "abi", "default": "cp27mu"}]},
+            "build": {"options": [{"var": "abi/cp27mu"}]},
         }
     )
     repo = make_repo([make_build(spec.to_dict(), [pybuild])])

diff --git a/spk/storage/_archive_test.py b/spk/storage/_archive_test.py
@@ -1,5 +1,6 @@
 from typing import List
 import py.path
+import getpass
 import tarfile
 
 from .. import build, api, storage
@@ -55,6 +56,7 @@ def test_archive_io(tmpdir: py.path.local) -> None:
         "payloads/Y7",
         "payloads/Y7/5TF2HNNMMJRXZ3HNUVXENZZZOWDQSIBJANAWF6J6WOFRVWEVPA====",
         "renders",
+        f"renders/{getpass.getuser()}",
         "tags",
         "tags/spk",
         "tags/spk/pkg",