From 09c3aad9bdd52e0c0a4db866e1ccc7e2f20fd1e5 Mon Sep 17 00:00:00 2001 From: Ryan Bottriell Date: Mon, 22 Mar 2021 14:36:17 -0700 Subject: [PATCH] Update package spec files to allow build option defaults using / notation --- .spdev.yaml | 4 ++++ docs/reference.md | 6 ++--- docs/spec.md | 10 ++++---- examples/cmake/example.spk.yaml | 2 +- examples/cmake/postrelease.yaml | 3 +-- examples/cmake/prerelease.spk.yaml | 26 ++++++++++---------- self.spk.yaml | 30 +++++++++++------------ spk/api/_build_spec.py | 38 ++++++++++++++++++------------ spk/api/_build_spec_test.py | 2 +- spk/build/_binary_test.py | 12 ++++------ spk/cli/_cmd_new.py | 3 +-- spk/solve/_solver_test.py | 8 +++---- spk/storage/_archive_test.py | 2 ++ 13 files changed, 74 insertions(+), 72 deletions(-) diff --git a/.spdev.yaml b/.spdev.yaml index 3e78cb7b11..5fd747db7d 100644 --- 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 index dbf1752b39..6c48190bf2 100644 --- 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 index bd211b010a..5379d3bce3 100644 --- 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 index c98bfae779..60b7d044c9 100644 --- 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 index 0cec74cdcb..ec0c954674 100644 --- 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 index 4577a96307..d77d61c366 100644 --- 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 index 957d27742e..715883b4c2 100644 --- 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 index 4e0ac4e2d1..ee91d81c0c 100644 --- 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 index 2ef4748f7f..0302c01235 100644 --- 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 index 33870b9ae4..66687ab42c 100644 --- 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 index ee9421d34e..a42796a9bf 100644 --- 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 index fc9604c1ca..982b901f92 100644 --- 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 index fc3d4cdcb9..d2143330ad 100644 --- 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",