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.

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

Count the beta prerelease number just from master #72993

Merged
merged 1 commit into from
Jun 7, 2020

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Jun 4, 2020

We were computing a merge-base between the remote beta and master
branches, but this was giving incorrect answers for the first beta if
the remote hadn't been pushed yet. For instance, 1.45.0-beta.3359
corresponds to the number of merges since the 1.44 beta, but we really
want just .1 for the sole 1.45 beta promotion merge.

We don't really need to query the remote beta at all -- master..HEAD
suffices if we assume that we're on the intended beta branch already.

We were computing a merge-base between the remote beta and master
branches, but this was giving incorrect answers for the first beta if
the remote hadn't been pushed yet. For instance, `1.45.0-beta.3359`
corresponds to the number of merges since the 1.44 beta, but we really
want just `.1` for the sole 1.45 beta promotion merge.

We don't really need to query the remote beta at all -- `master..HEAD`
suffices if we assume that we're on the intended beta branch already.
@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 4, 2020
@cuviper
Copy link
Member Author

cuviper commented Jun 4, 2020

FWIW, in my first draft I had a snippet like this:

        // We used to run `git ls-remote origin master`, but you still need that commit locally to
        // do things like `merge-base` or `rev-list`, so we might as well fetch it.
        run(Command::new("git").arg("fetch").arg("origin").arg("master").current_dir(&self.src));

... and then listed from FETCH_HEAD..HEAD. I can add that back if desired, but we don't really need a most-current master, just at least as recent as this beta was branched.

In fact, I think the old code that used ls-remote had a race in this regard, if something happened to merge into master after the local repo had last cloned/updated -- you could get a master commit that doesn't exist locally, and then merge-base would fail.

@Mark-Simulacrum
Copy link
Member

Yes, this seems reasonable. I admit that I can't entirely follow the reasoning -- likely would need to sketch some commit trees out or so -- but I think that's not necessary, if it doesn't work that's not going to break anything I suspect anyway.

I thought about getting this into the current beta (backporting it) but I guess there's not that much point so I'm going to not do that.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 4, 2020

📌 Commit 37a24b3 has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 4, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 5, 2020
Count the beta prerelease number just from master

We were computing a merge-base between the remote beta and master
branches, but this was giving incorrect answers for the first beta if
the remote hadn't been pushed yet. For instance, `1.45.0-beta.3359`
corresponds to the number of merges since the 1.44 beta, but we really
want just `.1` for the sole 1.45 beta promotion merge.

We don't really need to query the remote beta at all -- `master..HEAD`
suffices if we assume that we're on the intended beta branch already.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Jun 5, 2020
Count the beta prerelease number just from master

We were computing a merge-base between the remote beta and master
branches, but this was giving incorrect answers for the first beta if
the remote hadn't been pushed yet. For instance, `1.45.0-beta.3359`
corresponds to the number of merges since the 1.44 beta, but we really
want just `.1` for the sole 1.45 beta promotion merge.

We don't really need to query the remote beta at all -- `master..HEAD`
suffices if we assume that we're on the intended beta branch already.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 7, 2020
Rollup of 5 pull requests

Successful merges:

 - rust-lang#72810 (validate basic sanity for TerminatorKind)
 - rust-lang#72989 (Revert pr 71840)
 - rust-lang#72993 (Count the beta prerelease number just from master)
 - rust-lang#73057 (Clean up E0644 explanation)
 - rust-lang#73059 (remove outdated comment)

Failed merges:

r? @ghost
@bors bors merged commit 101e593 into rust-lang:master Jun 7, 2020
@cuviper cuviper mentioned this pull request Jun 26, 2020
@cuviper cuviper deleted the beta-number branch August 9, 2020 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants