Revert "Change git dependencies to use HEAD by default"

This reverts commit 7dd9872.

src/cargo/core/resolver/dep_cache.rs

@@ -14,8 +14,10 @@ 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};
@@ -38,6 +40,10 @@ 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> {
@@ -46,6 +52,7 @@ impl<'a> RegistryQueryer<'a> {
         replacements: &'a [(PackageIdSpec, Dependency)],
         try_to_use: &'a HashSet<PackageId>,
         minimal_versions: bool,
+        config: Option<&'a Config>,
     ) -> Self {
         RegistryQueryer {
             registry,
@@ -55,6 +62,8 @@ impl<'a> RegistryQueryer<'a> {
             registry_cache: HashMap::new(),
             summary_cache: HashMap::new(),
             used_replacements: HashMap::new(),
+            config,
+            warned_git_collisions: HashSet::new(),
         }
     }
 
@@ -66,13 +75,52 @@ 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,7 +133,8 @@ pub fn resolve(
         Some(config) => config.cli_unstable().minimal_versions,
         None => false,
     };
-    let mut registry = RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions);
+    let mut registry =
+        RegistryQueryer::new(registry, replacements, try_to_use, minimal_versions, config);
     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::V3
+        ResolveVersion::V2
     }
 }

src/cargo/core/source/source_id.rs

@@ -394,9 +394,45 @@ impl Ord for SourceId {
 
         // Sort first based on `kind`, deferring to the URL comparison below if
         // the kinds are equal.
-        match self.inner.kind.cmp(&other.inner.kind) {
-            Ordering::Equal => {}
-            other => return other,
+        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;
+                }
+            }
         }
 
         // If the `kind` and the `url` are equal, then for git sources we also
@@ -473,9 +509,43 @@ 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) {
-        self.inner.kind.hash(into);
+        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),
+        }
         match self.inner.kind {
             SourceKind::Git(_) => self.inner.canonical_url.hash(into),
             _ => self.inner.url.as_str().hash(into),

src/cargo/sources/git/source.rs

@@ -126,10 +126,12 @@ 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).with_context(|| {
-                    "failed to lookup reference in preexisting repository, and \
+                let rev = db
+                    .resolve(&self.manifest_reference, None)
+                    .with_context(|| {
+                        "failed to lookup reference in preexisting repository, and \
                          can't check for updates in offline mode (--offline)"
-                })?;
+                    })?;
                 (db, rev)
             }