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

Migrate ALL providers to expecting main branch, not just GitHub, and announce the change #1175

Closed
2 tasks
sgibson91 opened this issue Oct 29, 2020 · 9 comments · Fixed by #1186
Closed
2 tasks

Comments

@sgibson91
Copy link
Member

The work done in #1172 means that binderhub now looks for the main branch if a ref isn't provided, rather than master. This is probably a change we should roll out across all providers but I, at least, am unfamiliar with where these other communities are in this process and this change will be breaking previously established behaviour, possibly disrupting a lot of users. Therefore, I think we should have some form of "announcement" (blog post, twitter thread...) ready to go when we do make this change, just so users aren't blind-sided and they can find information on how to fix their links.

Note: This change does not affect gists over which the user has no control of the ref naming.

TODOs

placeholder = "main";
if (provider === "gist") {
    placeholder = "master";
}
@minrk
Copy link
Member

minrk commented Oct 29, 2020

I think what might be even more appropriate, if more complex, is to use a special not-valid ref e.g. $default which tells us to use the git repo's default branch. It's not common to specify the branch name when using the default branch for things like git clone, and we are seeing the consequences of Binder's API diverging from this standard practice. I think it's worthwhile to investigate what would be involved in that (i.e. is there a good git and/or github call to resolve the default branch?) before hardcoding "main" as default everywhere. It may not be feasible, in which case this change is probably right, but I think we can prevent the inevitable confusion by letting the branch actually be unspecified.

@sgibson91
Copy link
Member Author

Yes, I agree that that is the ideal resolution to this!

@sgibson91
Copy link
Member Author

sgibson91 commented Oct 29, 2020

Would it need to be resolved before or after the repo is cloned?

Edited to add: We could probably use the GET request in this code block if it needed to be resolved before #1145 (comment)

@minrk
Copy link
Member

minrk commented Oct 29, 2020

Would it need to be resolved before or after the repo is cloned?

That's what I was aiming at with "is there a good git and/or github call to resolve the default branch?" GitHub definitely has an API for it, but I wonder if we can use the git remote API to make it work for all the git providers without needing a new implementation for every provider. That's always been a mysterious API to me, though.

@minrk
Copy link
Member

minrk commented Oct 29, 2020

Looks like it's possible, if we parse this output:

$ git ls-remote --symref https://github.com/minrk/jupyter-activity-data HEAD
ref: refs/heads/main	HEAD
97275c467fc137d7a3947c8f9b43635c69a4ea8f	HEAD
$ git ls-remote --symref https://github.com/jupyterhub/jupyterhub HEAD
ref: refs/heads/master	HEAD
f92560fed024288ed834e8e7f8a26363d6117a7c	HEAD

@sgibson91
Copy link
Member Author

That's very cool!

@betatim
Copy link
Member

betatim commented Oct 29, 2020

Take a look at #843 which was closed by #895. Maybe we can hoist this out of the Git repo provider and use it for GitHub and GitLab as well?

@minrk
Copy link
Member

minrk commented Oct 30, 2020

Yes, this is related to #895 which also uses ls-remote to resolve refs. The addition of --symref HEAD is what lets us find the branch name associated with HEAD.

But this is even easier than I thought! HEAD is already a valid ref, so it's working already and we should just make our default ref HEAD instead of a branch name, and our existing APIs already work:

https://mybinder.org/v2/gh/binderhub-ci-repos/requirements/HEAD

This is strictly better than resolving the default branch name and using that in the URL, because it allows creating a stable link that will work across default branch changes without changes to the link.

Need to test with all providers, but I think it'll work.

@minrk
Copy link
Member

minrk commented Oct 30, 2020

#1186 switches the default ref to HEAD. Works everywhere without modification, except for gist, where we were resolving master -> latest ref ourselves.

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

Successfully merging a pull request may close this issue.

3 participants