Auto merge of #9859 - dtolnay-contrib:pullrequest, r=ehuss

rev = "refs/pull/𑑛/head"

GitHub exposes a named reference associated with the head of every pull request. For example, you can fetch *this* pull request:

```console
$ git fetch origin refs/pull/9859/head
$ git show FETCH_HEAD
```

Usually when I care about pulling in a patch of one of my dependencies using `[patch.crates-io]`, this is the ref that I want to depend on. None of the alternatives are good:

- `{ git = "the fork", branch = "the-pr-branch" }` — this is second closest to what I want, except that when the PR merges and the PR author deletes their fork, it'll breaks.

- `{ git = "the fork", rev = "commithash" }` — same failure mode as the previous. Also doesn't stay up to date with PR, which is sometimes what I want.

- `{ git = "the upstream", rev = "commithash" }` — doesn't work until the PR is merged or the repo owner creates a named branch or tag containing the PR commit among its ancestors, because otherwise the commit doesn't participate in Cargo's fetch.

- `{ git = "my own fork of PR author's repo", branch = "the-pr-branch" }` — doesn't stay up to date with PR.

This PR adds support for specifying a git dependency on the head of a pull request via the existing `rev` setting of git dependencies: `{ git = "the upstream", rev = "refs/pull/9859/head" }`.

Previously this would fail because the `cargo::sources::git::fetch` function touched in this pull request did not fetch the refspec that we care about. The failures look like:

```console
error: failed to get `mockall` as a dependency of package `testing v0.0.0`

Caused by:
  failed to load source for dependency `mockall`

Caused by:
  Unable to update https://github.com/asomers/mockall?rev=refs/pull/330/head

Caused by:
  revspec 'refs/pull/330/head' not found; class=Reference (4); code=NotFound (-3)
```

If dual purposing `rev` for this is not appealing, I would alternatively propose `{ git = "the upstream", pull-request = "9859" }` which Cargo will interpret using GitHub's ref naming convention as `refs/pull/9859/head`.

Author: bors

src/cargo/sources/git/utils.rs

@@ -799,13 +799,17 @@ pub fn fetch(
             refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD"));
         }
 
-        // For `rev` dependencies we don't know what the rev will point to. To
-        // handle this situation we fetch all branches and tags, and then we
-        // pray it's somewhere in there.
-        GitReference::Rev(_) => {
-            refspecs.push(String::from("refs/heads/*:refs/remotes/origin/*"));
-            refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD"));
-            tags = true;
+        GitReference::Rev(rev) => {
+            if rev.starts_with("refs/") {
+                refspecs.push(format!("{0}:{0}", rev));
+            } else {
+                // We don't know what the rev will point to. To handle this
+                // situation we fetch all branches and tags, and then we pray
+                // it's somewhere in there.
+                refspecs.push(String::from("refs/heads/*:refs/remotes/origin/*"));
+                refspecs.push(String::from("HEAD:refs/remotes/origin/HEAD"));
+                tags = true;
+            }
         }
     }
 
@@ -1025,9 +1029,13 @@ fn github_up_to_date(
         GitReference::Branch(branch) => branch,
         GitReference::Tag(tag) => tag,
         GitReference::DefaultBranch => "HEAD",
-        GitReference::Rev(_) => {
-            debug!("can't use github fast path with `rev`");
-            return Ok(false);
+        GitReference::Rev(rev) => {
+            if rev.starts_with("refs/") {
+                rev
+            } else {
+                debug!("can't use github fast path with `rev = \"{}\"`", rev);
+                return Ok(false);
+            }
         }
     };
 

src/doc/src/reference/specifying-dependencies.md

@@ -147,6 +147,13 @@ the latest commit on a branch named `next`:
 regex = { git = "https://github.com/rust-lang/regex", branch = "next" }
 ```
 
+Anything that is not a branch or tag falls under `rev`. This can be a commit
+hash like `rev = "4c59b707"`, or a named reference exposed by the remote
+repository such as `rev = "refs/pull/493/head"`. What references are available
+varies by where the repo is hosted; GitHub in particular exposes a reference to
+the most recent commit of every pull request as shown, but other git hosts often
+provide something equivalent, possibly under a different naming scheme.
+
 Once a `git` dependency has been added, Cargo will lock that dependency to the
 latest commit at the time. New commits will not be pulled down automatically
 once the lock is in place. However, they can be pulled down manually with

tests/testsuite/git.rs

@@ -230,6 +230,68 @@ fn cargo_compile_git_dep_tag() {
     project.cargo("build").run();
 }
 
+#[cargo_test]
+fn cargo_compile_git_dep_pull_request() {
+    let project = project();
+    let git_project = git::new("dep1", |project| {
+        project
+            .file("Cargo.toml", &basic_lib_manifest("dep1"))
+            .file(
+                "src/dep1.rs",
+                r#"
+                    pub fn hello() -> &'static str {
+                        "hello world"
+                    }
+                "#,
+            )
+    });
+
+    // Make a reference in GitHub's pull request ref naming convention.
+    let repo = git2::Repository::open(&git_project.root()).unwrap();
+    let oid = repo.refname_to_id("HEAD").unwrap();
+    let force = false;
+    let log_message = "open pull request";
+    repo.reference("refs/pull/330/head", oid, force, log_message)
+        .unwrap();
+
+    let project = project
+        .file(
+            "Cargo.toml",
+            &format!(
+                r#"
+                    [project]
+                    name = "foo"
+                    version = "0.0.0"
+
+                    [dependencies]
+                    dep1 = {{ git = "{}", rev = "refs/pull/330/head" }}
+                "#,
+                git_project.url()
+            ),
+        )
+        .file(
+            "src/main.rs",
+            &main_file(r#""{}", dep1::hello()"#, &["dep1"]),
+        )
+        .build();
+
+    let git_root = git_project.root();
+
+    project
+        .cargo("build")
+        .with_stderr(&format!(
+            "[UPDATING] git repository `{}`\n\
+             [COMPILING] dep1 v0.5.0 ({}?rev=refs/pull/330/head#[..])\n\
+             [COMPILING] foo v0.0.0 ([CWD])\n\
+             [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]\n",
+            path2url(&git_root),
+            path2url(&git_root),
+        ))
+        .run();
+
+    assert!(project.bin("foo").is_file());
+}
+
 #[cargo_test]
 fn cargo_compile_with_nested_paths() {
     let git_project = git::new("dep1", |project| {