Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix cargo install with a semver metadata version. #9467

Merged
merged 1 commit into from
May 10, 2021
Merged

Fix cargo install with a semver metadata version. #9467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 29 additions & 3 deletions src/cargo/core/package_id.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 31,22 @@ struct PackageIdInner {
source_id: SourceId,
}

// Custom equality that uses full equality of SourceId, rather than its custom equality.
// Custom equality that uses full equality of SourceId, rather than its custom equality,
// and Version, which usually ignores `build` metadata.
//
// The `build` part of the version is usually ignored (like a "comment").
// However, there are some cases where it is important. The download path from
// a registry includes the build metadata, and Cargo uses PackageIds for
// creating download paths. Including it here prevents the PackageId interner
// from getting poisoned with PackageIds where that build metadata is missing.
impl PartialEq for PackageIdInner {
fn eq(&self, other: &Self) -> bool {
self.name == other.name
&& self.version == other.version
&& self.version.major == other.version.major
&& self.version.minor == other.version.minor
&& self.version.patch == other.version.patch
&& self.version.pre == other.version.pre
&& self.version.build == other.version.build
&& self.source_id.full_eq(other.source_id)
}
}
Expand All @@ -44,7 55,11 @@ impl PartialEq for PackageIdInner {
impl Hash for PackageIdInner {
fn hash<S: hash::Hasher>(&self, into: &mut S) {
self.name.hash(into);
self.version.hash(into);
self.version.major.hash(into);
self.version.minor.hash(into);
self.version.patch.hash(into);
self.version.pre.hash(into);
self.version.build.hash(into);
self.source_id.full_hash(into);
}
}
Expand Down Expand Up @@ -97,6 112,8 @@ impl PartialEq for PackageId {
if ptr::eq(self.inner, other.inner) {
return true;
}
// This is here so that PackageId uses SourceId's and Version's idea
// of equality. PackageIdInner uses a more exact notion of equality.
self.inner.name == other.inner.name
&& self.inner.version == other.inner.version
&& self.inner.source_id == other.inner.source_id
Expand All @@ -105,6 122,9 @@ impl PartialEq for PackageId {

impl Hash for PackageId {
fn hash<S: hash::Hasher>(&self, state: &mut S) {
// This is here (instead of derived) so that PackageId uses SourceId's
// and Version's idea of equality. PackageIdInner uses a more exact
// notion of hashing.
self.inner.name.hash(state);
self.inner.version.hash(state);
self.inner.source_id.hash(state);
Expand Down Expand Up @@ -166,6 186,12 @@ impl PackageId {
}
}

/// Returns a value that implements a "stable" hashable value.
///
/// Stable hashing removes the path prefix of the workspace from path
/// packages. This helps with reproducible builds, since this hash is part
/// of the symbol metadata, and we don't want the absolute path where the
/// build is performed to affect the binary output.
pub fn stable_hash(self, workspace: &Path) -> PackageIdStableHash<'_> {
PackageIdStableHash(self, workspace)
}
Expand Down
61 changes: 59 additions & 2 deletions tests/testsuite/install.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 5,15 @@ use std::io::prelude::*;

use cargo_test_support::cross_compile;
use cargo_test_support::git;
use cargo_test_support::registry::{registry_path, registry_url, Package};
use cargo_test_support::registry::{self, registry_path, registry_url, Package};
use cargo_test_support::{
basic_manifest, cargo_process, no_such_file_err_msg, project, symlink_supported, t,
};

use cargo_test_support::install::{
assert_has_installed_exe, assert_has_not_installed_exe, cargo_home,
};
use cargo_test_support::paths;
use cargo_test_support::paths::{self, CargoPathExt};
use std::env;
use std::path::PathBuf;

Expand Down Expand Up @@ -1739,3 1739,60 @@ fn locked_install_without_published_lockfile() {
.with_stderr_contains("[WARNING] no Cargo.lock file published in foo v0.1.0")
.run();
}

#[cargo_test]
fn install_semver_metadata() {
// Check trying to install a package that uses semver metadata.
// This uses alt registry because the bug this is exercising doesn't
// trigger with a replaced source.
registry::alt_init();
Package::new("foo", "1.0.0 abc")
.alternative(true)
.file("src/main.rs", "fn main() {}")
.publish();

cargo_process("install foo --registry alternative --version 1.0.0 abc").run();
cargo_process("install foo --registry alternative")
.with_stderr("\
[UPDATING] `[ROOT]/alternative-registry` index
[IGNORED] package `foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)` is already installed, use --force to override
[WARNING] be sure to add [..]
")
.run();
// "Updating" is not displayed here due to the --version fast-path.
cargo_process("install foo --registry alternative --version 1.0.0 abc")
.with_stderr("\
[IGNORED] package `foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)` is already installed, use --force to override
[WARNING] be sure to add [..]
")
.run();
cargo_process("install foo --registry alternative --version 1.0.0 --force")
.with_stderr(
"\
[UPDATING] `[ROOT]/alternative-registry` index
[INSTALLING] foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)
[COMPILING] foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)
[FINISHED] [..]
[REPLACING] [ROOT]/home/.cargo/bin/foo[EXE]
[REPLACED] package [..]
[WARNING] be sure to add [..]
",
)
.run();
// Check that from a fresh cache will work without metadata, too.
paths::home().join(".cargo/registry").rm_rf();
paths::home().join(".cargo/bin").rm_rf();
cargo_process("install foo --registry alternative --version 1.0.0")
.with_stderr("\
[UPDATING] `[ROOT]/alternative-registry` index
[DOWNLOADING] crates ...
[DOWNLOADED] foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)
[INSTALLING] foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)
[COMPILING] foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)
[FINISHED] [..]
[INSTALLING] [ROOT]/home/.cargo/bin/foo[EXE]
[INSTALLED] package `foo v1.0.0 abc (registry `[ROOT]/alternative-registry`)` (executable `foo[EXE]`)
[WARNING] be sure to add [..]
")
.run();
}