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

package registration process #1

Open
StefanKarpinski opened this issue Jan 8, 2019 · 3 comments
Open

package registration process #1

StefanKarpinski opened this issue Jan 8, 2019 · 3 comments
Assignees

Comments

@StefanKarpinski
Copy link
Contributor

See discussion in JuliaLang/Pkg.jl#849.

@StefanKarpinski
Copy link
Contributor Author

I should condense down the complete process we're going with and put it in the README.

@StefanKarpinski StefanKarpinski self-assigned this Feb 7, 2019
@StefanKarpinski
Copy link
Contributor Author

On the Pkg call yesterday, @KristofferC, @fredrikekre and I had a longish discussion about this. They pointed out something I hadn't really realized before: there's an open question about which tree is actually tested by Registrator and should become the tagged package tree—is it the one that's in the pull request branch or is it the one that results from merging the pull request branch back into master or whatever it's (presumably long-lived) base branch is?

  • If we use the version on the pull request branch, then if the pull request is squashed or rebased before merging (which is common and easy in the GitHub UI), the tree that was tested and approved may not exist anywhere in the resulting git history.

  • On the other hand, if we use the tree that would result from merging the pull request with its base branch, then there's a chance that something else gets merged into the base branch before the pull request gets merged, which again means that the tested and approved tree may not exist anywhere in the resulting git history.

The idea that we converged on was to optimistically test the tree that would be the result of merging the pull request with its base branch at the time of triggering Registrator and remember this tree. If, when the pull-request is merged a different tree results because of intervening changes on the base branch, then Registrator would test that tree as well and either approve or reject it. If it's approved, that new tree is the one that should be tagged as the new version. If it's rejected, the process would have to start over again.

In discussing this, we noticed another issue, which is that in the future when we're doing CIBot-style forward and reverse dependency checking there's a similar race condition with the state of the registry when the testing is done. The scenario is this:

  • Regsitrator approval is requested and granted for a given tree, which means that it checked that the new version works with high and low versions of dependencies that it claims to work with and maybe high and low versions of reverse dependencies.

  • The ball is back in the package maintainers court to tag a version. They do so, possibly manually, which may take a while. They tag the approved tree with the approved version number so everything should be cool. Ball returned to the Registrator.

  • Next, Registator should merge the pull request against the registry which adds metadata about this new version to the registry. However, in the meantime, other changes to the registry may have been merged which invalidate the checks that were done that the new package version works.

The worst case is that a new, incompatible version of a dependency is tagged and registered in the meantime and tests for neither package would catch this. The dependency was approved and merged before the depender version that it breaks was merged, so it can't know there's a problem. The depender pull request was approved before that happened but merged afterwards.

The solution to this second problem seems to be similar to the first one: record what state of the registry when Registrator approved a new version and if the registry state tree has changed in a way that could potentially have invalidated that approval, rerun the checks again. There are several levels of cleverness that could be applied here:

  1. Ignore the problem entirely;
  2. Rerun all checks if the registry has changed at all;
  3. Rerun all checks if any forward or reverse dependency has changed;
  4. Rerun only checks that could have been affected by the registry change.

We can probably start at 0 and move up to 3 over time.

@nkottary
Copy link
Member

Regarding the registration race-condition, I like @KristofferC 's idea of triggering the registration from a commit comment. Seems like a simple and easy solution. Here's an example of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants