Change git dependencies to use HEAD by default

crates/cargo-test-support/src/cross_compile.rs

@@ -177,7 +177,7 @@ rustup does not appear to be installed. Make sure that the appropriate
         },
     }
 
-    panic!(message);
+    panic!("{}", message);
 }
 
 /// The alternate target-triple to build with.

src/cargo/core/resolver/dep_cache.rs

@@ -14,10 +14,8 @@ use crate::core::resolver::errors::describe_path;
 use crate::core::resolver::types::{ConflictReason, DepInfo, FeaturesSet};
 use crate::core::resolver::{ActivateError, ActivateResult, ResolveOpts};
 use crate::core::{Dependency, FeatureValue, PackageId, PackageIdSpec, Registry, Summary};
-use crate::core::{GitReference, SourceId};
 use crate::util::errors::{CargoResult, CargoResultExt};
 use crate::util::interning::InternedString;
-use crate::util::Config;
 use log::debug;
 use std::cmp::Ordering;
 use std::collections::{BTreeSet, HashMap, HashSet};
@@ -40,10 +38,6 @@ pub struct RegistryQueryer<'a> {
     >,
     /// all the cases we ended up using a supplied replacement
     used_replacements: HashMap<PackageId, Summary>,
-    /// Where to print warnings, if configured.
-    config: Option<&'a Config>,
-    /// Sources that we've already wared about possibly colliding in the future.
-    warned_git_collisions: HashSet<SourceId>,
 }
 
 impl<'a> RegistryQueryer<'a> {
@@ -52,7 +46,6 @@ impl<'a> RegistryQueryer<'a> {
         replacements: &'a [(PackageIdSpec, Dependency)],
         try_to_use: &'a HashSet<PackageId>,
         minimal_versions: bool,
-        config: Option<&'a Config>,
     ) -> Self {
         RegistryQueryer {
             registry,
@@ -62,8 +55,6 @@ impl<'a> RegistryQueryer<'a> {
             registry_cache: HashMap::new(),
             summary_cache: HashMap::new(),
             used_replacements: HashMap::new(),
-            config,
-            warned_git_collisions: HashSet::new(),
         }
     }
 
@@ -75,52 +66,13 @@ impl<'a> RegistryQueryer<'a> {
         self.used_replacements.get(&p)
     }
 
-    /// Issues a future-compatible warning targeted at removing reliance on
-    /// unifying behavior between these two dependency directives:
-    ///
-    /// ```toml
-    /// [dependencies]
-    /// a = { git = 'https://example.org/foo' }
-    /// a = { git = 'https://example.org/foo', branch = 'master }
-    /// ```
-    ///
-    /// Historical versions of Cargo considered these equivalent but going
-    /// forward we'd like to fix this. For more details see the comments in
-    /// src/cargo/sources/git/utils.rs
-    fn warn_colliding_git_sources(&mut self, id: SourceId) -> CargoResult<()> {
-        let config = match self.config {
-            Some(config) => config,
-            None => return Ok(()),
-        };
-        let prev = match self.warned_git_collisions.replace(id) {
-            Some(prev) => prev,
-            None => return Ok(()),
-        };
-        match (id.git_reference(), prev.git_reference()) {
-            (Some(GitReference::DefaultBranch), Some(GitReference::Branch(b)))
-            | (Some(GitReference::Branch(b)), Some(GitReference::DefaultBranch))
-                if b == "master" => {}
-            _ => return Ok(()),
-        }
-
-        config.shell().warn(&format!(
-            "two git dependencies found for `{}` \
-             where one uses `branch = \"master\"` and the other doesn't; \
-             this will break in a future version of Cargo, so please \
-             ensure the dependency forms are consistent",
-            id.url(),
-        ))?;
-        Ok(())
-    }
-
     /// Queries the `registry` to return a list of candidates for `dep`.
     ///
     /// This method is the location where overrides are taken into account. If
     /// any candidates are returned which match an override then the override is
     /// applied by performing a second query for what the override should
     /// return.
     pub fn query(&mut self, dep: &Dependency) -> CargoResult<Rc<Vec<Summary>>> {
-        self.warn_colliding_git_sources(dep.source_id())?;
         if let Some(out) = self.registry_cache.get(dep).cloned() {
             return Ok(out);
         }

src/cargo/core/resolver/mod.rs

@@ -133,8 +133,7 @@ pub fn resolve(
         Some(config) => config.cli_unstable().minimal_versions,
         None => false,
     };
-    let mut registry =
-        RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions, config);
+    let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions);
     let cx = activate_deps_loop(cx, &mut registry, summaries, config)?;
 
     let mut cksums = HashMap::new();

src/cargo/core/resolver/resolve.rs

@@ -409,6 +409,6 @@ impl Default for ResolveVersion {
     /// file anyway so it takes the opportunity to bump the lock file version
     /// forward.
     fn default() -> ResolveVersion {
-        ResolveVersion::V2
+        ResolveVersion::V3
     }
 }

src/cargo/core/source/source_id.rs

@@ -394,45 +394,9 @@ impl Ord for SourceId {
 
         // Sort first based on `kind`, deferring to the URL comparison below if
         // the kinds are equal.
-        match (&self.inner.kind, &other.inner.kind) {
-            (SourceKind::Path, SourceKind::Path) => {}
-            (SourceKind::Path, _) => return Ordering::Less,
-            (_, SourceKind::Path) => return Ordering::Greater,
-
-            (SourceKind::Registry, SourceKind::Registry) => {}
-            (SourceKind::Registry, _) => return Ordering::Less,
-            (_, SourceKind::Registry) => return Ordering::Greater,
-
-            (SourceKind::LocalRegistry, SourceKind::LocalRegistry) => {}
-            (SourceKind::LocalRegistry, _) => return Ordering::Less,
-            (_, SourceKind::LocalRegistry) => return Ordering::Greater,
-
-            (SourceKind::Directory, SourceKind::Directory) => {}
-            (SourceKind::Directory, _) => return Ordering::Less,
-            (_, SourceKind::Directory) => return Ordering::Greater,
-
-            (SourceKind::Git(a), SourceKind::Git(b)) => {
-                use GitReference::*;
-                let ord = match (a, b) {
-                    (Tag(a), Tag(b)) => a.cmp(b),
-                    (Tag(_), _) => Ordering::Less,
-                    (_, Tag(_)) => Ordering::Greater,
-
-                    (Rev(a), Rev(b)) => a.cmp(b),
-                    (Rev(_), _) => Ordering::Less,
-                    (_, Rev(_)) => Ordering::Greater,
-
-                    // See module comments in src/cargo/sources/git/utils.rs
-                    // for why `DefaultBranch` is treated specially here.
-                    (Branch(a), DefaultBranch) => a.as_str().cmp("master"),
-                    (DefaultBranch, Branch(b)) => "master".cmp(b),
-                    (Branch(a), Branch(b)) => a.cmp(b),
-                    (DefaultBranch, DefaultBranch) => Ordering::Equal,
-                };
-                if ord != Ordering::Equal {
-                    return ord;
-                }
-            }
+        match self.inner.kind.cmp(&other.inner.kind) {
+            Ordering::Equal => {}
+            other => return other,
         }
 
         // If the `kind` and the `url` are equal, then for git sources we also
@@ -509,43 +473,9 @@ impl fmt::Display for SourceId {
 // The hash of SourceId is used in the name of some Cargo folders, so shouldn't
 // vary. `as_str` gives the serialisation of a url (which has a spec) and so
 // insulates against possible changes in how the url crate does hashing.
-//
-// Note that the semi-funky hashing here is done to handle `DefaultBranch`
-// hashing the same as `"master"`, and also to hash the same as previous
-// versions of Cargo while it's somewhat convenient to do so (that way all
-// versions of Cargo use the same checkout).
 impl Hash for SourceId {
     fn hash<S: hash::Hasher>(&self, into: &mut S) {
-        match &self.inner.kind {
-            SourceKind::Git(GitReference::Tag(a)) => {
-                0usize.hash(into);
-                0usize.hash(into);
-                a.hash(into);
-            }
-            SourceKind::Git(GitReference::Branch(a)) => {
-                0usize.hash(into);
-                1usize.hash(into);
-                a.hash(into);
-            }
-            // For now hash `DefaultBranch` the same way as `Branch("master")`,
-            // and for more details see module comments in
-            // src/cargo/sources/git/utils.rs for why `DefaultBranch`
-            SourceKind::Git(GitReference::DefaultBranch) => {
-                0usize.hash(into);
-                1usize.hash(into);
-                "master".hash(into);
-            }
-            SourceKind::Git(GitReference::Rev(a)) => {
-                0usize.hash(into);
-                2usize.hash(into);
-                a.hash(into);
-            }
-
-            SourceKind::Path => 1usize.hash(into),
-            SourceKind::Registry => 2usize.hash(into),
-            SourceKind::LocalRegistry => 3usize.hash(into),
-            SourceKind::Directory => 4usize.hash(into),
-        }
+        self.inner.kind.hash(into);
         match self.inner.kind {
             SourceKind::Git(_) => self.inner.canonical_url.hash(into),
             _ => self.inner.url.as_str().hash(into),

src/cargo/ops/resolve.rs

@@ -13,10 +13,12 @@
 use crate::core::compiler::{CompileKind, RustcTargetData};
 use crate::core::registry::PackageRegistry;
 use crate::core::resolver::features::{FeatureResolver, ForceAllTargets, ResolvedFeatures};
-use crate::core::resolver::{self, HasDevUnits, Resolve, ResolveOpts};
+use crate::core::resolver::{self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion};
 use crate::core::summary::Summary;
 use crate::core::Feature;
-use crate::core::{PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace};
+use crate::core::{
+    GitReference, PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace,
+};
 use crate::ops;
 use crate::sources::PathSource;
 use crate::util::errors::{CargoResult, CargoResultExt};
@@ -597,7 +599,31 @@ fn register_previous_locks(
             .deps_not_replaced(node)
             .map(|p| p.0)
             .filter(keep)
-            .collect();
+            .collect::<Vec<_>>();
+
+        // In the v2 lockfile format and prior the `branch=master` dependency
+        // directive was serialized the same way as the no-branch-listed
+        // directive. Nowadays in Cargo, however, these two directives are
+        // considered distinct and are no longer represented the same way. To
+        // maintain compatibility with older lock files we register locked nodes
+        // for *both* the master branch and the default branch.
+        //
+        // Note that this is only applicable for loading older resolves now at
+        // this point. All new lock files are encoded as v3-or-later, so this is
+        // just compat for loading an old lock file successfully.
+        if resolve.version() <= ResolveVersion::V2 {
+            let source = node.source_id();
+            if let Some(GitReference::DefaultBranch) = source.git_reference() {
+                let new_source =
+                    SourceId::for_git(source.url(), GitReference::Branch("master".to_string()))
+                        .unwrap()
+                        .with_precise(source.precise().map(|s| s.to_string()));
+
+                let node = node.with_source_id(new_source);
+                registry.register_lock(node, deps.clone());
+            }
+        }
+
         registry.register_lock(node, deps);
     }
 

src/cargo/sources/git/source.rs

@@ -126,12 +126,10 @@ impl<'cfg> Source for GitSource<'cfg> {
             // database, then try to resolve our reference with the preexisting
             // repository.
             (None, Some(db)) if self.config.offline() => {
-                let rev = db
-                    .resolve(&self.manifest_reference, None)
-                    .with_context(|| {
-                        "failed to lookup reference in preexisting repository, and \
+                let rev = db.resolve(&self.manifest_reference).with_context(|| {
+                    "failed to lookup reference in preexisting repository, and \
                          can't check for updates in offline mode (--offline)"
-                    })?;
+                })?;
                 (db, rev)
             }