# mercurial module needs review and feedback! **URL:** \ **Category:** Ansible Project\ **Created:** [January 13, 2013, 2:03am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200 "2013-01-13T02:03:41Z")\ **Posts on this page:** 20\ **Page:** 1
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 13, 2013, 2:03am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/1 "2013-01-13T02:03:41Z")
[https://github.com/yeukhon/ansible-mercurial](https://github.com/yeukhon/ansible-mercurial) The early hg module was barely usable to me so I have to write one myself. Tests: 1. currently using it myself 2. ssh, https, private and public repos 3. doc is cleaned up suggested by Michael [here](https://github.com/ansible/ansible/issues/1860) . 4. more sane checkpoints in clone 5. fixed some bugs 6. cleaned up some bit of the code (including a bit nicer error display). Please help review and test if possible! You can always just symlink this module to ur library. Example: ---
**Author:** ![Joel\_Roller](https://avatars.discourse-cdn.com/v4/letter/j/90ced4/32.png) [@Joel\_Roller](https://forum.ansible.com/u/Joel_Roller)\ **Post date:** [January 16, 2013, 12:40pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/2 "2013-01-16T12:40:31Z")
I missed the early hg module, was it Brad Olson’s? [https://github.com/bradobro/ansible-module-mercurial](https://github.com/bradobro/ansible-module-mercurial) I see that there are some differences: - yeukhon internals nicely separates arguments into lists, perhaps cleaner function paths? - bradobro supports more arguments: revision, owner, (and clean if Brad would pull from roller) - bradobro returns changed false if repo id is unchanged I am using bradobro one for my stuff, but I there are no tests provided. -joel ---
**Author:** ![Michael\_DeHaan](https://avatars.discourse-cdn.com/v4/letter/m/c4cdca/32.png) [@Michael\_DeHaan](https://forum.ansible.com/u/Michael_DeHaan)\ **Post date:** [January 16, 2013, 12:44pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/3 "2013-01-16T12:44:19Z")
The other module didn't (IIRC) use the new Python module infrastructure, so it's not possible to include in core for maintaince reasons. I don't know what owner did (not looking ATM), but we can use file attributes on any module regardless now. Other things do seem like they should be fixed up. ---
**Author:** ![Joel\_Roller](https://avatars.discourse-cdn.com/v4/letter/j/90ced4/32.png) [@Joel\_Roller](https://forum.ansible.com/u/Joel_Roller)\ **Post date:** [January 16, 2013, 1:12pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/4 "2013-01-16T13:12:51Z")
> The other module didn’t (IIRC) use the new Python module > infrastructure, so it’s not possible to include in core for maintaince > reasons. Sure, it’d be nice to have a mercurial module in core. It actually does look like bradobro has done some updates re AnsibleModule() and not bothered to announce them. > I don’t know what owner did (not looking ATM), but we can use file > attributes on any module regardless now. bradobro’s owner argument used su to run mercurial as that user. This was very convenient, allowing me to create a deployment user with generate\_ssh\_key, and then fetch those keys to the repository host. I’ve had issues in the past with ssh agent forwarding for this purpose. Perhaps that’s a fairly overloaded use of the word owner though, using file module for actual ownership changing sounds interesting. ---
**Author:** ![Michael\_DeHaan](https://avatars.discourse-cdn.com/v4/letter/m/c4cdca/32.png) [@Michael\_DeHaan](https://forum.ansible.com/u/Michael_DeHaan)\ **Post date:** [January 16, 2013, 2:21pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/5 "2013-01-16T14:21:12Z")
> > I don't know what owner did (not looking ATM), but we can use file > > attributes on any module regardless now. > > bradobro's owner argument used su to run mercurial as that user. This was > very convenient, allowing me to create a deployment user with > generate\_ssh\_key, and then fetch those keys to the repository host. I've > had issues in the past with ssh agent forwarding for this purpose. Using su there is not acceptable -- modules should never do that, and there's no reason to assume someone was running as root. Rather, the task should specify the sudo\_user, or the play should just use that user directly. All of this exists in core ansible. ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 16, 2013, 4:18pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/6 "2013-01-16T16:18:30Z")
Here is my thought. I’d avoid using sudo, sudo\_user whenever possible. The core should be simple. I have to use ssh to deployment form private ssh repo on bitbucket so I have ran into cases when root doesn’t have access to ~/.ssh/config I am proposing three changes to the existing module. 1. change mercurial to hg (as you can see, I was developing it on top of the old code, erased nearly everything, but forgot to rename it). 2. append StrictHostKeyChecking=no to ssh if the user chooses to (meaning new option) To accept key without prompt, we can disable the prompt by specifying StrictHostKeyChecking=no in the config file or in the ssh command. This first goes through the check, and then accept even if no match is found in known hosts. I don’t know the safest method. But if one requires a strict key checking, ssh should check against a list of approved known hosts managed by ansible as part of the configuration, or simply accept it 1. rev to revision I went through git and this old module again and i find they both use revision rather than calling it rev. I will also try to make sure attributes are the same across all modules. There are still some refactoring to be done in my hg module. I am not trying to take away his credit (I mention it in my first line of README), except, to be in the core, and to be a good open source project, active development is required. ---
**Author:** ![Joel\_Roller](https://avatars.discourse-cdn.com/v4/letter/j/90ced4/32.png) [@Joel\_Roller](https://forum.ansible.com/u/Joel_Roller)\ **Post date:** [January 16, 2013, 5:45pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/7 "2013-01-16T17:45:14Z")
> Here is my thought. > I’d avoid using sudo, sudo\_user whenever possible. The core should be simple. I actually missed the 0.8 change that allows you to put sudo\_user in any task. That certainly does make everything simpler. > 1. rev to revision > I went through git and this old module again and i find they both use revision rather than calling it rev. > I will also try to make sure attributes are the same across all modules. This is option is missing from the doc block. (Though it is in the example) > I am not trying to take away his credit (I mention it in my first line of README), except, to be in the core, and to be a good open source project, active development is required. Sorry, I missed this, too. I guess the code was different enough that I assumed an entirely different source. I’d request a “clean” or “force” update feature as well, if you’d like I can submit a patch later. Also, it may be nice to alias the git modules options where possible. -joel ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 16, 2013, 10:46pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/8 "2013-01-16T22:46:06Z")
Hi Joe. > Sorry, I missed this, too. I guess the code was different enough that I assumed an entirely different source > It’s good, no sorry. I like to make sure everything is all right before saying GOOD. That’s why I put guards (hg verify and hg tip) as much as possible. > I’d request a “clean” or “force” update feature as well, if you’d like I can submit a patch later. Also, it may be nice to alias the git modules options where possible. > I see the usage for local development, and the use after manually fixing something on a production machine and wishing to discard them in the next deployment/configuration cycle. > But please educate me the common usage, espeically in an auto-deployment. We can add them since git and svn have them. I just want to know it before it goes to the code. > if you’d like I can submit a patch later. Also, it may be nice to alias the git modules options where possible. > Yes. Of course. That’s how we learn. As a student open source helps me grow. As a user, that’s the heart of Open source 🙂 Cheer, Yeukhon ---
**Author:** ![Joel\_Roller](https://avatars.discourse-cdn.com/v4/letter/j/90ced4/32.png) [@Joel\_Roller](https://forum.ansible.com/u/Joel_Roller)\ **Post date:** [January 17, 2013, 12:36pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/9 "2013-01-17T12:36:49Z")
I use this specifically for development. Auto-deployment is great for production, but it’s also really nice to bootstrap a dev environment. I use an clean\_update=no inventory variable on dev machines, that way I can test everything including ansible deploy before the commit. The most extreme case being a task that updates your ansible plays, you may want to test it without losing your work. -joel ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 17, 2013, 4:40pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/10 "2013-01-17T16:40:00Z")
That’s actually a nice idea. I’d like to include that, regardless 😃 A patch is welcome! Make a pull request 🙂 Thanks! ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 22, 2013, 5:40am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/11 "2013-01-22T05:40:54Z")
So I finally have a little more time to work on this. Been really busy this whole weekend with other stuff. I think to do cleanup, one might want hg up -C to discard uncommitted changes then hg purge However, purge (or clean) is not enabled by default, which requires enabling it in hgrc file. There is --config but it never works out for me. So if we go with purge, we have to (1) make a backup of the current hgrc file, then (2) edit the hgrc file and finally (3) restore the old hgrc file So the option is called force which will discard modified files, and untracked files. ---
**Author:** ![Brad\_Olson](https://avatars.discourse-cdn.com/v4/letter/b/779978/32.png) [@Brad\_Olson](https://forum.ansible.com/u/Brad_Olson)\ **Post date:** [January 23, 2013, 2:17am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/12 "2013-01-23T02:17:24Z")
As far as I’m concerned, the mercurial module was a dirty hack I never thought would make it into core because Michael thought people should just switch to git. (I do to, but, alas, have to deploy some things from Mercurial.) I made it for myself, the way I liked it, and didn’t know many others were using it. To set the record straight: 1. I’m totally happy to have someone come up with a better module or take over maintenance of this one. Yeukhon, if you can build it better, go for it! I haven’t looked at your code yet, but plan to. Thanks for the contribution. If you want to collaborate, I’m open. I’d love to see the module in core, but have no interest in arguing about it with Michael. 2. I have to deploy code as a particular use. For the same reason the file module has to specify an owner, I have to specify a different user of a repo. Sudo is the only way I see to do that, but I’m open to suggestions. 3. Michael wrote, "My module uses #\<\\>, but if there are new methods, I haven’t kept up. 4. Michael writes, “I don’t know what owner did (not looking ATM), but we can use file attributes on any module regardless now.” Not sure what that means, so there must be some developments I haven’t kept up with. If you point me to the docs or explain a bit, I can try adjust it. 5. Sorry I missed a patch from roller. I always clean, but I can see why you might want it as an option. I’ll check my github messages. Thanks for the heads up, joel. 6. Yeah, no tests. I need some. New responsibilities and not enough bandwidth. ---
**Author:** ![Brad\_Olson](https://avatars.discourse-cdn.com/v4/letter/b/779978/32.png) [@Brad\_Olson](https://forum.ansible.com/u/Brad_Olson)\ **Post date:** [January 23, 2013, 2:21am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/13 "2013-01-23T02:21:58Z")
Nice code, Yeukhon! ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 23, 2013, 7:39am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/14 "2013-01-23T07:39:39Z")
Hi. Thanks for the encouragement. I think we all hg users can help build it. I don’t know every great commands in hg. There are certainly shortcuts and improvements can be made. So I am totally opened to working with everyone! Just educate me if you guys have spare time 😃 I looked at all the existing modules, they all use \<\\> re to #4, I think he’s saying you can set use the file module to set owner, group and mode permission. With sudo\_user you perform su-like operation. I have tests, but I need those tests to roll genetic. I don’t know any other good ways to run tests besides rolling up a vagrant vm. I am going to finsih the force option tomorrow night if what I suggested above makes sense. basically, hg update -C to discard modified changes, and then hg purge (which requires enable the extensions in hgrc temporarily). Yeukhon ---
**Author:** ![Michael\_DeHaan](https://avatars.discourse-cdn.com/v4/letter/m/c4cdca/32.png) [@Michael\_DeHaan](https://forum.ansible.com/u/Michael_DeHaan)\ **Post date:** [January 23, 2013, 1:25pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/15 "2013-01-23T13:25:47Z")
> As far as I'm concerned, the mercurial module was a dirty hack I never > thought would make it into core because Michael thought people should just > switch to git. (I do to, but, alas, have to deploy some things from > Mercurial.) I made it for myself, the way I liked it, and didn't know many > others were using it. There's enough of an audience now to where having more SCM types is starting to make sense. Though everyone should pick git 🙂 > To set the record straight: > > 1. I'm totally happy to have someone come up with a better module or take > over maintenance of this one. Yeukhon, if you can build it better, go for > it! I haven't looked at your code yet, but plan to. Thanks for the > contribution. If you want to collaborate, I'm open. I'd love to see the > module in core, but have no interest in arguing about it with Michael. > > 2. I have to deploy code as a particular use. For the same reason the file > module has to specify an owner, I have to specify a different user of a > repo. Sudo is the only way I see to do that, but I'm open to suggestions. > > 3. Michael wrote, "My module uses #\<\\>, but if > there are new methods, I haven't kept up. The new 'run\_command' is very recent in 1.0. > 4. Michael writes, "I don't know what owner did (not looking ATM), but we > can use file attributes on any module regardless now." Not sure what that > means, so there must be some developments I haven't kept up with. If you > point me to the docs or explain a bit, I can try adjust it. Whatever the owner= flag did, if it was doing chmod's and such, it should not need to. But I wasn't looking. > 5. Sorry I missed a patch from roller. I always clean, but I can see why you > might want it as an option. I'll check my github messages. Thanks for the > heads up, joel. > > 6. Yeah, no tests. I need some. New responsibilities and not enough > bandwidth. I don't think we really need automated tests of hg checkouts that badly -- so I'm fine with this just being tested when people work on it. It won't change much. That being said, the git module has had a fair amount of turnover. ---
**Author:** ![Brad\_Olson](https://avatars.discourse-cdn.com/v4/letter/b/779978/32.png) [@Brad\_Olson](https://forum.ansible.com/u/Brad_Olson)\ **Post date:** [January 23, 2013, 7:37pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/16 "2013-01-23T19:37:39Z")
Thanks for the input, Michael. +1 on git, and thanks for the run\_command note. Joel & Yeukhon, My work schedule won’t allow me much bandwidth for this until after Feb 20, but I’m very interested in seeing a stable hg module. Do either of you have energy now to lead this module? Yeukhon, could you articulate what made the old module unusable for you? Conversely, are there any options you’d like added to your module? Joel, you probably have both modules in your head more, plus you’ve written the spec docs. Can you propose a unifying path forward on this? ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 24, 2013, 10:51pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/17 "2013-01-24T22:51:17Z")
Sure. I can lead the module. Anyone can help. As far as tests goes, yeah, I haven’t dived into ansible’s tests yet. Not sure exactly how they test it. I can’t remember the exact problems, but it threw me errors. Even reading the code I wasn’t able to correct it. Given time cosntraints, I decided to just write one up by calling hg commands. I think the biggest issue I have with my own implemtnation is all the health check points. Maybe too much for big repos but if it’s automated they seem logical to have. Also user feedback. Whether they find it useful or not. I will roll out the changes later. John ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 26, 2013, 2:06am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/18 "2013-01-26T02:06:19Z")
The repo has been updated with force option. [https://github.com/yeukhon/ansible-mercurial](https://github.com/yeukhon/ansible-mercurial) ---
**Author:** ![John\_Wong](https://avatars.discourse-cdn.com/v4/letter/j/6f9a4e/32.png) [@John\_Wong](https://forum.ansible.com/u/John_Wong)\ **Post date:** [January 26, 2013, 10:55pm UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/19 "2013-01-26T22:55:04Z")
It’s now in the development branch. [https://github.com/ansible/ansible/pull/1922](https://github.com/ansible/ansible/pull/1922) Thanks for all the feedback and help. Yeukhon ---
**Author:** ![Joel\_Roller](https://avatars.discourse-cdn.com/v4/letter/j/90ced4/32.png) [@Joel\_Roller](https://forum.ansible.com/u/Joel_Roller)\ **Post date:** [January 27, 2013, 4:42am UTC](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200/20 "2013-01-27T04:42:09Z")
Thank you all for getting this all together. I haven’t had a chance to check out the new module yet, but I’ll try to verify my use cases this weekend. A note about the force option: I find the purge behavior a bit surprising. This goes pretty far beyond what default hg clean updates do. Considering the feature involves enabling a module, it may be something left as a separate option rather than the default. It seems like it’d be a nice feature, and I like hack temporarily enabling extra modules to get it, but it seems like something that should not be the default. Congrats on going core, too. 🙂 If I want to propose patches at this point, would I be right to branch ansible/ansible rather than yeukhon/ansible-mercurial? -joel [Next page](https://forum.ansible.com/t/mercurial-module-needs-review-and-feedback/12200.md?page=2)