Skip to content

Commit

Permalink
fix: panic due to unexpected internal state (#222)
Browse files Browse the repository at this point in the history
When there was no change in the src/ directory of the top-level crate,
the dependency resolution would not be able to auto-bump the version
as no change occurred, but another part would usually detect a change
as it wasn't confined to the top-level src/ directory.

This could lead to a panic as an invariant wasn't upheld.

This was fixed by letting both parts agree to use the src/ directory
to determine changes of the top-level directory, and by making panics
impossible while improving the messaging around this state should it
still occur. The latter is rough, probably rare, but usable.
Byron committed Oct 19, 2021
1 parent 3cdebf5 commit ce68733
Showing 4 changed files with 23 additions and 8 deletions.
1 change: 1 addition & 0 deletions cargo-smart-release/README.md
Original file line number Diff line number Diff line change
@@ -86,6 +86,7 @@ Here is what `cargo smart-release` does differently: "It tries really hard to do
* changelog rewriting of user content will drop links if they are not of the 'inline' form
* it's very young and probably tries to eat underwear
* it needs a git repository to govern the workspace
* When determining if something changed in top-level crates, only the `src/` directory is used. This value is hard-coded.

## Acknowledgements

25 changes: 19 additions & 6 deletions cargo-smart-release/src/command/release/mod.rs
Original file line number Diff line number Diff line change
@@ -210,12 +210,25 @@ fn present_dependencies(
.as_ref()
.and_then(|lr| (*lr >= bump.desired_release).then(|| lr))
{
log::warn!(
"Latest published version of '{}' is {}, the new version is {}. Consider using --bump <level> or --bump-dependencies <level> or update the index with --update-crates-index.",
dep.package.name,
latest_release,
bump.desired_release
);
let bump_flag = match dep.kind {
dependency::Kind::UserSelection => "--bump <level>",
dependency::Kind::DependencyOrDependentOfUserSelection => "--bump-dependencies <level>",
};
if bump.desired_release == bump.package_version {
log::warn!(
"'{}' is unchanged. Consider using {} along with --no-bump-on-demand to force a version change.",
dep.package.name,
bump_flag
);
} else {
log::warn!(
"Latest published version of '{}' is {}, the new version is {}. Consider using {} or update the index with --update-crates-index.",
dep.package.name,
latest_release,
bump.desired_release,
bump_flag
);
}
error = true;
}
if bump.next_release > dep.package.version {
4 changes: 3 additions & 1 deletion cargo-smart-release/src/git/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::{convert::TryInto, process::Command};

use anyhow::{anyhow, bail};
use cargo_metadata::camino::Utf8Path;
use cargo_metadata::Package;
use git_repository as git;
use git_repository::{
@@ -36,7 +37,8 @@ pub fn change_since_last_release(package: &Package, ctx: &crate::Context) -> any
let current_commit = c?;
let released_target = tag_ref.peel_to_id_in_place()?;

match repo_relative_crate_dir {
// If it's a top-level crate, use the src-directory for now
match repo_relative_crate_dir.or_else(|| Some(Utf8Path::new("src"))) {
None => (current_commit != released_target).then(|| PackageChangeKind::ChangedOrNew),
Some(dir) => {
let components = dir.components().map(component_to_bytes);
1 change: 0 additions & 1 deletion cargo-smart-release/src/version.rs
Original file line number Diff line number Diff line change
@@ -101,7 +101,6 @@ pub(crate) fn bump_package_with_spec(
);
let unreleased = &segments[0];
if unreleased.history.is_empty() {
log::trace!("{}: no changes since the last release. Version unchanged", package.name);
false
} else if unreleased.history.iter().any(|item| item.message.breaking) {
let is_breaking = if is_pre_release(&v) {

0 comments on commit ce68733

Please sign in to comment.