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

Fix regex for public bitbucket repo #3533

Merged
merged 3 commits into from Mar 23, 2018

Conversation

@stsewd
Copy link
Member

@stsewd stsewd commented Jan 18, 2018

I just realized that on #3501 I forgot to test a bitbucket repo as a logout user.

When you are login, the clone url is

screenshot-2018-1-18 stsewd test bucktet git bitbucket

But when you are log out look like this

bitbucket

This ends with .git

Sorry for the bug :(

@RichardLitt
Copy link
Member

@RichardLitt RichardLitt commented Jan 18, 2018

I'm not sure that manually writing and adding these tests is the best way to do this. The git protocol is very complex. Is there an external library we can use?

For instance, https://github.com/npm/hosted-git-info would work if we were in JavaScript land. https://github.com/npm/hosted-git-info/blob/master/index.js#L44 is an example of a regex we could borrow to bootstrap making a library if there isn't one.

Copy link
Member

@humitos humitos left a comment

Thanks!

@humitos
Copy link
Member

@humitos humitos commented Feb 15, 2018

I'm not sure that manually writing and adding these tests is the best way to do this. The git protocol is very complex. Is there an external library we can use?

Agree. Although, most of the code is writting by hand so, following this pattern here is enough. On the other hand, as you said, we will want to achieve this by relying in a external library. We have talked about using gitpython: https://gitpython.readthedocs.io/en/stable/intro.html

Copy link
Contributor

@davidfischer davidfischer left a comment

👍

GitPython is for interacting with git itself (inspecting a repo, performing a branch, etc.). What we are looking for is a library that parses and assembles GitHub/Bitbucket/etc. I'm not aware of a library like that in Python. It's very possible that the code we have is the best start of such a library.

@RichardLitt
Copy link
Member

@RichardLitt RichardLitt commented Feb 20, 2018

What we are looking for is a library that parses and assembles GitHub/Bitbucket/etc. I'm not aware of a library like that in Python. It's very possible that the code we have is the best start of such a library.

Might be worth just making a port of hosted-git-info, then.

@agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Mar 23, 2018

We started to use gitpython on a different branch, though I'm not sure if it has helpers to determine valid repo names. Perhaps this pushes us to use pygit2 or dulwich instead. I could see importing mercurial directly for checks on hg patterns, though I've never used mercurial as a python import before -- i'm sure it has what we need though as it's the entire cli tool.

@humitos
Copy link
Member

@humitos humitos commented Mar 23, 2018

Perhaps this pushes us to use pygit2 or dulwich instead

I didn't check dulwich, but pygit2 seems to be too complex to install properly and I didn't like the API when I checked it yesterday. GitPython was very clean and direct to understand.

dulwich seems to be similar than GitPython.

@humitos
Copy link
Member

@humitos humitos commented Mar 23, 2018

i'm sure it has what we need though as it's the entire cli tool.

Mmm... I wouldn't be too sure. We need to validate web service URLs (github.com, gitlab.com and bitbucket.com) not valid git URLs --that's another story.

Mercurial allows you to just run: hg clone http://invalid.bitbucket.url/ and that totally fine for mercurial, but it's not for us.

As a note, we only use these REGEX to get the username and repo given a URL

@humitos
Copy link
Member

@humitos humitos commented Mar 23, 2018

@agjohnson
Copy link
Contributor

@agjohnson agjohnson commented Mar 23, 2018

Ah true, we're looking to classify the provider here as well.

We could copy the following regex patterns in from:
https://github.com/npm/hosted-git-info/blob/latest/git-host-info.js

It might be a helpful port to python though, bonus points to anyone that wants to maintain that :)

I'd say this is all a new ticket though. This looks okay to merge now.

@agjohnson agjohnson merged commit f3c0c0a into readthedocs:master Mar 23, 2018
1 check passed
@stsewd stsewd deleted the fix-regex-public-bitbucket-repo branch Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants