Skip to content

Commit

Permalink
Cleanup the error when cargo-pgrx version doesn't match Cargo.toml (#…
Browse files Browse the repository at this point in the history
…1240)

As was discussed in the comments in another issue:
#1076 (comment)
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 <[email protected]>
Co-authored-by: Thom Chiovoloni <[email protected]>
  • Loading branch information
3 people authored Aug 1, 2023
1 parent 7ab42ce commit 2011757
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 14 deletions.
2 changes: 1 addition & 1 deletion cargo-pgrx/src/command/get.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;
Expand Down
4 changes: 2 additions & 2 deletions cargo-pgrx/src/command/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;
Expand Down Expand Up @@ -398,7 +398,7 @@ pub(crate) fn get_version(manifest_path: impl AsRef<Path>) -> eyre::Result<Strin
if v == "@CARGO_VERSION@" {
let metadata = crate::metadata::metadata(&Default::default(), Some(&manifest_path))
.wrap_err("couldn't get cargo metadata")?;
crate::metadata::validate(&metadata)?;
crate::metadata::validate(Some(manifest_path), &metadata)?;
let manifest_path = crate::manifest::manifest_path(&metadata, None)
.wrap_err("Couldn't get manifest path")?;
let manifest = Manifest::from_path(&manifest_path)
Expand Down
2 changes: 1 addition & 1 deletion cargo-pgrx/src/command/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl CommandExecute for Package {
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")?;
Expand Down
2 changes: 1 addition & 1 deletion cargo-pgrx/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;

Expand Down
36 changes: 27 additions & 9 deletions cargo-pgrx/src/metadata.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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;

Expand All @@ -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<impl AsRef<std::path::Path>>,
metadata: &Metadata,
) -> eyre::Result<()> {
let cargo_pgrx_version = env!("CARGO_PKG_VERSION");
let cargo_pgrx_version_req = VersionReq::parse(&format!("~{}", cargo_pgrx_version))?;

Expand All @@ -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-{}`.",
Expand All @@ -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::<Vec<_>>()
.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(())
}

0 comments on commit 2011757

Please sign in to comment.