crystal-lang / shards

Dependency manager for the Crystal language
Other
763 stars 100 forks source link

Resolver for Mercurial repositories #458

Closed f-fr closed 3 years ago

f-fr commented 3 years ago

This is a (first) implementation of a resolver for Mercurial repositories. It is basically a copy of the git resolver with git commands replaced by corresponding hg commands.

It also adds some tests for the hg resolver, which are also a copy of the specs for the git resolver. Because hg knows bookmarks (roughly corresponds to git branches) and named branches (not known to git), tests running on branches are duplicated, once for bookmarks and once for named branches.

Note that the specs take some time to run. The reason is that the test suite runs a lot of hg commands and hg is written in Python. Each command has a certain startup time (of the Python interpreter), which is negligible on the command line but very notable when running a bunch of commands in a test suite.

There is way around this: making use of the hg command server feature. But this requires more modifications to the code. So for the sake of ease of reviewing the changes, I'll put the command server stuff in a separate pull request.

I would be happy to hear your comment ;)

f-fr commented 3 years ago

Of course, the tests fail. One probably needs ci to set up mercurial (the tests work locally for me). Any help would be appreciated

straight-shoota commented 3 years ago

I've added a commit to install mercurial on circle and travis. Seems the Github action runner already has it installed. Specs are still broken, but not because hg is missing. They succeed locally, though. So not sure what exactly is wrong in CI. But I'm not familiar with hg at all.

f-fr commented 3 years ago

Thanks a lot. For some reason hg does not seem to create parent directories on clone (it does in my system).

There are still three failing tests but according to the error messages something in wrong with the mercurial installation in these cases.

f-fr commented 3 years ago

Finally all tests pass ;)

I fixed some problems with the setup scripts so they work now

They also revealed some problems with running shell commands on Windows, which are fixed now.

f-fr commented 3 years ago

I added the changes suggested by @Sija in #459. Note that many of them also apply to GitResolver (which has been used as a template for HgResolver).

f-fr commented 3 years ago

One other question: the method #matches? in GitResolver is never called and actually does not work at all (because it refers to some local variable/method dependency which is not defined). I copied the method to HgResolver but, obviously, it doesn't work either.

What should I do in such cases? Submit a new bug report? A new separate PR that removes the method? Discuss such issues here first?

The point is that I do not know what the purpose of this method is/was. So just removing it might be "the wrong thing" ...

straight-shoota commented 3 years ago

It's probably just a leftover from a previous refactoring. Since it clearly doesn't work, it should be removed. You could just add a commit here, or post a separate PR.

Blacksmoke16 commented 3 years ago

We should also be sure to add a section to SPEC.md like https://github.com/crystal-lang/shards/blob/master/SPEC.md#git.

f-fr commented 3 years ago

so is there something I should/could do?

straight-shoota commented 3 years ago

Yes, please merge the master branch, resolving conflicts.