From 2011757872a8ef38a93d6d8a1b3cc70e03f93330 Mon Sep 17 00:00:00 2001 From: Eric Ridge Date: Mon, 31 Jul 2023 20:08:32 -0400 Subject: [PATCH] Cleanup the error when cargo-pgrx version doesn't match Cargo.toml (#1240) As was discussed in the comments in another issue: https://github.com/pgcentralfoundation/pgrx/issues/1076#issuecomment-1656213886 the error message when `cargo-pgrx`'s version doesn't match the pgrx crate dependency version is confusing. This cleans it up and also tells you which Cargo.toml file it is. --------- Co-authored-by: Jubilee <46493976+workingjubilee@users.noreply.github.com> Co-authored-by: Thom Chiovoloni --- cargo-pgrx/src/command/get.rs | 2 +- cargo-pgrx/src/command/install.rs | 4 ++-- cargo-pgrx/src/command/package.rs | 2 +- cargo-pgrx/src/manifest.rs | 2 +- cargo-pgrx/src/metadata.rs | 36 +++++++++++++++++++++++-------- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/cargo-pgrx/src/command/get.rs b/cargo-pgrx/src/command/get.rs index b21681411..1fef9c3cf 100644 --- a/cargo-pgrx/src/command/get.rs +++ b/cargo-pgrx/src/command/get.rs @@ -35,7 +35,7 @@ impl CommandExecute for Get { fn execute(self) -> eyre::Result<()> { let metadata = crate::metadata::metadata(&Default::default(), self.manifest_path.as_ref()) .wrap_err("couldn't get cargo metadata")?; - crate::metadata::validate(&metadata)?; + crate::metadata::validate(self.manifest_path.as_ref(), &metadata)?; let package_manifest_path = crate::manifest::manifest_path(&metadata, self.package.as_ref()) .wrap_err("Couldn't get manifest path")?; diff --git a/cargo-pgrx/src/command/install.rs b/cargo-pgrx/src/command/install.rs index 40bb589b8..b75de2563 100644 --- a/cargo-pgrx/src/command/install.rs +++ b/cargo-pgrx/src/command/install.rs @@ -53,7 +53,7 @@ impl CommandExecute for Install { fn execute(mut self) -> eyre::Result<()> { let metadata = crate::metadata::metadata(&self.features, self.manifest_path.as_ref()) .wrap_err("couldn't get cargo metadata")?; - crate::metadata::validate(&metadata)?; + crate::metadata::validate(self.manifest_path.as_ref(), &metadata)?; let package_manifest_path = crate::manifest::manifest_path(&metadata, self.package.as_ref()) .wrap_err("Couldn't get manifest path")?; @@ -398,7 +398,7 @@ pub(crate) fn get_version(manifest_path: impl AsRef) -> eyre::Result eyre::Result<()> { let metadata = crate::metadata::metadata(&self.features, self.manifest_path.as_ref()) .wrap_err("couldn't get cargo metadata")?; - crate::metadata::validate(&metadata)?; + crate::metadata::validate(self.manifest_path.as_ref(), &metadata)?; let package_manifest_path = crate::manifest::manifest_path(&metadata, self.package.as_ref()) .wrap_err("Couldn't get manifest path")?; diff --git a/cargo-pgrx/src/manifest.rs b/cargo-pgrx/src/manifest.rs index cdaf042b8..9b52fddab 100644 --- a/cargo-pgrx/src/manifest.rs +++ b/cargo-pgrx/src/manifest.rs @@ -203,7 +203,7 @@ pub(crate) fn get_package_manifest( ) -> eyre::Result<(Manifest, PathBuf)> { let metadata = crate::metadata::metadata(&features, manifest_path.as_ref()) .wrap_err("couldn't get cargo metadata")?; - crate::metadata::validate(&metadata)?; + crate::metadata::validate(manifest_path.as_ref(), &metadata)?; let package_manifest_path = crate::manifest::manifest_path(&metadata, package_nane) .wrap_err("Couldn't get manifest path")?; diff --git a/cargo-pgrx/src/metadata.rs b/cargo-pgrx/src/metadata.rs index e3be1d8d4..0de096d34 100644 --- a/cargo-pgrx/src/metadata.rs +++ b/cargo-pgrx/src/metadata.rs @@ -1,3 +1,4 @@ +use std::collections::BTreeMap; //LICENSE Portions Copyright 2019-2021 ZomboDB, LLC. //LICENSE //LICENSE Portions Copyright 2021-2023 Technology Concepts & Design, Inc. @@ -9,6 +10,7 @@ //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. use cargo_metadata::{Metadata, MetadataCommand}; use eyre::eyre; +use owo_colors::*; use semver::VersionReq; use std::path::Path; @@ -26,7 +28,10 @@ pub fn metadata( } #[tracing::instrument(level = "error", skip_all)] -pub fn validate(metadata: &Metadata) -> eyre::Result<()> { +pub fn validate( + path: Option>, + metadata: &Metadata, +) -> eyre::Result<()> { let cargo_pgrx_version = env!("CARGO_PKG_VERSION"); let cargo_pgrx_version_req = VersionReq::parse(&format!("~{}", cargo_pgrx_version))?; @@ -37,17 +42,11 @@ pub fn validate(metadata: &Metadata) -> eyre::Result<()> { || package.name == "pgrx-tests" }); + let mut mismatches = BTreeMap::new(); for package in pgrx_packages { let package_semver = package.version.clone(); if !cargo_pgrx_version_req.matches(&package_semver) { - return Err(eyre!( - r#"`{}-{}` shouldn't be used with `cargo-pgrx-{}`, please use `{} = "~{}"` in your `Cargo.toml`."#, - package.name, - package.version, - cargo_pgrx_version, - package.name, - cargo_pgrx_version, - )); + mismatches.insert(package.name.clone(), package.version.clone()); } else { tracing::trace!( "`{}-{}` is compatible with `cargo-pgrx-{}`.", @@ -58,5 +57,24 @@ pub fn validate(metadata: &Metadata) -> eyre::Result<()> { } } + if !mismatches.is_empty() { + let many = mismatches.len(); + let mismatches = mismatches + .into_iter() + .map(|(p, v)| format!("`{p} = {v}`")) + .collect::>() + .join(", "); + + return Err(eyre!( + "The installed `cargo-pgrx` v{cargo_pgrx_version} \ + is not compatible with the {mismatches} {} in `{}`. `cargo-pgrx` \ + and pgrx dependency versions must be identical.", + if many == 1 { "dependency" } else { "dependencies" }, + path.map(|p| p.as_ref().display().to_string()) + .unwrap_or_else(|| "./Cargo.toml".to_string()) + .yellow() + )); + } + Ok(()) }