From ee8e9e5c60755c407d404d4bb439d33bdae338f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Wawrzyniec=20Urba=C5=84czyk?= Date: Mon, 8 May 2023 16:13:53 +0200 Subject: [PATCH] Fix the Engine version check in GUI (#6570) 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. --- Cargo.lock | 120 ++++++++++++---------- Cargo.toml | 2 + app/gui/Cargo.toml | 2 +- app/gui/config.yaml | 4 +- app/gui/config/Cargo.toml | 3 +- app/gui/config/src/lib.rs | 99 +++++++++++++++++- app/gui/src/controller/project.rs | 25 +---- app/gui/src/model/project/synchronized.rs | 16 +-- build/build/Cargo.toml | 2 +- build/ci_utils/Cargo.toml | 2 +- lib/rust/ensogl/core/Cargo.toml | 2 +- 11 files changed, 184 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7dbc0c47165..97367fa5e9a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -148,7 +148,7 @@ dependencies = [ "enso-macro-utils", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -278,7 +278,7 @@ checksum = "10f203db73a71dfa2fb6dd22763990fa26f3d2625a6da2da900d23b87d26be27" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -295,7 +295,7 @@ checksum = "1cd7fce9ba8c3c042128ce72d8b2ddbf3a05747efb67ea0313c635e10bda47a2" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -961,7 +961,7 @@ dependencies = [ "cached_proc_macro_types", "darling", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1055,7 +1055,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1399,7 +1399,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6d2301688392eb071b0bf1a37be05c469d3cc4dbbd95df672fe28ab021e6a096" dependencies = [ "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1426,7 +1426,7 @@ dependencies = [ "proc-macro2", "quote", "scratch", - "syn", + "syn 1.0.107", ] [[package]] @@ -1443,7 +1443,7 @@ checksum = "357f40d1f06a24b60ae1fe122542c1fb05d28d32acb2aed064e84bc2ad1e252e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1467,7 +1467,7 @@ dependencies = [ "proc-macro2", "quote", "strsim", - "syn", + "syn 1.0.107", ] [[package]] @@ -1478,7 +1478,7 @@ checksum = "9c972679f83bdf9c42bd905396b6c3588a843a17f0f16dfcfa3e2c5d57441835" dependencies = [ "darling_core", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1658,7 +1658,7 @@ checksum = "fcc3dd5e9e9c0b295d6e1e4d811fb6f157d5ffd784b8d202fc62eac8035a770b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -1671,7 +1671,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version 0.4.0", - "syn", + "syn 1.0.107", ] [[package]] @@ -1962,7 +1962,7 @@ dependencies = [ "enso-build-macros-lib", "itertools", "proc-macro2", - "syn", + "syn 1.0.107", ] [[package]] @@ -1978,7 +1978,7 @@ dependencies = [ "quote", "regex", "serde_yaml", - "syn", + "syn 1.0.107", ] [[package]] @@ -2019,6 +2019,7 @@ dependencies = [ "enso-prelude", "ensogl", "semver 1.0.16", + "thiserror", ] [[package]] @@ -2218,7 +2219,7 @@ dependencies = [ "Inflector", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -2227,7 +2228,7 @@ version = "0.2.0" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -2323,7 +2324,7 @@ dependencies = [ "enso-macro-utils", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -2416,7 +2417,7 @@ dependencies = [ "Inflector", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -2434,7 +2435,7 @@ version = "0.1.0" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -2462,7 +2463,7 @@ dependencies = [ "paste", "proc-macro2", "quote", - "syn", + "syn 1.0.107", "wasm-bindgen-test", ] @@ -2634,7 +2635,7 @@ version = "0.1.0" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -3266,7 +3267,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -3327,7 +3328,7 @@ checksum = "aa4da3c766cd7a0db8242e326e9e4e081edd567072893ed320008189715366a4" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", "synstructure", ] @@ -3426,7 +3427,7 @@ checksum = "236b4e4ae2b8be5f7a5652f6108c4a0f2627c569db4e7923333d31c7dbfed0fb" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -3558,7 +3559,7 @@ checksum = "95a73af87da33b5acf53acfebdc339fe592ecf5357ac7c0a7734ab9d8c876a70" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -3730,7 +3731,7 @@ dependencies = [ "quote", "serde", "serde_json", - "syn", + "syn 1.0.107", ] [[package]] @@ -3741,7 +3742,7 @@ checksum = "a755cc59cda2641ea3037b4f9f7ef40471c329f55c1fa2db6fa0bb7ae6c1f7ce" dependencies = [ "graphql_client_codegen", "proc-macro2", - "syn", + "syn 1.0.107", ] [[package]] @@ -4149,7 +4150,7 @@ dependencies = [ "sha2", "strum", "symlink", - "syn", + "syn 1.0.107", "sysinfo", "tar", "tempfile", @@ -4653,7 +4654,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -4783,7 +4784,7 @@ dependencies = [ "cfg-if 0.1.10", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -5025,7 +5026,7 @@ dependencies = [ "proc-macro-crate", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -5107,7 +5108,7 @@ checksum = "b501e44f11665960c7e7fcf062c7d96a14ade4aa98116c004b2e37b5be7d736c" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -5164,7 +5165,7 @@ dependencies = [ "proc-macro-error", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -5332,7 +5333,7 @@ dependencies = [ "pest_meta", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -5363,7 +5364,7 @@ checksum = "069bdb1e05adc7a8990dce9cc75370895fbe4e3d58b9b73bf1aee56359344a55" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -5511,7 +5512,7 @@ dependencies = [ "proc-macro-error-attr", "proc-macro2", "quote", - "syn", + "syn 1.0.107", "version_check", ] @@ -5534,9 +5535,9 @@ checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" [[package]] name = "proc-macro2" -version = "1.0.50" +version = "1.0.56" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6ef7d57beacfaf2d8aee5937dab7b7f28de3cb8b1828479bb5de2a7106f2bae2" +checksum = "2b63bdb0cd06f1f4dedf69b254734f9b45af66e4a031e42a7480257d9898b435" dependencies = [ "unicode-ident", ] @@ -5561,7 +5562,7 @@ dependencies = [ "itertools", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -5603,9 +5604,9 @@ checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" [[package]] name = "quote" -version = "1.0.23" +version = "1.0.26" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8856d8364d252a14d474036ea1358d63c9e6965c8e5c1885c18f73d70bff9c7b" +checksum = "4424af4bf778aae2051a77b60283332f386554255d722233d09fbfc7e30da2fc" dependencies = [ "proc-macro2", ] @@ -6137,7 +6138,7 @@ checksum = "af487d118eecd09402d70a5d72551860e788df87b464af30e5ea6a38c75c541e" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -6343,7 +6344,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -6431,7 +6432,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn", + "syn 1.0.107", ] [[package]] @@ -6451,6 +6452,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "2.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a34fcf3e8b60f57e6a14301a2e916d323af98b0ea63c599441eec8558660c822" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "sync_wrapper" version = "0.1.1" @@ -6465,7 +6477,7 @@ checksum = "f36bdaa60a83aca3921b5259d5400cbf5e90fc51931376a9bd4a0eb79aa7210f" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", "unicode-xid", ] @@ -6554,22 +6566,22 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.38" +version = "1.0.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6a9cd18aa97d5c45c6603caea1da6628790b37f7a34b6ca89522331c5180fed0" +checksum = "978c9a314bd8dc99be594bc3c175faaa9794be04a5a5e153caba6915336cebac" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.38" +version = "1.0.40" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1fb327af4685e4d03fa8cbcf1716380da910eeb2bb8be417e7f9fd3fb164f36f" +checksum = "f9456a42c5b0d803c8cd86e73dd7cc9edd429499f37a3550d286d5e86720569f" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.15", ] [[package]] @@ -6683,7 +6695,7 @@ checksum = "d266c00fde287f55d3f1c3e96c500c362a2b8c695076ec180f27918820bc6df8" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -6889,7 +6901,7 @@ checksum = "4017f8f45139870ca7e672686113917c71c7a6e02d4924eda67186083c03081a" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", ] [[package]] @@ -7281,7 +7293,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 1.0.107", "wasm-bindgen-shared", ] @@ -7315,7 +7327,7 @@ checksum = "2aff81306fcac3c7515ad4e177f521b5c9a15f2b08f4e32d823066102f35a5f6" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 1.0.107", "wasm-bindgen-backend", "wasm-bindgen-shared", ] diff --git a/Cargo.toml b/Cargo.toml index b22bd852415..e00b7ac0e0e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -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" diff --git a/app/gui/Cargo.toml b/app/gui/Cargo.toml index 5a57c3454fc..500fe57b4ad 100644 --- a/app/gui/Cargo.toml +++ b/app/gui/Cargo.toml @@ -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" } diff --git a/app/gui/config.yaml b/app/gui/config.yaml index e6ca0802fd4..08ece4c7e1b 100644 --- a/app/gui/config.yaml +++ b/app/gui/config.yaml @@ -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" diff --git a/app/gui/config/Cargo.toml b/app/gui/config/Cargo.toml index 6b574e3c7da..bb4bc0343e6 100644 --- a/app/gui/config/Cargo.toml +++ b/app/gui/config/Cargo.toml @@ -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" } diff --git a/app/gui/config/src/lib.rs b/app/gui/config/src/lib.rs index ca71959078b..b01b31c494b 100644 --- a/app/gui/config/src/lib.rs +++ b/app/gui/config/src/lib.rs @@ -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) } @@ -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(¤t, &newer)?; + Ok(()) + } +} diff --git a/app/gui/src/controller/project.rs b/app/gui/src/controller/project.rs index 6c9c205b754..e78384ead0c 100644 --- a/app/gui/src/controller/project.rs +++ b/app/gui/src/controller/project.rs @@ -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()); } } } @@ -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(); diff --git a/app/gui/src/model/project/synchronized.rs b/app/gui/src/model/project/synchronized.rs index a9c51a071f4..62e92a6ba24 100644 --- a/app/gui/src/model/project/synchronized.rs +++ b/app/gui/src/model/project/synchronized.rs @@ -240,8 +240,9 @@ 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 { @@ -249,10 +250,13 @@ impl UnsupportedEngineVersion { 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 } diff --git a/build/build/Cargo.toml b/build/build/Cargo.toml index e621630144d..f6c2bc623d7 100644 --- a/build/build/Cargo.toml +++ b/build/build/Cargo.toml @@ -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 } diff --git a/build/ci_utils/Cargo.toml b/build/ci_utils/Cargo.toml index 4ec09d4079c..3180a18f567 100644 --- a/build/ci_utils/Cargo.toml +++ b/build/ci_utils/Cargo.toml @@ -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 } diff --git a/lib/rust/ensogl/core/Cargo.toml b/lib/rust/ensogl/core/Cargo.toml index b6b5648be8b..3311e4167c2 100644 --- a/lib/rust/ensogl/core/Cargo.toml +++ b/lib/rust/ensogl/core/Cargo.toml @@ -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" }