Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expand the templating language used by pinning #1024

Merged
merged 2 commits into from
May 16, 2024
Merged

Expand the templating language used by pinning #1024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4101815
Use nom parser for parsing a VersionFilter
jrray May 14, 2024
8894a64
Expand the templating language used by pinning
jrray May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 11 additions & 14 deletions crates/spk-schema/crates/foundation/src/version_range/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1344,21 +1344,18 @@ impl FromStr for VersionFilter {
type Err = Error;

fn from_str(range: &str) -> Result<Self> {
let mut out = VersionFilter::default();
if range.is_empty() {
return Ok(out);
}
for rule_str in range.split(VERSION_RANGE_SEP) {
if rule_str.is_empty() {
return Err(Error::String(format!(
"Empty segment not allowed in version range, got: {range}"
)));
}
let rule = VersionRange::from_str(rule_str)?;
out.rules.insert(rule);
}
use nom::combinator::all_consuming;
use nom::error::convert_error;

Ok(out)
all_consuming(parsing::version_range)(range)
.map(|(_, vr)| match vr {
VersionRange::Filter(f) => f,
vr => VersionFilter::single(vr),
})
.map_err(|err| match err {
nom::Err::Error(e) | nom::Err::Failure(e) => Error::String(convert_error(range, e)),
nom::Err::Incomplete(_) => unreachable!(),
})
}
}

Expand Down
65 changes: 60 additions & 5 deletions crates/spk-schema/crates/ident/src/request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -834,14 +834,69 @@ impl PkgRequest {
self.rendered_to_pkgrequest(rendered)
}
Some(pin) => {
let mut digits = pkg.version().parts.iter().chain(std::iter::repeat(&0));
enum ScannerMode {
Base,
Pre,
Post,
}
let mut scanner_mode = ScannerMode::Base;
let version = pkg.version();
let mut digits = version.parts.iter().chain(std::iter::repeat(&0));

let mut rendered = Vec::with_capacity(pin.len());
for char in pin.chars() {
if char == 'x' {
rendered.extend(digits.next().unwrap().to_string().chars());
} else {
rendered.push(char);
match (char, &scanner_mode) {
('x', ScannerMode::Base) => {
rendered.extend(digits.next().unwrap().to_string().chars());
}
('x', ScannerMode::Pre) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note I intentionally make it an error to try to use x in pre/post position because it is foolish to try to predict how many pre/post release components your build environment is going to end up with. One of the other new tokens is likely a better choice, or the recipe author can simply hard code versions instead of using a template if they are explicitly pinning the build requirement to some fixed version.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another pitfall is if there are multiple post-release components the iteration order may be not be the order expected, since it is alphabetical but still legal to write them out of sorted order.

Some even more complicated grammar is needed to handle weird edge cases but there is no user demand for this behavior. I don't think multiple post-release components have been used in practice.

return Err(Error::String(
"'x' in pre-release position not supported; try 'X' instead"
.to_string(),
));
}
('x', ScannerMode::Post) => {
return Err(Error::String(
"'x' in post-release position not supported; try 'X' instead"
.to_string(),
));
}
('X', ScannerMode::Pre) => {
rendered.extend(version.pre.to_string().chars());
}
('X', ScannerMode::Post) => {
rendered.extend(version.post.to_string().chars());
}
('v', ScannerMode::Base) => {
rendered.extend(version.base_normalized().chars());
}
('V', ScannerMode::Base) => {
rendered.extend(version.to_string().chars());
}
(x, _) => {
match x {
'-' => scanner_mode = ScannerMode::Pre,
'+' => scanner_mode = ScannerMode::Post,
_ => {}
};
rendered.push(x);
}
}
}

loop {
// Remove trailing '+', e.g., if `+X` was used but the package
// had no post-release components.
if rendered.last() == Some(&'+') {
rendered.pop();
continue;
}
// Similarly, remove any trailing '-'.
if rendered.last() == Some(&'-') {
rendered.pop();
continue;
}
break;
}

self.rendered_to_pkgrequest(rendered)
Expand Down
19 changes: 19 additions & 0 deletions crates/spk-schema/crates/ident/src/request_test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,25 @@ fn test_var_request_pinned_roundtrip() {
#[case("1.2.3.4.5", "API", "API:1.2.3.4.5")]
#[case("1.2.3", "API:x.x", "API:1.2")]
#[case("1.2.3", "true", "Binary:1.2.3")]
#[case::v_expands_into_base_version("1.2.3+r.1", "v", "1.2.3")]
#[case::capital_v_expands_into_full_version("1.2.3+r.1", "V", "1.2.3+r.1")]
#[case::capital_x_in_post_position_expands_all_post_releases(
"1.2.3+r.1,s.2",
"x.x+X",
"1.2+r.1,s.2"
)]
#[case::capital_x_in_post_position_with_no_actual_post_release("1.2.3", "x.x+X", "1.2")]
#[case::capital_x_in_pre_and_post_position_with_no_actual_post_release_expected_order(
"1.2.3", "x.x-X+X", "1.2"
)]
#[case::capital_x_in_pre_and_post_position_with_no_actual_post_release_unexpected_order(
"1.2.3", "x.x+X-X", "1.2"
)]
#[case::v_in_post_release_do_not_expand_to_version("1.2.3+v.1", "x.x.x+v.2", "1.2.3+v.2")]
#[should_panic]
#[case::x_in_pre_release_position_is_not_allowed("1.2.3-r.1", "x.x-x", "n/a")]
#[should_panic]
#[case::x_in_post_release_position_is_not_allowed("1.2.3+r.1", "x.x+x", "n/a")]
fn test_pkg_request_pin_rendering(
#[case] version: &str,
#[case] pin: &str,
Expand Down
45 changes: 39 additions & 6 deletions docs/ref/spec.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,12 +346,45 @@ A build option can be one of [VariableRequest](#variablerequest), or [PackageReq
#### PackageRequest

| Field | Type | Description |
| ------------------- | --------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| pkg | _[`RangeIdentifier`](#rangeidentifier)_ | Specifies a desired package, components and acceptable version range. |
| prereleasePolicy | _[PreReleasePolicy](#prereleasepolicy)_ | Defines how pre-release versions should be handled when resolving this request |
| inclusionPolicy | _[InclusionPolicy](#inclusionpolicy)_ | Defines when the requested package should be included in the environment |
| fromBuildEnv | _str_ or _bool_ | Either true, or a template to generate this request from using the version of the package that was resolved into the build environment. This template takes the form`x.x.x`, where any _x_ is replaced by digits in the version number. For example, if `python/2.7.5` is in the build environment, the template `~x.x` would become `~2.7`. The special values of `Binary` and `API` can be used to request a binary or api compatible package to the one in the build environment, respectively. For Example, if `mypkg/1.2.3.4` is in the build environment, the template `API` would become `API:1.2.3.4`. A value of `true` works the same as `Binary`. |
| ifPresentInBuildEnv | _bool_ | Either true or false; if true, then `fromBuildEnv` only applies if the package was present in the build environment. This allows different variants to have different runtime requirements. |
| ------------------- | --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| pkg | _[`RangeIdentifier`](#rangeidentifier)_ | Specifies a desired package, components and acceptable version range. |
| prereleasePolicy | _[PreReleasePolicy](#prereleasepolicy)_ | Defines how pre-release versions should be handled when resolving this request |
| inclusionPolicy | _[InclusionPolicy](#inclusionpolicy)_ | Defines when the requested package should be included in the environment |
| fromBuildEnv | _str_ or _bool_ | Either true, or a template to generate this request from using the version of the package that was resolved into the build environment. See [FromBuildEnvTemplate](#frombuildenvtemplate) for more information. |
| ifPresentInBuildEnv | _bool_ | Either true or false; if true, then `fromBuildEnv` only applies if the package was present in the build environment. This allows different variants to have different runtime requirements. |

##### FromBuildEnvTemplate
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do anchors work with deeper titles? I didn't see any existing ones that go deeper than 4 hashes.


This template takes the form of any valid version range expression, but any
_x_ characters that appear are replaced by digits in the version number. For
example, if `python/2.7.5` is in the build environment, the template `~x.x`
would become `~2.7`. The special values of `Binary` and `API` can be used to
request a binary or API compatible package to the one in the build environment,
respectively. For Example, if `mypkg/1.2.3.4` is in the build environment, the
template `API` would become `API:1.2.3.4`. A value of `true` works the same as
`Binary`.

###### Advanced Usage

Besides `x`, other characters are available:

- 'v' - Expands to the full base version of the package.
- 'V' - Expands to the full version of the package, including any pre/post
release information.
- 'X' - Expands all the pre or post release information, depending on its
position in the template. It's okay if the package does not have any pre or
post release components.

Examples:

If the target package is `python/3.9.5-alpha.1+post.1,hotfix.2`, then:

- `~x.x` -> `~3.9`
- `~v` -> `~3.9.5`
- `~V` -> `~3.9.5-alpha.1+post.1,hotfix.2`
- `~x.x-X` -> `~3.9-alpha.1`
- `~x.x+X` -> `~3.9+hotfix.2,post.1`
- `~x.x-X+X` -> `~3.9-alpha1+hotfix.2,post.1`

#### RangeIdentifier

Expand Down
Loading