[MRG] Add a Mercurial contentprovider

[paugier]

MyBinder could support Mercurial repositories

See jupyterhub/binderhub#1148

[welcome]

Thanks for submitting your first pull request! You are awesome! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please make sure you followed the pull request template, as this will help us review your contribution more quickly.

You can meet the other Jovyans by joining our Discourse forum. There is also a intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

[paugier]

@gracinet you may be interested to check what I did 🙂

[gracinet]

@paugier I don't know about the repo2docker context, but the Mercurial part itself is simple and should do the job.

Note that Mercurial requires Python >= 3.6 (apparently only Evolve actually enfores it through its requirements). That explains the CI failure, so you probably want to add some conditionals in your requirements and imports if repo2docker wants to keep compatibility with 3.5 (let's see maybe what project maintainers think of it first).

[paugier]

Is Python 3.5 really necessary since for binderhub python_requires='>=3.6' (https://github.com/jupyterhub/binderhub/blob/master/setup.py#L32) ?

[betatim]

Nice work!

As Georges pointed out there are several comments and function names that still contain "git" which should be tidied up.

The ordered list of content providers is in

repo2docker/repo2docker/app.py

Line 144 in 64371cb

content_providers = List(

You have to add the hg provider to that list (below the Zenodo one I think) for it to become available to users.

I created #951 to increase the minimal required Python version to 3.6. Let's see if anyone remembers if there is a reason we need to stick to 3.5

[paugier]

I added Mercurial to the content_providers list. I'm not sure that it is the right order, but I added it just before Git because for some random http addresses (not clearly Git ones), we need to call hg id -i https://..., which is less fast than a local check.

[betatim]

We now use Python 3.6 as the minimal require version for repo2docker. This means if you rebase your branch on the latest master the checks/selectors for Python 3.5 can be removed again.

[paugier]

We now use Python 3.6 as the minimal require version for repo2docker.

Ah good! It's going to be simpler!

Another question about setup_requires, requirements.txt and Pipfile: since the Mercurial requirements are in setup_requires, I don't need to put them in requirements.txt and Pipfile?

[betatim]

We don't need requirements.txt but I don't really know how people who use Pipfiles setup things. Maybe @manics knows?

[manics]

I don't know why we've also got a Pipfile in addition to requirements... I almost opened a new issue to ask why we have it 😄

[betatim]

The reason we have a Pipfile is because at some point people asked if we could add it because they use it to setup their dev environments.

[manics]

I found this related issue: jupyterhub/zero-to-jupyterhub-k8s#1221
@minrk could you give us some help please? Does Pipfile.lock need to be updated in sync with Pipfile?

[paugier]

Do you understand why the tests failed in Docker Cloud? I get a 404 error when I click on Details.

[minrk]

Does Pipfile.lock need to be updated in sync with Pipfile?

It should be. If you update the Pipfile, run:

pipenv install --dev

this will update Pipfile.lock, and you can commit the difference. Since I don't think many folks use these, there's a good chance Pipfile.lock is out of date already.

The Pipfile and requirements file only directly contain dev dependencies, so if it's a runtime dependency adding to install_requires, no additional changes are required.

The docker build is failing because the image lacks a compiler, which mercurial needs because they don't publish wheels
except for Windows (?!). FWIW, this also means that repo2docker would not be installable without a compiler on platforms other than Windows, which is a pretty big change, and makes me think this should be an optional dependency.

[paugier]

The docker build is failing because the image lacks a compiler, which mercurial needs because they don't publish wheels
except for Windows (?!).

Indeed it's very strange. I'm going to ask on Mercurial mailing list.

[paugier]

Regarding Pipfile, I think it doesn't need to be updated in this PR.

[betatim]

For my education: what are these mecurial extensions that you activate in the docker image? What happens if we don't enable them (and interact with a repo that uses them)?

One more thing we need is instructions to install mercurial in the docs. My suggestion would be to add a section like https://repo2docker.readthedocs.io/en/latest/install.html#prerequisite-docker but name it somehow "Optional: ". In particular explaining how to install hg and hg-evolve so that they can work together. It sounds like the recommended way of installing the hg executable is with the package manager of your OS, but then hg-evolve (a python package) needs to be installed into the same Python environment as the hg executable. Getting this right when using brew and/or conda and/or virtual envs as well as having to edit a config file to enable the extensions seems tricky. Hence a short set of instructions in our docs with a link to a full guide in the mercurial docs would be great.

(We should also really have a section for git, but that is a new PR)

To set environment variables you need to edit the GitHub Actions config/workflows which are defined in .github/. This is what we use for our CI.

I like manics suggestion of keeping the hg-evolve package as a dependency in setup.py so that it gets installed in the Python environment that repo2docker is installed in, especially if it doesn't have any extra dependencies. However it seems like the comment about hg-evolve and hg having to be installed in the same environment would mean we'd also have to install repo2docker into that environment. Right now we recommend to use a virtual env/conda env to install repo2docker. In particular on OSX you'd never want to install things in the global environment that is shipped with OSX. So my take away here is that we need some more explanation/understanding/thinking about how to install things.

[manics]

Personally I think it's better for tests to default to testing everything, with a flag/environment variable used to explicitly disable them if required. This reduces the risk of future refactoring inadvertently disabling them (been there in other projects..... 😄).

[paugier]

Personally I think it's better for tests to default to testing everything, with a flag/environment variable used to explicitly disable them if required.

Ok I understand! I will change that.

[paugier]

For my education: what are these mercurial extensions that you activate in the docker image? What happens if we don't enable them (and interact with a repo that uses them)?

topic and evolve are 2 extensions provided by the python package hg-evolve useful for shareable history edition. Topics are light and temporary feature branches.

If you clone a repository with topics without the topic extensions, you just don't get the commits in topics.

evolve adds many commands for shareable history and it may be that enabling evolve is not necessary for repo2docker. What do you think @gracinet? I enabled evolve + topics because it is standard for real users but actually we don't do history edition in repo2docker!

One more thing we need is instructions to install mercurial in the docs.

Yes of course! I will add that.

having to edit a config file to enable the extensions seems tricky.

With Mercurial, editing config files is anyway nearly mandatory. I think Mercurial developers really like config files 🙂 For example I don't think there are equivalents of git config --global user.name "FIRST_NAME LAST_NAME" or git remote set-url origin https://github.com/USERNAME/REPOSITORY.git. And to use advanced features you have to activate extensions from a config file. By default, you only have simple commands.

However, I could add --config extensions.topic in hg commands in mercurial.py so that repo2docker users wouldn't need the config file. I see no drawbacks to this solution.

Regarding having hg-evolve package as a dependency in setup.py, the idea may seem nice but it seems to me that it is not practically very useful. I think it is better to treat hg as git, to forget that hg is written partly in Python and to let users setup their Mercurial environment. Installing hg-evolve is never very complicated (depending how you install Mercurial, it can be already installed as on Windows with TortoiseHg or one uses pip install hg-evolve --user).

Of course, basic users should be able to use repo2docker with Mercurial without hg-evolve and topics.

For MyBinder, hg-evolve should be installed but most users don't have to know that.

[paugier]

I think it is now ready for review.

Mercurial is optional but by default tested. Mercurial tests can be disable with environment variables. There is no need for any Mercurial configuration file.

For the CI, Mercurial and hg-evolve are both installed with pip (in .github/workflows/test.yml) because in Ubuntu 18.04, the Mercurial installed with apt is too old for the last version of hg-evolve (Mercurial 4.5, running with Python 2.7).

[manics]

Do you think we could make hg-evolve compulsory for Mercurial support? This will simplify the docs (especially for anyone new to Mercurial), as well as removing one of the conditionals in the tests.

[minrk]

Nice! I think we can fine-tune things like requirements and installation a bit, but this looks like a great start to me!

[paugier]

Do you think we could make hg-evolve compulsory for Mercurial support? This will simplify the docs (especially for anyone new to Mercurial), as well as removing one of the conditionals in the tests.

Yes you are right. It is indeed simpler and nicer like that 05002a4

[manics]

Great!

Do you have an example hg repo url we can test this on?

[paugier]

Do you have an example hg repo url we can test this on?

I tried for example jupyter-repo2docker https://foss.heptapod.net/fluiddyn/fluidsim --ref phaseshift, which works at home.

[paugier]

Hi, do I need to do something more on this?

[manics]

I don't think so, just need to give people time to test it.

[betatim]

Thanks a lot for adding mercurial support and the patience in responding to all the comments, questions and suggestions :)