Closed Bug 1109799 Opened 10 years ago Closed 9 years ago

Release hgtool as a python package

Categories

(Release Engineering :: General, defect)

Product:
Release Engineering
Component:
General
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME

People

(Reporter: armenzg, Assigned: s244sing, Mentored)

References

Details

(Whiteboard: [good first bug])

Attachments

(1 file, 7 obsolete files)

[PATCH]: Initial implementation as a package with code fixes.
60.12 KB, patch
Details | Diff | Splinter Review 
In order to fix bug 724190 we could turn hgtool [1] into a python package.

This way we can request it to be installed inside of mozharness virtualenv and finally be up-to-date.

This is important to do as it will allow re-using hgtool logic across the board everywhere we need to without having to checkout tools everywhere or drop a new copy.

We should also add tests for this.

I think it might be best to move this to github and add travis CI support.

I have never done this so please chime in if this is not a good idea.

[1] http://hg.mozilla.org/build/tools/file/f3ad9647443b/buildfarm/utils/hgtool.py
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #0)
> In order to fix bug 724190 we could turn hgtool [1] into a python package.
> 
> This way we can request it to be installed inside of mozharness virtualenv
> and finally be up-to-date.
... 
> We should also add tests for this.
> 
> I think it might be best to move this to github and add travis CI support.

gps, is this something that should belong in the version-control-tools repo? (And released as a package from there)
Flags: needinfo?(gps)
Simarpreet will be giving us a hand with this.
Assignee: nobody → s244sing
If you put hgtool in version-control-tools, you get a pretty decent testing environment that already knows how to run Mercurial tests for little effort. You also get my eyeballs on it, which should help ensure hg best practices are followed. So, yes, I would encourage you to move hgtool to version-control-tools.
Flags: needinfo?(gps)
Thanks gps!

For Simarpreet's knowledge:
https://hg.mozilla.org/hgcustom/version-control-tools
Adding an initial package patch for hgtool to reside as a separate python package inside of versions-control-tools.
Attachment #8541507 - Flags: review?(armenzg)
Attachment #8541507 - Flags: feedback?(gps)
On a sidenote, after the review of this package is done here I will be more than happy to submit this to PyPI.
Comment on attachment 8541507 [details] [diff] [review]
[PATCH]: Initial distribution package for hgtool.

Review of attachment 8541507 [details] [diff] [review]:
-----------------------------------------------------------------

::: HgTool/MANIFEST
@@ +1,1 @@
> +# file GENERATED by distutils, do NOT edit

I don't believe this file should be checked in.

::: HgTool/hgtool/hgtool.py
@@ +1,1 @@
> +#!/usr/bin/env python

It's kinda weird to see a shebang in a package file.

I think you should use entry_points in setup.py instead. https://python-packaging-user-guide.readthedocs.org/en/latest/distributing.html#scripts

@@ +13,5 @@
> +
> +from util.hg import mercurial, out, remove_path
> +
> +if __name__ == '__main__':
> +    from optparse import OptionParser

I thought we're using Python 2.7 everywhere now. If so, please replace with argparse.

@@ +17,5 @@
> +    from optparse import OptionParser
> +    import logging
> +
> +    parser = OptionParser(__doc__)
> +    parser.set_defaults(

This is typically done by passing default= to .add_option (or add_argument in argparse land).

@@ +71,5 @@
> +    if options.propsfile:
> +        try:
> +            import json
> +        except ImportError:
> +            import simplejson as json

We should have Python 2.7 everywhere. The built-in json package is fine for most uses. The simple usage here is well within the built-in's umbrella.
Attachment #8541507 - Flags: feedback?(gps) → feedback+
Comment on attachment 8541507 [details] [diff] [review]
[PATCH]: Initial distribution package for hgtool.

Review of attachment 8541507 [details] [diff] [review]:
-----------------------------------------------------------------

::: HgTool/hgtool/hgtool.py
@@ +13,5 @@
> +
> +from util.hg import mercurial, out, remove_path
> +
> +if __name__ == '__main__':
> +    from optparse import OptionParser

I think this might be a leftover from the original development of hgtool when 2.6 was the common denominator on the releng machines.
Comment on attachment 8541507 [details] [diff] [review]
[PATCH]: Initial distribution package for hgtool.

Review of attachment 8541507 [details] [diff] [review]:
-----------------------------------------------------------------

Hi Simarpreet,
Happy new year and thanks for tackling this.

Could you please address the issues that gps mentions in comment 7?

Could you please make the name of the package lower case? "hgtool" instead of "HgTool".

If you need to, you can look at which files need to be committed by looking at this repo:
https://github.com/bhearsum/redo

I don't think we need these files:
HgTool/hgtool/test/__init__.py
HgTool/dist/hgtool-1.0.0.tar.gz

By the way, don't add binary files in patches. I get this message when applying the patch.
> File HgTool/dist/hgtool-1.0.0.tar.gz: git binary diffs are not supported.

For the record, you also need lib/python/util/hg.py [1]. That's where the bulk of the logic comes from. This gets imported through this:
> from util.hg import mercurial, out, remove_path
We will need to port the tests as well.
I can currently run the tests on the tools repo with this command: nosetests lib/python/mozilla_buildtools/test/test_util_hg.py [2]

Your probably will want to write a small script to know when you've reached a working prototype:
* create and activate a virtualenv
* run such script
** the script does a basic import of the newly created package
** the script does a basic hg operation so you know you're on the right track

By the way, if it helps, let me know if you would like to chat on a synchronous way to go through all the many things I said. I know I can use too much jargon.

[1] http://hg.mozilla.org/build/tools/file/66f013a830df/lib/python/util/hg.py
[2] http://hg.mozilla.org/build/tools/file/66f013a830df/lib/python/mozilla_buildtools/test/test_util_hg.py

::: HgTool/CHANGES.txt
@@ +1,1 @@
> +v1, Thu Dec 25 14:04:53 EST 2014 -- Initial release. 

White space at the end. Please remove it.

::: HgTool/MANIFEST
@@ +1,1 @@
> +# file GENERATED by distutils, do NOT edit

I agree.
Attachment #8541507 - Flags: review?(armenzg) → review-
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #9)
> Comment on attachment 8541507 [details] [diff] [review]
> [PATCH]: Initial distribution package for hgtool.
> 
> Review of attachment 8541507 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi Simarpreet,
> Happy new year and thanks for tackling this.
> 
> Could you please address the issues that gps mentions in comment 7?

Yes, sorry about the delay. I'm almost done working on it and should email a patch out by tonight.
> 
> Could you please make the name of the package lower case? "hgtool" instead
> of "HgTool".
> 

Okay will do.

> If you need to, you can look at which files need to be committed by looking
> at this repo:
> https://github.com/bhearsum/redo
> 
> I don't think we need these files:
> HgTool/hgtool/test/__init__.py
> HgTool/dist/hgtool-1.0.0.tar.gz
> 

Okay will fix this.

> By the way, don't add binary files in patches. I get this message when
> applying the patch.
> > File HgTool/dist/hgtool-1.0.0.tar.gz: git binary diffs are not supported.
> 
> For the record, you also need lib/python/util/hg.py [1]. That's where the
> bulk of the logic comes from. This gets imported through this:
> > from util.hg import mercurial, out, remove_path
> We will need to port the tests as well.
> I can currently run the tests on the tools repo with this command: nosetests
> lib/python/mozilla_buildtools/test/test_util_hg.py [2]
> 

Yes in my new implementation I try to package util.hg and related sub dependencies within the hgtool package. I tried building just the hgtool package but it was failing on util.hg and util.commands dependencies. Including them seems to fix this issue. My next patch will address this concern.

> Your probably will want to write a small script to know when you've reached
> a working prototype:
> * create and activate a virtualenv
> * run such script
> ** the script does a basic import of the newly created package
> ** the script does a basic hg operation so you know you're on the right track
> 

Okay will look into this as a test case for the package.

> By the way, if it helps, let me know if you would like to chat on a
> synchronous way to go through all the many things I said. I know I can use
> too much jargon.
> 
> [1] http://hg.mozilla.org/build/tools/file/66f013a830df/lib/python/util/hg.py
> [2]
> http://hg.mozilla.org/build/tools/file/66f013a830df/lib/python/
> mozilla_buildtools/test/test_util_hg.py
> 
> ::: HgTool/CHANGES.txt
> @@ +1,1 @@
> > +v1, Thu Dec 25 14:04:53 EST 2014 -- Initial release. 
> 
> White space at the end. Please remove it.
> 

Okay will do.

> ::: HgTool/MANIFEST
> @@ +1,1 @@
> > +# file GENERATED by distutils, do NOT edit
> 
> I agree.

So to be clear, we don't need the MANIFEST file?
This is a WIP for hgtool as a package. I'm still trying to test but in the meantime I'm submitting it for a review to get any obvious bugs sorted out.
Attachment #8541507 - Attachment is obsolete: true
Attachment #8544345 - Flags: review?(armenzg)
Just a quick question while I'm testing this. So I've been trying to do the following with both the original hgtool.py as a script and the new packaged hgtool version (submitted as a patch here). Both yield similar results.

> [simar@TuxCore hgtool ]$ ./hgtool.py https://hg.mozilla.org/hgcustom/version-control-tools /tmp/vctest
> Traceback (most recent call last):
>   File "./hgtool.py", line 94, in <module>
>     autoPurge=options.auto_purge)
> TypeError: mercurial() got an unexpected keyword argument 'clone_by_rev'


I'm a not using the tool correctly? From the looks of it clone_by_rev looks like an optional param for hgtool to work properly.
Flags: needinfo?(armenzg)
Comment on attachment 8544345 [details] [diff] [review]
[WIP] hgtool: v2. Cleaned up patch. argparse implemented.

Review of attachment 8544345 [details] [diff] [review]:
-----------------------------------------------------------------

r- to remove unneeded files and also to add redo as a dependency.

(In reply to Simarpreet Singh from comment #10)
> (In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from
> comment #9)
> > Comment on attachment 8541507 [details] [diff] [review]
...
> > ::: HgTool/MANIFEST
> > @@ +1,1 @@
> > > +# file GENERATED by distutils, do NOT edit
> > 
> > I agree.
> 
> So to be clear, we don't need the MANIFEST file?

Correct.

(In reply to Simarpreet Singh from comment #12)
> Just a quick question while I'm testing this. So I've been trying to do the
> following with both the original hgtool.py as a script and the new packaged
> hgtool version (submitted as a patch here). Both yield similar results.
> 
> > [simar@TuxCore hgtool ]$ ./hgtool.py https://hg.mozilla.org/hgcustom/version-control-tools /tmp/vctest
> > Traceback (most recent call last):
> >   File "./hgtool.py", line 94, in <module>
> >     autoPurge=options.auto_purge)
> > TypeError: mercurial() got an unexpected keyword argument 'clone_by_rev'
> 
> 
> I'm a not using the tool correctly? From the looks of it clone_by_rev looks
> like an optional param for hgtool to work properly.

This works for me (Running from tools/buildfarm/utils):
./hgtool.py https://hg.mozilla.org/hgcustom/version-control-tools /tmp/vctest

::: hgtool/hgtool/util/hg.py
@@ +2,5 @@
> +import os, re, subprocess
> +from urlparse import urlsplit
> +
> +from commands import run_cmd, get_output, remove_path
> +from retry import retry

I believe the only files needed under hgtool/hgtool/util/ is commands.py and retry.py (Hence remove the other ones).

Could you also add to hgtool's dependencies the "redo" package? This can be done by adding it to setup.py with install_requires.

I would like to remove the need for retry.py (since it is basically a fork from the redo package).

::: hgtool/setup.py
@@ +9,5 @@
> +    author='Chris AtLee',
> +    author_email='catlee@mozilla.com',
> +    packages=find_packages(),
> +    # XXX: Update URL when submitted.
> +    url='',

url='https://hg.mozilla.org/hgcustom/version-control-tools/file/default/hgtool',
Attachment #8544345 - Flags: review?(armenzg) → review-
Flags: needinfo?(armenzg)
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #13)
> Comment on attachment 8544345 [details] [diff] [review]
> [WIP] hgtool: v2. Cleaned up patch. argparse implemented.
> 
> Review of attachment 8544345 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r- to remove unneeded files and also to add redo as a dependency.
> 

Thanks for the review Armen.

> (In reply to Simarpreet Singh from comment #10)
> > (In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from
> > comment #9)
> > > Comment on attachment 8541507 [details] [diff] [review]
> ...
> > > ::: HgTool/MANIFEST
> > > @@ +1,1 @@
> > > > +# file GENERATED by distutils, do NOT edit
> > > 
> > > I agree.
> > 
> > So to be clear, we don't need the MANIFEST file?
> 
> Correct.

Okay will take care of this.

> 
> (In reply to Simarpreet Singh from comment #12)
> > Just a quick question while I'm testing this. So I've been trying to do the
> > following with both the original hgtool.py as a script and the new packaged
> > hgtool version (submitted as a patch here). Both yield similar results.
> > 
> > > [simar@TuxCore hgtool ]$ ./hgtool.py https://hg.mozilla.org/hgcustom/version-control-tools /tmp/vctest
> > > Traceback (most recent call last):
> > >   File "./hgtool.py", line 94, in <module>
> > >     autoPurge=options.auto_purge)
> > > TypeError: mercurial() got an unexpected keyword argument 'clone_by_rev'
> > 
> > 
> > I'm a not using the tool correctly? From the looks of it clone_by_rev looks
> > like an optional param for hgtool to work properly.
> 
> This works for me (Running from tools/buildfarm/utils):
> ./hgtool.py https://hg.mozilla.org/hgcustom/version-control-tools /tmp/vctest
> 

Figured it out why I was having this issue. Turns out that the version of util library and associated files (commands.py and hg.py) that I was using was not up to date and was missing some key functionality. My new patch addresses these issues and I not longer have this bug.

> ::: hgtool/hgtool/util/hg.py
> @@ +2,5 @@
> > +import os, re, subprocess
> > +from urlparse import urlsplit
> > +
> > +from commands import run_cmd, get_output, remove_path
> > +from retry import retry
> 
> I believe the only files needed under hgtool/hgtool/util/ is commands.py and
> retry.py (Hence remove the other ones).
> 
> Could you also add to hgtool's dependencies the "redo" package? This can be
> done by adding it to setup.py with install_requires.
> 
> I would like to remove the need for retry.py (since it is basically a fork
> from the redo package).
> 

Implemented in my next patch.

> ::: hgtool/setup.py
> @@ +9,5 @@
> > +    author='Chris AtLee',
> > +    author_email='catlee@mozilla.com',
> > +    packages=find_packages(),
> > +    # XXX: Update URL when submitted.
> > +    url='',
> 
> url='https://hg.mozilla.org/hgcustom/version-control-tools/file/default/
> hgtool',

Thanks, added this in.
The patch is a collapse of the following:


hgtool: remove unnecessary retry.py. We now have redo as a dep.
----------------
hgtool: Critical fix for util libs. Updated with new files from mainline.
----------------
hgtool: Remove useless files and update setup.py based on CR.


Tested using the following test line:

/home/simar/.virtualenvs/hgtool-venv/bin/hgtool --repo https://hg.mozilla.org/hgcustom/version-control-tools --dest /tmp/vctest

Seems to clone the repo fine.
Attachment #8544958 - Flags: review?(armenzg)
Comment on attachment 8544958 [details] [diff] [review]
[PATCH][v3]: Removed unnecessary files, fixed util lib, added redo as a dep.

Review of attachment 8544958 [details] [diff] [review]:
-----------------------------------------------------------------

Almost there but not quire.

Can you please join both patches and give me one big patch? It is difficult to read a patch based upon another batch.

You probably need to stop using retry and start using retrying. See the error I get at the end of this output.
Note that I use --no-site-packages when I create the venv to make sure that system wide python packages are not used by mistake.
Before attaching the next patch, could you please follow the steps that I followed to see that you get a working output?

From this:
> from retry import retry, retrier
to this:
> from redo import retrying

armenzg@armenzg-thinkpad:~/repos/version-control-tools$ cd hgtool/
armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ python setup.py sdist
running sdist
running egg_info
writing requirements to hgtool.egg-info/requires.txt
writing hgtool.egg-info/PKG-INFO
writing top-level names to hgtool.egg-info/top_level.txt
writing dependency_links to hgtool.egg-info/dependency_links.txt
writing entry points to hgtool.egg-info/entry_points.txt
reading manifest file 'hgtool.egg-info/SOURCES.txt'
writing manifest file 'hgtool.egg-info/SOURCES.txt'
running check
creating hgtool-1.0.0
creating hgtool-1.0.0/hgtool
creating hgtool-1.0.0/hgtool.egg-info
creating hgtool-1.0.0/hgtool/util
making hard links in hgtool-1.0.0...
hard linking README.txt -> hgtool-1.0.0
hard linking setup.py -> hgtool-1.0.0
hard linking hgtool/__init__.py -> hgtool-1.0.0/hgtool
hard linking hgtool/hgtool.py -> hgtool-1.0.0/hgtool
hard linking hgtool.egg-info/PKG-INFO -> hgtool-1.0.0/hgtool.egg-info
hard linking hgtool.egg-info/SOURCES.txt -> hgtool-1.0.0/hgtool.egg-info
hard linking hgtool.egg-info/dependency_links.txt -> hgtool-1.0.0/hgtool.egg-info
hard linking hgtool.egg-info/entry_points.txt -> hgtool-1.0.0/hgtool.egg-info
hard linking hgtool.egg-info/requires.txt -> hgtool-1.0.0/hgtool.egg-info
hard linking hgtool.egg-info/top_level.txt -> hgtool-1.0.0/hgtool.egg-info
hard linking hgtool/util/__init__.py -> hgtool-1.0.0/hgtool/util
hard linking hgtool/util/commands.py -> hgtool-1.0.0/hgtool/util
hard linking hgtool/util/hg.py -> hgtool-1.0.0/hgtool/util
Writing hgtool-1.0.0/setup.cfg
creating dist
Creating tar archive
removing 'hgtool-1.0.0' (and everything under it)
armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ cd ..
armenzg@armenzg-thinkpad:~/repos/version-control-tools$ virtualenv venv --no-site-packages
New python executable in venv/bin/python
Installing setuptools, pip...done.
(venv)armenzg@armenzg-thinkpad:~/repos/version-control-tools$ pip install hgtool/dist/hgtool-1.0.0.tar.gz 
Unpacking ./hgtool/dist/hgtool-1.0.0.tar.gz
  Running setup.py (path:/tmp/pip-b5jqm0-build/setup.py) egg_info for package from file:///home/armenzg/repos/version-control-tools/hgtool/dist/hgtool-1.0.0.tar.gz
    
Downloading/unpacking redo>=1.4 (from hgtool==1.0.0)
  Downloading redo-1.4.tar.gz
  Running setup.py (path:/home/armenzg/repos/version-control-tools/venv/build/redo/setup.py) egg_info for package redo
    
Installing collected packages: redo, hgtool
  Running setup.py install for redo
    
    Installing retry script to /home/armenzg/repos/version-control-tools/venv/bin
  Running setup.py install for hgtool
    
    Installing hgtool script to /home/armenzg/repos/version-control-tools/venv/bin
Successfully installed redo hgtool
Cleaning up...
(venv)armenzg@armenzg-thinkpad:~/repos/version-control-tools$ hgtool --repo https://hg.mozilla.org/hgcustom/version-control-tools --dest /tmp/vctest
Traceback (most recent call last):
  File "/home/armenzg/repos/version-control-tools/venv/bin/hgtool", line 9, in <module>
    load_entry_point('hgtool==1.0.0', 'console_scripts', 'hgtool')()
  File "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-packages/pkg_resources.py", line 353, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-packages/pkg_resources.py", line 2321, in load_entry_point
    return ep.load()
  File "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-packages/pkg_resources.py", line 2048, in load
    entry = __import__(self.module_name, globals(),globals(), ['__name__'])
  File "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-packages/hgtool/hgtool.py", line 13, in <module>
    from util.hg import mercurial, out, remove_path
  File "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-packages/hgtool/util/hg.py", line 10, in <module>
    from retry import retry, retrier
ImportError: No module named retry
(venv)armenzg@armenzg-thinkpad:~/repos/version-control-tools$ pip freeze
argparse==1.2.1
hgtool==1.0.0
redo==1.4
wsgiref==0.1.2

::: hgtool/hgtool/hgtool.py
@@ -66,4 @@
>      parser.add_argument(
>          "--purge", dest="auto_purge", action="store_true",
>          help="Purge the destination directory (if it exists).",
> -        default=False)

Why are you removing this default?

::: hgtool/hgtool/util/commands.py
@@ +1,1 @@
>  """Functions for running commands"""

Are the changes I see in here for commands.py simply bringing it up-to-date?
Attachment #8544958 - Flags: review?(armenzg) → review-
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #16)
> Comment on attachment 8544958 [details] [diff] [review]
> [PATCH][v3]: Removed unnecessary files, fixed util lib, added redo as a dep.
> 
> Review of attachment 8544958 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Almost there but not quire.

Thank you for bearing through this. I really appreciate your help in reviewing this patch.
> 
> Can you please join both patches and give me one big patch? It is difficult
> to read a patch based upon another batch.
> 

Yes will do so.

> You probably need to stop using retry and start using retrying. See the
> error I get at the end of this output.
> Note that I use --no-site-packages when I create the venv to make sure that
> system wide python packages are not used by mistake.
> Before attaching the next patch, could you please follow the steps that I
> followed to see that you get a working output?
> 
> From this:
> > from retry import retry, retrier
> to this:
> > from redo import retrying
> 
> armenzg@armenzg-thinkpad:~/repos/version-control-tools$ cd hgtool/
> armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ python
> setup.py sdist
> running sdist
> running egg_info
> writing requirements to hgtool.egg-info/requires.txt
> writing hgtool.egg-info/PKG-INFO
> writing top-level names to hgtool.egg-info/top_level.txt
> writing dependency_links to hgtool.egg-info/dependency_links.txt
> writing entry points to hgtool.egg-info/entry_points.txt
> reading manifest file 'hgtool.egg-info/SOURCES.txt'
> writing manifest file 'hgtool.egg-info/SOURCES.txt'
> running check
> creating hgtool-1.0.0
> creating hgtool-1.0.0/hgtool
> creating hgtool-1.0.0/hgtool.egg-info
> creating hgtool-1.0.0/hgtool/util
> making hard links in hgtool-1.0.0...
> hard linking README.txt -> hgtool-1.0.0
> hard linking setup.py -> hgtool-1.0.0
> hard linking hgtool/__init__.py -> hgtool-1.0.0/hgtool
> hard linking hgtool/hgtool.py -> hgtool-1.0.0/hgtool
> hard linking hgtool.egg-info/PKG-INFO -> hgtool-1.0.0/hgtool.egg-info
> hard linking hgtool.egg-info/SOURCES.txt -> hgtool-1.0.0/hgtool.egg-info
> hard linking hgtool.egg-info/dependency_links.txt ->
> hgtool-1.0.0/hgtool.egg-info
> hard linking hgtool.egg-info/entry_points.txt -> hgtool-1.0.0/hgtool.egg-info
> hard linking hgtool.egg-info/requires.txt -> hgtool-1.0.0/hgtool.egg-info
> hard linking hgtool.egg-info/top_level.txt -> hgtool-1.0.0/hgtool.egg-info
> hard linking hgtool/util/__init__.py -> hgtool-1.0.0/hgtool/util
> hard linking hgtool/util/commands.py -> hgtool-1.0.0/hgtool/util
> hard linking hgtool/util/hg.py -> hgtool-1.0.0/hgtool/util
> Writing hgtool-1.0.0/setup.cfg
> creating dist
> Creating tar archive
> removing 'hgtool-1.0.0' (and everything under it)
> armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ cd ..
> armenzg@armenzg-thinkpad:~/repos/version-control-tools$ virtualenv venv
> --no-site-packages
> New python executable in venv/bin/python
> Installing setuptools, pip...done.
> (venv)armenzg@armenzg-thinkpad:~/repos/version-control-tools$ pip install
> hgtool/dist/hgtool-1.0.0.tar.gz 
> Unpacking ./hgtool/dist/hgtool-1.0.0.tar.gz
>   Running setup.py (path:/tmp/pip-b5jqm0-build/setup.py) egg_info for
> package from
> file:///home/armenzg/repos/version-control-tools/hgtool/dist/hgtool-1.0.0.
> tar.gz
>     
> Downloading/unpacking redo>=1.4 (from hgtool==1.0.0)
>   Downloading redo-1.4.tar.gz
>   Running setup.py
> (path:/home/armenzg/repos/version-control-tools/venv/build/redo/setup.py)
> egg_info for package redo
>     
> Installing collected packages: redo, hgtool
>   Running setup.py install for redo
>     
>     Installing retry script to
> /home/armenzg/repos/version-control-tools/venv/bin
>   Running setup.py install for hgtool
>     
>     Installing hgtool script to
> /home/armenzg/repos/version-control-tools/venv/bin
> Successfully installed redo hgtool
> Cleaning up...
> (venv)armenzg@armenzg-thinkpad:~/repos/version-control-tools$ hgtool --repo
> https://hg.mozilla.org/hgcustom/version-control-tools --dest /tmp/vctest
> Traceback (most recent call last):
>   File "/home/armenzg/repos/version-control-tools/venv/bin/hgtool", line 9,
> in <module>
>     load_entry_point('hgtool==1.0.0', 'console_scripts', 'hgtool')()
>   File
> "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-
> packages/pkg_resources.py", line 353, in load_entry_point
>     return get_distribution(dist).load_entry_point(group, name)
>   File
> "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-
> packages/pkg_resources.py", line 2321, in load_entry_point
>     return ep.load()
>   File
> "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-
> packages/pkg_resources.py", line 2048, in load
>     entry = __import__(self.module_name, globals(),globals(), ['__name__'])
>   File
> "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-
> packages/hgtool/hgtool.py", line 13, in <module>
>     from util.hg import mercurial, out, remove_path
>   File
> "/home/armenzg/repos/version-control-tools/venv/local/lib/python2.7/site-
> packages/hgtool/util/hg.py", line 10, in <module>
>     from retry import retry, retrier
> ImportError: No module named retry
> (venv)armenzg@armenzg-thinkpad:~/repos/version-control-tools$ pip freeze
> argparse==1.2.1
> hgtool==1.0.0
> redo==1.4
> wsgiref==0.1.2
> 

Ah good catch, I missed the --no-site-pacakges option while testing so it got through the cracks.

> ::: hgtool/hgtool/hgtool.py
> @@ -66,4 @@
> >      parser.add_argument(
> >          "--purge", dest="auto_purge", action="store_true",
> >          help="Purge the destination directory (if it exists).",
> > -        default=False)
> 
> Why are you removing this default?
> 

In the initial implementation I didn't see a default for the --purge option. I removed it to stay consistent after initially relying on my gut instinct of including the default=False in. I'll add this back to the next combined patch I submit. I think by taking the safe approach to keep default=False for auto_purge is a safe option.

> ::: hgtool/hgtool/util/commands.py
> @@ +1,1 @@
> >  """Functions for running commands"""
> 
> Are the changes I see in here for commands.py simply bringing it up-to-date?

Yes. I just happened to have an older copy of commands.py and hg.py. You can grab the current sources from upstream and they should match with the changes I had in here.
This is the cumulative patch from all the changes so far.
Attachment #8544345 - Attachment is obsolete: true
Attachment #8544958 - Attachment is obsolete: true
Attachment #8545664 - Flags: review?(armenzg)
Comment on attachment 8545664 [details] [diff] [review]
[PATCH][v4]: Fixed some bugs with redo lib as per CR. Collapsed all patched into one.

Review of attachment 8545664 [details] [diff] [review]:
-----------------------------------------------------------------

OK. We have reached the milestone of getting a working solution. The steps to test this patch are listed below. [1]

Since we're changing the status quo of where hgtool will live we need to get buy in from both the repository owner we're moving to and release engineering who designed the initial hgtool.

We will also have to make sure that we indeed still have the expected behaviour.
In the tools repository there is a set of python tests that are run like this:
nosetests lib/python/mozilla_buildtools/test/test_util_hg.py
We will need to bring them forward and make sure they pass.
I think we should use tox to test the python package.
http://tox.readthedocs.org/en/latest

Simar, does this make sense to you?
While you work on porting the tests I will get buy in.
Does this work for you?

[1]
cd hgtool
python setup.py sdist
cd ..
virtualenv venv --no-site-packages
source venv/bin/activate
pip install hgtool/dist/hgtool-1.0.0.tar.gz
hgtool --repo https://hg.mozilla.org/hgcustom/version-control-tools --dest /tmp/vctest
Attachment #8545664 - Flags: review?(armenzg) → review+
After all this we will be able to fix bug 724190 where we will strip hgtool from the mozharness repository and start using the released hgtool python package.

I will start uploading redo to the internal pypi (bug 1119262).
gps, I believe Simarpreet dealt with the issues you raised.

Are you OK for us to land once we get buy in from releng?
I know you're not fond of having dependencies on python packages because it makes everything a bit more complicated but would it be OK if we went ahead and have the redo dependency?
Flags: needinfo?(gps)
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #19)
> Comment on attachment 8545664 [details] [diff] [review]
> [PATCH][v4]: Fixed some bugs with redo lib as per CR. Collapsed all patched
> into one.
> 
> Review of attachment 8545664 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> OK. We have reached the milestone of getting a working solution. The steps
> to test this patch are listed below. [1]
> 
> Since we're changing the status quo of where hgtool will live we need to get
> buy in from both the repository owner we're moving to and release
> engineering who designed the initial hgtool.

Can I be assigned to bug 724190? Seems like its unassigned for the moment.

> 
> We will also have to make sure that we indeed still have the expected
> behaviour.
> In the tools repository there is a set of python tests that are run like
> this:
> nosetests lib/python/mozilla_buildtools/test/test_util_hg.py
> We will need to bring them forward and make sure they pass.
> I think we should use tox to test the python package.
> http://tox.readthedocs.org/en/latest
> 
> Simar, does this make sense to you?
> While you work on porting the tests I will get buy in.
> Does this work for you?
> 

Okay I will look into the above mentioned resources and start working on porting the tests.

> [1]
> cd hgtool
> python setup.py sdist
> cd ..
> virtualenv venv --no-site-packages
> source venv/bin/activate
> pip install hgtool/dist/hgtool-1.0.0.tar.gz
> hgtool --repo https://hg.mozilla.org/hgcustom/version-control-tools --dest
> /tmp/vctest
Flags: needinfo?(armenzg)
Assigned :)
Flags: needinfo?(armenzg)
If you want examples used by the Release Engineering team you can see them in here:
http://mxr.mozilla.org/build/find?text=&string=tox.ini

The last one for the tools repo might help a bit more.

It seems they test 2 different versions of hg.
After the discussion on release.engineering [1] we have decided to create the hgtool python package inside of the tools repository to avoid the maintenance burden for non-python-package based systems.

[1] https://groups.google.com/forum/#!topic/mozilla.release.engineering/KmgK0hPzYHY
Flags: needinfo?(gps)
Based on the previous discussions, this patch replicates the implementation of hgtool in the tools/ repo. This patch is based on the previous comments and suggestions made in this bug tracker and includes necessary fixes from the suggestions made in this bug tracker.
Attachment #8545664 - Attachment is obsolete: true
Attachment #8550981 - Flags: review?(armenzg)
Comment on attachment 8550981 [details] [diff] [review]
[PATCH]: hgtool: Initial implementation as a package.

Review of attachment 8550981 [details] [diff] [review]:
-----------------------------------------------------------------

You don't need any of the files under "utils" (algorithms, commands).
They all, already live under "lib/python/util".

Since we're now back to the original repo, we don't need to duplicate the code since we are using this:
> sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python"))

r- only because of the duplication of code.

Could you please attach this patch again in here without hgtool/hgtool/util/?
Could you please then request the review from rail?

These are the steps I took to use this package:
rm -rf hgtool/hgtool/util/
virtualenv env --no-site-packages
source ven/bin/activate
cd hgtool
python setup.py develop
pip freeze [1]
which hgtool [2]
hgtool --repo https://hg.mozilla.org/hgcustom/version-control-tools --dest /tmp/vctest

[1] (venv)armenzg@armenzg-thinkpad:~/repos/tools/hgtool$ pip freeze
argparse==1.2.1
hgtool==1.0.0

[2]
(venv)armenzg@armenzg-thinkpad:~/repos/tools/hgtool$ which hgtool
/home/armenzg/repos/tools/venv/bin/hgtool
wsgiref==0.1.2

::: hgtool/setup.py
@@ +9,5 @@
> +    author='Chris AtLee',
> +    author_email='catlee@mozilla.com',
> +    packages=find_packages(),
> +    # XXX: Update URL when submitted.
> +    url='',

http://hg.mozilla.org/build/tools/file/default/hgtool
Attachment #8550981 - Flags: review?(armenzg) → review-
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #27)
> http://hg.mozilla.org/build/tools/file/default/hgtool

s/http/https/
Removing the unnecessary util/ dir and collapsing the two patches into one single combined one.
Attachment #8550981 - Attachment is obsolete: true
Attachment #8551599 - Flags: review?(rail)
Comment on attachment 8551599 [details] [diff] [review]
[PATCH}: Initial implementation as a package.

Review of attachment 8551599 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks a lot for your contribution! See my comments below.

::: hgtool/README.txt
@@ +1,5 @@
> +======
> +hgtool
> +======
> +
> +hgtool allows to do safe operations with hg.

This file repeats the docstring. I'd rather remove this file all together to avoid duplication.

@@ +7,5 @@
> +Usage
> +=====
> +
> +hgtool [-p|--props-file] [-r|--rev revision] [-b|--branch branch]
> +       [-s|--shared-dir shared_dir] [--check-outgoing] repo [dest]

I'd remove usage from here. In the future we will definitely forget to keep it up to date.

::: hgtool/hgtool/hgtool.py
@@ +1,1 @@
> +"""%prog [-p|--props-file] [-r|--rev revision] [-b|--branch branch]

this file is very similar to buildfarm/utils/hgtool.py. It would be great to get not have similar files around. It will lead to confusion in the future is someone wants to update the code.

@@ +7,5 @@
> +
> +# Import snippet to find tools lib
> +import os
> +import sys
> +sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python"))

Hmmm, the line above implies that you do not install the script globally (or in virtualenv) using setup.py and rely on libraries in a relative directory. If you try to install the package in a fresh virtualenv without cloning the tools repo, it won't work.

@@ +13,5 @@
> +from util.hg import mercurial, out, remove_path
> +
> +def main():
> +    import argparse
> +    import logging

I'd move the imports to the top of the file for 2 reasons:
1) readability, easier to spot used/required modules
2) main() is the only function in this file. If one imports this module, the only reason would be using main()

@@ +28,5 @@
> +        default=os.environ.get('HG_REV'))
> +
> +    parser.add_argument(
> +        "-b", "--branch", dest="branch", help="which branch to update to",
> +        default=os.environ.get('HG_BRANCH', None))

None is redundant here.

@@ +81,5 @@
> +
> +    options = parser.parse_args()
> +
> +    if options.repo is None:
> +        parser.error("No repo argument specified.")

This block is not necessary, you already have required=True.

@@ +84,5 @@
> +    if options.repo is None:
> +        parser.error("No repo argument specified.")
> +
> +    if options.dest is None:
> +        options.dest = os.path.basename(options.repo)

If you try to clone https://hg.mozilla.org/mozilla-central/ without --dest specified, it'll try to clone it to "/" (because of the trailing slash). You need to do something like:

options.dest = os.path.basename(options.repo.rstrip("/"))

@@ +93,5 @@
> +    if options.propsfile:
> +        try:
> +            import json
> +        except ImportError:
> +            import simplejson as json

You can just "import json" here, we require python 2.7 which comes with json.

@@ +108,5 @@
> +        if options.shared_dir and out(options.shared_dir, options.repo):
> +            remove_path(options.shared_dir)
> +
> +    got_revision = mercurial(options.repo, options.dest, options.branch, options.revision,
> +                             shareBase=options.shared_dir,

I'd use named arguments for everything in this call.

@@ +114,5 @@
> +                             mirrors=options.mirrors,
> +                             bundles=options.bundles,
> +                             autoPurge=options.auto_purge)
> +
> +    print "Got revision %s" % got_revision

Use logging instead?

::: hgtool/setup.py
@@ +1,1 @@
> +#!/usr/bin/env/ python

There is an extra trailing slash.

@@ +6,5 @@
> +setup(
> +    name='hgtool',
> +    version='1.0.0',
> +    author='Chris AtLee',
> +    author_email='catlee@mozilla.com',

Would be great to run the final version through the initial author.

@@ +9,5 @@
> +    author='Chris AtLee',
> +    author_email='catlee@mozilla.com',
> +    packages=find_packages(),
> +    # XXX: Update URL when submitted.
> +    url='',

You can specify it, I think.

@@ +10,5 @@
> +    author_email='catlee@mozilla.com',
> +    packages=find_packages(),
> +    # XXX: Update URL when submitted.
> +    url='',
> +    license='LICENSE',

It should be 'MPL'.

@@ +13,5 @@
> +    url='',
> +    license='LICENSE',
> +    description='hgtool allows to do safe operations with hg',
> +    long_description=open('README.txt').read(),
> +    entry_points = {

No spaces around = here.
Attachment #8551599 - Flags: review?(rail) → review-
Comment on attachment 8551599 [details] [diff] [review]
[PATCH}: Initial implementation as a package.

Review of attachment 8551599 [details] [diff] [review]:
-----------------------------------------------------------------

::: hgtool/hgtool/hgtool.py
@@ +7,5 @@
> +
> +# Import snippet to find tools lib
> +import os
> +import sys
> +sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python"))

I think I see what you mean since the packaging would not pick up files under lib/python.

Should we add a symlink from hgtool/hgtool/hg.py to ../../lib/python/util/hg.py?
We would them just do:
> from hg import mercurial, out, remove_path

Does this make sense?

@@ +114,5 @@
> +                             mirrors=options.mirrors,
> +                             bundles=options.bundles,
> +                             autoPurge=options.auto_purge)
> +
> +    print "Got revision %s" % got_revision

For some reason logging was a pain for me to start learning.
Here's a simple example to get you started with:
https://github.com/armenzg/mozilla_ci_tools/blob/master/scripts/trigger.py#L13
Comment on attachment 8551599 [details] [diff] [review]
[PATCH}: Initial implementation as a package.

Review of attachment 8551599 [details] [diff] [review]:
-----------------------------------------------------------------

::: hgtool/hgtool/hgtool.py
@@ +7,5 @@
> +
> +# Import snippet to find tools lib
> +import os
> +import sys
> +sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python"))

I don't think that symlinking would help here.
I can see some strategies here:

1) add a dependency on buildtool (there a setup.py in the parent directory). Not the best option, because you'd need to maintain 2 packages.
2) explicitly add all required files in the distribution. This may require moving files around (not sure if you can use files from a parent directory in setup.py).
3) continue using a standalone version of hgtool.py generated by http://hg.mozilla.org/build/tools/file/b0dc9e1cd9b9/buildfarm/utils/Makefile
I tested that the current patch won't work unless run from inside the tools repo (in comparison to comment 28).

armenzg@armenzg-thinkpad:/tmp/temp$ virtualenv env --no-site-packages
New python executable in env/bin/python
Installing setuptools, pip...done.
armenzg@armenzg-thinkpad:/tmp/temp$ source env/bin/activate
(env)armenzg@armenzg-thinkpad:/tmp/temp$ pip install ~/repos/tools/hgtool/dist/hgtool-1.0.0.tar.gz
Unpacking /home/armenzg/repos/tools/hgtool/dist/hgtool-1.0.0.tar.gz
  Running setup.py (path:/tmp/pip-KpxupY-build/setup.py) egg_info for package from file:///home/armenzg/repos/tools/hgtool/dist/hgtool-1.0.0.tar.gz
    
Installing collected packages: hgtool
  Running setup.py install for hgtool
    
    Installing hgtool script to /tmp/temp/env/bin
Successfully installed hgtool
Cleaning up...
(env)armenzg@armenzg-thinkpad:/tmp/temp$ pip freeze
argparse==1.2.1
hgtool==1.0.0
wsgiref==0.1.2
(env)armenzg@armenzg-thinkpad:/tmp/temp$ which hgtool
/tmp/temp/env/bin/hgtool
(env)armenzg@armenzg-thinkpad:/tmp/temp$ hgtool --repo https://hg.mozilla.org/hgcustom/version-control-tools --dest /tmp/vctest
Traceback (most recent call last):
  File "/tmp/temp/env/bin/hgtool", line 9, in <module>
    load_entry_point('hgtool==1.0.0', 'console_scripts', 'hgtool')()
  File "/tmp/temp/env/local/lib/python2.7/site-packages/pkg_resources.py", line 353, in load_entry_point
    return get_distribution(dist).load_entry_point(group, name)
  File "/tmp/temp/env/local/lib/python2.7/site-packages/pkg_resources.py", line 2321, in load_entry_point
    return ep.load()
  File "/tmp/temp/env/local/lib/python2.7/site-packages/pkg_resources.py", line 2048, in load
    entry = __import__(self.module_name, globals(),globals(), ['__name__'])
  File "/tmp/temp/env/local/lib/python2.7/site-packages/hgtool/hgtool.py", line 13, in <module>
    from util.hg import mercurial, out, remove_path
ImportError: No module named util.hg
I've also tested comment 32 - #3 to know what to expect. I didn't know about this :)


armenzg@armenzg-thinkpad:~/repos/tools/buildfarm/utils$ make
PYTHONPATH=/home/armenzg/repos/tools/buildfarm/utils/../../lib/python python package-script.py -m util -m util.commands -m util.retry -m util.hg hgtool.py hgtool-nodeps.py
util /home/armenzg/repos/tools/lib/python/util/__init__.py
util.commands /home/armenzg/repos/tools/lib/python/util/commands.py
util.retry /home/armenzg/repos/tools/lib/python/util/retry.py
util.hg /home/armenzg/repos/tools/lib/python/util/hg.py
chmod +x hgtool-nodeps.py
PYTHONPATH=/home/armenzg/repos/tools/buildfarm/utils/../../lib/python python package-script.py -m util -m util.file -m util.commands -m util.retry -m util.git gittool.py gittool-nodeps.py
util /home/armenzg/repos/tools/lib/python/util/__init__.py
util.file /home/armenzg/repos/tools/lib/python/util/file.py
util.commands /home/armenzg/repos/tools/lib/python/util/commands.py
util.retry /home/armenzg/repos/tools/lib/python/util/retry.py
util.git /home/armenzg/repos/tools/lib/python/util/git.py
chmod +x gittool-nodeps.py
PYTHONPATH=/home/armenzg/repos/tools/buildfarm/utils/../../lib/python:/home/armenzg/repos/tools/buildfarm/utils python package-script.py -m util -m util.file -m util.commands -m util.retry -m unix_util retry.py retry-nodeps.py
util /home/armenzg/repos/tools/lib/python/util/__init__.py
util.file /home/armenzg/repos/tools/lib/python/util/file.py
util.commands /home/armenzg/repos/tools/lib/python/util/commands.py
util.retry /home/armenzg/repos/tools/lib/python/util/retry.py
unix_util /home/armenzg/repos/tools/buildfarm/utils/unix_util.py
chmod +x retry-nodeps.py
armenzg@armenzg-thinkpad:~/repos/tools/buildfarm/utils$ hg st
? buildfarm/utils/gittool-nodeps.py
? buildfarm/utils/hgtool-nodeps.py
? buildfarm/utils/retry-nodeps.py
(In reply to Rail Aliiev [:rail] from comment #32)
> Comment on attachment 8551599 [details] [diff] [review]
> [PATCH}: Initial implementation as a package.
> 
> Review of attachment 8551599 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: hgtool/hgtool/hgtool.py
> @@ +7,5 @@
> > +
> > +# Import snippet to find tools lib
> > +import os
> > +import sys
> > +sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python"))
> 
> I don't think that symlinking would help here.

Just documenting here (as talked on IRC), symlinking isn't the best idea with keeping Windows in mind.

> I can see some strategies here:
> 
> 1) add a dependency on buildtool (there a setup.py in the parent directory).
> Not the best option, because you'd need to maintain 2 packages.
> 2) explicitly add all required files in the distribution. This may require
> moving files around (not sure if you can use files from a parent directory
> in setup.py).

I've tried to look online if there is a way to access files from a parent directory but the only way out seems to use sys.path.append() as previously the script was doing. Not a good idea as Armen already demonstrated it breaks while trying to access anywhere outside of hgtool/ dir.

For now I've decided to go ahead with adding explicitly the files we need (based on discussion with rail on IRC).

> 3) continue using a standalone version of hgtool.py generated by
> http://hg.mozilla.org/build/tools/file/b0dc9e1cd9b9/buildfarm/utils/Makefile
Adding :catlee for feedback as he was the original creator of hgtool.

This adds back the required dependencies (as mentioned in comment 32 -- #2 option.
Attachment #8552861 - Flags: review?(rail)
Attachment #8552861 - Flags: feedback?(catlee)
Attachment #8552861 - Flags: feedback?(armenzg)
Simar, I think you need to updated the tools repo before attaching the next patch.
Have you been using bookmarks? Maybe you need a rebase based on default.
Comment on attachment 8552861 [details] [diff] [review]
[PATCH]: Code fixes from rail's review.

Review of attachment 8552861 [details] [diff] [review]:
-----------------------------------------------------------------

f+ for addressing the issues that rail pointed out, however, we need a patch that applies properly to the tools repo.
Attachment #8552861 - Flags: feedback?(armenzg) → feedback+
Combined the two patches into one and also made sure that I updated the tools/ repo before I applied them.
Attachment #8551599 - Attachment is obsolete: true
Attachment #8552861 - Attachment is obsolete: true
Attachment #8552861 - Flags: review?(rail)
Attachment #8552861 - Flags: feedback?(catlee)
Attachment #8553489 - Flags: review?(rail)
Attachment #8553489 - Flags: feedback?(catlee)
Attachment #8553489 - Flags: feedback?(armenzg)
After a long discussion we have figured out our long term approach.

This is the new plan:
* We create a new github repo called hgtool - https://github.com/armenzg/hgtool
* We put in there hgtool as a library plus hgtool the script
* The layout would look like this:
- hgtool
   |--> __init__.py
      | commands.py (forked from tools)
      | hgtool.py (based on hg.py)
  setup.py
  scripts
   |--> hgtool

We will be adding these three new dependencies:
* redo [1]
* mozfile [2]
* mozinfo [3]

The new hgtool.py module will use redo instead of util.retry.py [1]
redo is a newer version of util.retry.py

Some of the modifications to hg.py are these two lines:
> sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python"))
> from util.hg import mercurial, out, remove_path

Modifications to commands.py will encompass:
* Moving _rmtree_windows() into mozfile
* Instead of using _is_windows() we will use mozinfo.os == 'win'
* We could rework commands.py to use mozprocess, however, I would like to leave it out of scope
* Rework remove_path() to use mozfile

Once all of this happens, we will drop the package into lib/python/vendor which we should be able to use it seemlesly.

Speak up for any objections or if I got anything wrong!

[1] https://pypi.python.org/pypi/redo
[2] http://mozbase.readthedocs.org/en/latest/mozfile.html
http://hg.mozilla.org/mozilla-central/file/494632b9afed/testing/mozbase/mozfile/mozfile/mozfile.py#l135
[3] http://mozbase.readthedocs.org/en/latest/mozinfo.html
http://hg.mozilla.org/mozilla-central/file/default/testing/mozbase/mozinfo
Attachment #8553489 - Flags: review?(rail)
Attachment #8553489 - Flags: feedback?(catlee)
Attachment #8553489 - Flags: feedback?(armenzg)
I managed to create a mozilla repo instead:
https://github.com/mozilla/hgtool
I still would strongly prefer throwing hgtool into version-control-tools.
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #40)
> After a long discussion we have figured out our long term approach.
> 
> This is the new plan:
> * We create a new github repo called hgtool -
> https://github.com/armenzg/hgtool
> * We put in there hgtool as a library plus hgtool the script
> * The layout would look like this:
> - hgtool
>    |--> __init__.py
>       | commands.py (forked from tools)
>       | hgtool.py (based on hg.py)
>   setup.py
>   scripts
>    |--> hgtool
> 
> We will be adding these three new dependencies:
> * redo [1]
> * mozfile [2]
> * mozinfo [3]
> 
> The new hgtool.py module will use redo instead of util.retry.py [1]
> redo is a newer version of util.retry.py
> 
> Some of the modifications to hg.py are these two lines:
> > sys.path.append(os.path.join(os.path.dirname(__file__), "../../lib/python"))
> > from util.hg import mercurial, out, remove_path
> 

> Modifications to commands.py will encompass:
> * Moving _rmtree_windows() into mozfile
> * Instead of using _is_windows() we will use mozinfo.os == 'win'
> * We could rework commands.py to use mozprocess, however, I would like to
> leave it out of scope
> * Rework remove_path() to use mozfile
> 

I have some comments with regards to this, please see the PR on github. 

https://github.com/mozilla/hgtool/pull/1

> Once all of this happens, we will drop the package into lib/python/vendor
> which we should be able to use it seemlesly.
> 
> Speak up for any objections or if I got anything wrong!
> 
> [1] https://pypi.python.org/pypi/redo
> [2] http://mozbase.readthedocs.org/en/latest/mozfile.html
> http://hg.mozilla.org/mozilla-central/file/494632b9afed/testing/mozbase/
> mozfile/mozfile/mozfile.py#l135
> [3] http://mozbase.readthedocs.org/en/latest/mozinfo.html
> http://hg.mozilla.org/mozilla-central/file/default/testing/mozbase/mozinfo

Pull Request is created on Github to track further progress on this and can be found here: https://github.com/mozilla/hgtool/pull/1
(In reply to Gregory Szorc [:gps] from comment #42)
> I still would strongly prefer throwing hgtool into version-control-tools.

Would you be willing to have it on github as well? I'm finding github more and more useful when dealing with the community.
You aren't going to get many community contributors to hgtool. You will, however, get more contributions from me if it's in version-control-tools.

I'm not opposed to having a GitHub presence for repositories that are canonically located at hg.mozilla.org. We'll likely accept GitHub Pull Requests in time. But that time is not today.

Please put hgtool inside version-control-tools where it can be iterated on alongside all the other Mozilla code related to version control.
I discussed this earlier on IRC and releng and I prefer to have a standalone project.
If you prefer that to change please convince the original author of hgtool.
There were some discussions over the last few days were we finally came to an agreement as to were to put the tool.

Today we got agreement that the value of putting hgtool is version-control-tools outweighs the benefits of having it as a standalone repository.

The benefits being:
* full Mercurial stack continuous integration

Simar, it is very very unlikely that we will change our minds anymore.
We're sincerely sorry for driving you nuts! I hope you understand us!

Would you mind moving your work to the Mercurial repo and not use the Git repo?
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #47)
> There were some discussions over the last few days were we finally came to
> an agreement as to were to put the tool.
> 
> Today we got agreement that the value of putting hgtool is
> version-control-tools outweighs the benefits of having it as a standalone
> repository.
> 
> The benefits being:
> * full Mercurial stack continuous integration

Fair enough.

> 
> Simar, it is very very unlikely that we will change our minds anymore.
> We're sincerely sorry for driving you nuts! I hope you understand us!
> 
No worries!

> Would you mind moving your work to the Mercurial repo and not use the Git
> repo?

Actually, I tried applying the current submitted patch into version-control-tools and it seems to work fine. Do you mind trying it out as well? If so we can go ahead with what we have here.
Flags: needinfo?(armenzg)
It seems to be working (review to come after this comment).




armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ virtualenv ~/venv/hgtool --no-site-packages
New python executable in /home/armenzg/venv/hgtool/bin/python
Installing setuptools, pip...done.
armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ source ~/venv/hgtool/bin/activate
(hgtool)armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ python setup.py develop
running develop
running egg_info
writing dependency_links to hgtool.egg-info/dependency_links.txt
writing hgtool.egg-info/PKG-INFO
writing top-level names to hgtool.egg-info/top_level.txt
writing entry points to hgtool.egg-info/entry_points.txt
reading manifest file 'hgtool.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
writing manifest file 'hgtool.egg-info/SOURCES.txt'
running build_ext
Creating /home/armenzg/venv/hgtool/lib/python2.7/site-packages/hgtool.egg-link (link to .)
Adding hgtool 1.0.0 to easy-install.pth file
Installing hgtool script to /home/armenzg/venv/hgtool/bin

Installed /home/armenzg/repos/version-control-tools/hgtool
Processing dependencies for hgtool==1.0.0
Finished processing dependencies for hgtool==1.0.0
(hgtool)armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ which hgtool
/home/armenzg/venv/hgtool/bin/hgtool
(hgtool)armenzg@armenzg-thinkpad:~/repos/version-control-tools/hgtool$ hgtool --repo https://hg.mozilla.org/hgcustom/version-control-tools --dest /tmp/vctest
Reporting hg version in use
command: START
command: hg -q version
command: cwd: .
command: output:
Mercurial Distributed SCM (version 3.2.4)
command: END (0.08s elapsed)

command: START
command: hg clone https://hg.mozilla.org/hgcustom/version-control-tools /tmp/vctest
command: cwd: /home/armenzg/repos/version-control-tools/hgtool
command: env: {'HGPLAIN': '1'}
command: output:
requesting all changes
adding changesets
adding manifests
adding file changes
added 1947 changesets with 4038 changes to 1075 files
updating to bookmark @
802 files updated, 0 files merged, 0 files removed, 0 files unresolved

command: END (5.61 elapsed)

command: START
command: hg branch
command: cwd: /tmp/vctest
command: env: {'HGPLAIN': '1'}
command: output:
default

command: END (0.08 elapsed)

command: START
command: hg update -C
command: cwd: /tmp/vctest
command: output:
3 files updated, 0 files merged, 0 files removed, 0 files unresolved
updating bookmark @
command: END (0.28s elapsed)

command: START
command: hg parent --template {node|short}
command: cwd: /tmp/vctest
command: env: {'HGPLAIN': '1'}
command: output:
adef76133881
command: END (0.08 elapsed)

Got revision: adef76133881
Flags: needinfo?(armenzg)
Comment on attachment 8553489 [details] [diff] [review]
[PATCH]: Initial implementation as a package with code fixes.

Review of attachment 8553489 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you Simar. Let me know if the following does not make sense.

Could you please run pep8 against it for the extra lines?
You can ignore the too long if you want.

Could you please go over comment 40 and address what still applies in your next patch?

From a side conversation with Simar I revised some of the original comment:
> Modifications to commands.py will encompass:
> * Moving _rmtree_windows() into mozfile
I would like to leave this out of scope for now.
mozfile also has a _call_with_windows_retry() with different approach and we would need to harmonize the two.

> * Instead of using _is_windows() we will use mozinfo.os == 'win'
Still valid.

> * We could rework commands.py to use mozprocess, however, I would like to leave it out of scope
Still valid.

> * Rework remove_path() to use mozfile
I think we should drop it for now.
remove() from mozfile and remove_path() from commands.py are extremely similar, however, each of them use a different approach for Windows.

Once we have hgtool as a package, integrated well into tools it would make sense to aim at reworking commands.py to drop it.
I'm afraid we would be doing too many changes at once.
We have to switch mozharness and tools to test this new hgtool package and make sure we don't break anything for Windows and release processes.

::: hgtool/hgtool/hgtool.py
@@ +1,1 @@
> +# Import snippet to find tools lib

You don't need this comment anymore.

::: hgtool/setup.py
@@ +8,5 @@
> +    version='1.0.0',
> +    author='Chris AtLee',
> +    author_email='catlee@mozilla.com',
> +    packages=find_packages(),
> +    url='https://hg.mozilla.org/build/tools/file/default/hgtool',

We need this fixed.
Comment on attachment 8553489 [details] [diff] [review]
[PATCH]: Initial implementation as a package with code fixes.

Review of attachment 8553489 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

::: hgtool/hgtool/hgtool.py
@@ +1,1 @@
> +# Import snippet to find tools lib

This file should have the MPL boilerplate.

::: hgtool/hgtool/util/commands.py
@@ +1,1 @@
> +"""Functions for running commands"""

MPL header.
We're going to make drops of gittool.py and hgtool.py unto external_tools/

If there is interest again on releasing this as a package by all means.
For now I'm resolving it.
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
Component: Tools → General
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: