Skip to content

Commit

Permalink
Fix the Engine version check in GUI (#6570)
Browse files Browse the repository at this point in the history
This PR fixes #6560.

The fix has a few elements:
1) Bumps the Engine requirement to the latest release, namely `2023.1.1`.
2) Changed the logic of checking whether a given version matches the requirement. Previously, we relied on `VersionReq` from `semver` crate which did not behave intuitively when the required version had a prerelease suffix. Now we rely directly on Semantic Versioning rules of precedence.
3) Code cleanups, including deduplicating 3 copies of the version-checking code, and moving some tests to more sensible places.
  • Loading branch information
mwu-tow authored May 8, 2023
1 parent 069fcf3 commit ee8e9e5
Show file tree
Hide file tree
Showing 11 changed files with 184 additions and 93 deletions.
120 changes: 66 additions & 54 deletions Cargo.lock

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -134,3 +134,5 @@ syn = { version = "1.0", features = [
"visit-mut",
] }
quote = { version = "1.0.23" }
semver = { version = "1.0.0", features = ["serde"] }
thiserror = "1.0.40"
2 changes: 1 addition & 1 deletion app/gui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ itertools = { workspace = true }
js-sys = { workspace = true }
mockall = { version = "0.7.1", features = ["nightly"] }
nalgebra = { workspace = true }
semver = { version = "1.0.0", features = ["serde"] }
semver = { workspace = true }
serde = { version = "1.0", features = ["derive"] }
serde_json = { workspace = true }
sha3 = { version = "0.8.2" }
Expand Down
4 changes: 2 additions & 2 deletions app/gui/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ minimumSupportedVersion": "2.0.0-alpha.6"

# The minimum engine version supported by the application. The projects opened with the older versions
# will have the "Unsupported engine version" message displayed.
engineVersionSupported: "2023.1.1-nightly.2023.2.8"
engineVersionSupported: "2023.1.1"

# The minimum language edition supported by the application. It will be displayed as edition user
# should put in their package.yaml file to have project compatible with the IDE.
languageEditionSupported: "2023.1.1-nightly.2023.2.8"
languageEditionSupported: "2023.1.1"
3 changes: 2 additions & 1 deletion app/gui/config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ edition = "2021"
ensogl = { path = "../../../lib/rust/ensogl" }
enso-prelude = { path = "../../../lib/rust/prelude" }
enso-json-to-struct = { path = "../../../lib/rust/json-to-struct" }
semver = "1.0.0"
semver = { workspace = true }
thiserror = { workspace = true }

[build-dependencies]
config-reader = { path = "../../../lib/rust/config-reader" }
99 changes: 96 additions & 3 deletions app/gui/config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,74 @@ use enso_json_to_struct::json_to_struct;


// ==============
// === Config ===
// === Errors ===
// ==============

///Error type with information that the Engine version does not meet the requirements.
#[derive(Clone, Debug, thiserror::Error)]
#[error("Unsupported Engine version: required {required} (or newer), found {found}.")]
pub struct UnsupportedEngineVersion {
/// The version of the Engine that is required.
pub required: semver::Version,
/// The version of the Engine that was found.
pub found: semver::Version,
}



// ===============
// === Version ===
// ===============

include!(concat!(env!("OUT_DIR"), "/config.rs"));

pub use generated::*;

pub fn engine_version_requirement() -> semver::VersionReq {
semver::VersionReq::parse(&format!(">={engine_version_supported}")).unwrap()
/// The minimum supported engine version.
pub fn engine_version_required() -> semver::Version {
// Safe to unwrap, as `engine_version_supported` compile-time and is validated by the test.
semver::Version::parse(engine_version_supported).unwrap()
}

/// Check if the given Engine version meets the requirements.
///
/// Effectively, this checks if the given version is greater or equal to the minimum supported.
/// "Greater or equal" is defined by the [Semantic Versioning specification](https://semver.org/)
/// term of precedence.
pub fn check_engine_version_requirement(
required_version: &semver::Version,
tested_version: &semver::Version,
) -> Result<(), UnsupportedEngineVersion> {
// We don't want to rely on the `semver::VersionReq` semantics here. Unfortunately the
// [Semantic Versioning specification](https://semver.org/) does not define the semantics of
// the version requirement operators, so different implementations may behave differently.
//
// The `semver::VersionReq` implementation follows the Cargo's implementation, namely:
// ```
// In particular, in order for any VersionReq to match a pre-release version, the VersionReq
// must contain at least one Comparator that has an explicit major, minor, and patch version
// identical to the pre-release being matched, and that has a nonempty pre-release component.
// ```
// See: https://docs.rs/semver/latest/semver/struct.VersionReq.html#associatedconstant.STAR
// This leads to counter-intuitive behavior, where `2023.0.0-dev` does not fulfill the
// `>= 2022.0.0-dev` requirement.
if tested_version < required_version {
Err(UnsupportedEngineVersion {
required: required_version.clone(),
found: tested_version.clone(),
})
} else {
Ok(())
}
}

/// Check if the given Engine version meets the requirements for this build.
///
/// See [`check_engine_version_requirement`] for more details.
pub fn check_engine_version(
engine_version: &semver::Version,
) -> Result<(), UnsupportedEngineVersion> {
check_engine_version_requirement(&engine_version_required(), engine_version)
}


Expand Down Expand Up @@ -64,3 +123,37 @@ pub fn read_args() -> Args {
lazy_static! {
pub static ref ARGS: Args = read_args();
}



// =============
// === Tests ===
// =============

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn check_that_version_requirement_parses() {
// We just expect that it won't panic.
let _ = engine_version_required();
}

#[test]
fn new_project_engine_version_fills_requirements() {
// Sanity check: required version must be supported.
assert!(check_engine_version(&engine_version_required()).is_ok());
}

#[test]
fn newer_prerelease_matches() -> anyhow::Result<()> {
// Whatever version we have currently defined with `-dev` prerelease.
let current =
semver::Version { pre: semver::Prerelease::new("dev")?, ..engine_version_required() };
let newer = semver::Version { major: current.major + 1, ..current.clone() };

check_engine_version_requirement(&current, &newer)?;
Ok(())
}
}
25 changes: 2 additions & 23 deletions app/gui/src/controller/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,16 +214,8 @@ impl Project {
}

fn display_warning_on_unsupported_engine_version(&self) {
let requirement = enso_config::engine_version_requirement();
let version = self.model.engine_version();
if !requirement.matches(&version) {
let message = format!(
"Unsupported Engine version. Please update edition in {} \
to {}.",
package_yaml_path(&self.model.name()),
enso_config::language_edition_supported
);
self.status_notifications.publish_event(message);
if let Err(e) = enso_config::check_engine_version(&self.model.engine_version()) {
self.status_notifications.publish_event(e.to_string());
}
}
}
Expand Down Expand Up @@ -273,19 +265,6 @@ mod tests {
use engine_protocol::language_server;
use std::assert_matches::assert_matches;

#[test]
fn parse_supported_engine_version() {
// Should not panic.
enso_config::engine_version_requirement();
}

#[test]
fn new_project_engine_version_fills_requirements() {
let requirements = enso_config::engine_version_requirement();
let version = semver::Version::parse(enso_config::engine_version_supported).unwrap();
assert!(requirements.matches(&version))
}

#[wasm_bindgen_test]
fn adding_missing_main() {
let _ctx = TestWithLocalPoolExecutor::set_up();
Expand Down
16 changes: 10 additions & 6 deletions app/gui/src/model/project/synchronized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,19 +240,23 @@ pub struct RenameInReadOnly;
/// engine's version (which is likely the cause of the problems).
#[derive(Debug, Fail)]
pub struct UnsupportedEngineVersion {
project_name: String,
root_cause: failure::Error,
project_name: String,
version_mismatch: enso_config::UnsupportedEngineVersion,
root_cause: failure::Error,
}

impl UnsupportedEngineVersion {
fn error_wrapper(properties: &Properties) -> impl Fn(failure::Error) -> failure::Error {
let engine_version = properties.engine_version.clone();
let project_name = properties.name.project.as_str().to_owned();
move |root_cause| {
let requirement = enso_config::engine_version_requirement();
if !requirement.matches(&engine_version) {
let project_name = project_name.clone();
UnsupportedEngineVersion { project_name, root_cause }.into()
if let Err(version_mismatch) = enso_config::check_engine_version(&engine_version) {
UnsupportedEngineVersion {
project_name: project_name.clone(),
version_mismatch,
root_cause,
}
.into()
} else {
root_cause
}
Expand Down
2 changes: 1 addition & 1 deletion build/build/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ regex = { workspace = true }
reqwest = { version = "0.11.5", default-features = false, features = [
"stream"
] }
semver = { version = "1.0.4", features = ["serde"] }
semver = { workspace = true }
serde = { version = "1.0.130", features = ["derive"] }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion build/ci_utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ regex = { workspace = true }
reqwest = { version = "0.11.5", default-features = false, features = [
"stream"
] }
semver = { version = "1.0.4", features = ["serde"] }
semver = { workspace = true }
serde = { version = "1.0.130", features = ["derive"] }
serde_json = { workspace = true }
serde_yaml = { workspace = true }
Expand Down
2 changes: 1 addition & 1 deletion lib/rust/ensogl/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ num_enum = { version = "0.5.1" }
num-traits = { version = "0.2" }
ordered-float = { workspace = true }
rustc-hash = { version = "1.0.1" }
semver = { version = "1.0.9" }
semver = { workspace = true }
serde = { version = "1" }
smallvec = { workspace = true }
typenum = { version = "1.11.2" }
Expand Down

0 comments on commit ee8e9e5

Please sign in to comment.