Closed Bug 724190 Opened 12 years ago Closed 9 years ago

Move hgtool.py and gittool.py into external_tools/

Categories

(Release Engineering :: Applications: MozharnessCore, defect, P4)

defect

Tracking

(firefox43 fixed)

RESOLVED FIXED
Tracking Status
firefox43 --- fixed

People

(Reporter: mozilla, Assigned: armenzg)

References

Details

(Whiteboard: [mozharness])

Attachments

(1 file)

I ported hgtool to mozharness back in bug 650882.
Since then a number of hgtool fixes have landed in build/tools but not ported to mozharness.
MercurialVCS appears to assume old mercurial.  We should probably support both old+new; not sure if that's covered by porting recent hgtool changes.

[14:54]	<hwine>	ERROR: test_push_with_revision (test_base_vcs_mercurial.TestHg)
[14:54]	<hwine>	----------------------------------------------------------------------
[14:54]	<hwine>	Traceback (most recent call last): File "/Users/Hal/repos/mozharness/test/test_base_vcs_mercurial.py", line 282, in test_push_with_revision
[14:54]	<hwine>	m.push(src=self.repodir, remote=self.wc, revision=self.revisions[-1])
[14:54]	<hwine>	File "/Users/Hal/repos/mozharness/mozharness/base/vcs/mercurial.py", line 293, in push
[14:54]	<hwine>	throw_exception=True)
[14:54]	<hwine>	File "/Users/Hal/repos/mozharness/mozharness/base/script.py", line 407, in run_command
[14:54]	* aki	looks
[14:54]	<hwine>	raise subprocess.CalledProcessError(p.returncode, command)
[14:54]	<hwine>	CalledProcessError: Command '['hg', '--config', 'ui.merge=internal:merge', 'push', '-r', 'cccf6d352383', '--new-branch', '/var/folders/3y/24xyfrcd2334c5pftngvtsdh0000gn/T/tmp38WWpJ/wc']' returned non-zero exit status 1
[14:54]	<hwine>	-------------------- >> begin captured stdout << ---------------------
[14:54]	<hwine>	aki: http://pastebin.mozilla.org/1727939
[14:55]	<hwine>	Running command: ['bash', '/Users/Hal/repos/mozharness/test/helper_files/init_hgrepo.sh', '/var/folders/3y/24xyfrcd2334c5pftngvtsdh0000gn/T/tmp38WWpJ/repo'] 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
[14:55]	<hwine>	Return code: 0
[14:55]	<hwine>	aki - afaik, recent hg clients exit 1 if asked to push with no changes
[14:55]	<aki>	hwine: hm, pip freeze tells me mercurial==1.7.3
[14:56]	<hwine>	Ahh, HAH!!!! :)
[14:56]	<aki>	maybe that test is old mercurial
[14:57]	<hwine>	it's not the test - base/vcs/mercurial is what throws
Depends on: 780800
Blocks: 777530
No longer blocks: 777530
Product: mozilla.org → Release Engineering
Are we still out-of-sync here?
Component: Other → Tools
QA Contact: hwine
Yes.
Should we just include hgtool as one of the external tools we bundle with mozharness?
Could do.  I had the unstated goal of retiring all of build/tools at one point, but other people apparently want to keep adding there instead of migrating fully to mh, so the port seems futile.
Component: Tools → Mozharness
Can we turn hgtool into a python package?
It seems like a good candidate for a contributor project.

It would be great if we managed to make mozharness have two virtualenvs: one for itself and one for the actual harness to be run.

It would be nice if we used tools out there instead of hgtool:
http://docs.ros.org/independent/api/vcstools/html
yeah, a public package for hgtool would be great.
Depends on: 1109799
Filed as bug 1109799.
Depends on: 1119262
Assignee: nobody → s244sing
I have a working patch in bug 1193968.
Moving it to external_tools is better than the python package approach because the transition path is so much easier.
Assignee: s244sing → armenzg
Summary: port recent hgtool changes to mozharness → Move hgtool.py and gittool.py into external_tools/
Porting forward our discussion from the other bug.


(In reply to Jordan Lund (:jlund) from comment #1)
> Comment on attachment 8647154 [details] [diff] [review] [diff] [review]
> [mozharness] add gittool.py and hgtool.py into external_tools
> 
> Review of attachment 8647154 [details] [diff] [review] [diff] [review]:
> -----------------------------------------------------------------
> 
> bear with me as I get up to speed. This is a design change so while there is
> no code to review, there are a lot of implications to consider.
> 
> this looks like the copies from current puppet. I suppose the version we
> deploy on some of our slaves[1]. I notice that these versions are different
> than the source located in tools[2]. given that:
> 
Correct.
The versions on the tools repo are the canonical source.
The ones on puppet are *generated* by serializing the libraries which those files import (e.g. http://mxr.mozilla.org/build/source/tools/lib/python/util/{hg|git}.py)
Generated as single files makes them not need to checkout the tools repo.

> 1) adding these in mozharness (and subsequently, in gecko), I fear that
> these will get out of date. How do you propose we keep them updated?
> 
I'm OK if they were to fall out of date.
In any case, dropping new generated version can very easily be tested by pushing to try.
It is not as bad to test as:
1) Drop a new version on puppet and push it to slaves (rollback if issues)
2) Check-in a new tools version, back out if we see an issues across trees

#1 & #2 are worse scenarioes than testing on try. It is also an idempotent deployment which can be backed out by sheriffs (if something was missed on try) w/o releng being involved or need for monitoring.

> 2) this will introduce a third place that we could be using (hg|git)tool.py.
> taking a quick peek here shows how we extensively use the tools copy and the
> slave copy (either tools copy as well from runner or the puppet copy)[3]. I
> could see leading to confusion for things like debugging. Thoughts?
> 
We should follow up with a patch making use the hgtool and gittools from external_tools.
Drop using the puppet and tools versions.

> 3) at the very least, if we go with your patch, I think we should also point
> all mozharness references to (hg|git)tool.py to use the external_tools
> copy[4]. Thoughts?
> 
Agreed.

> 4) then there is mozharness's already existing hgtool.py and gittool.py.
> iirc, these are wrappers for calling the tools (and now with this patch the
> external_tools copy). these have been out of date for a
> while(https://bugzil.la/724190). it was there that catlee suggested doing
> what you are trying to do in this patch. *and* it was you who suggested
> making hgtool.py a python package instead of doing this bug
> (https://bugzilla.mozilla.org/show_bug.cgi?id=724190#c6). Looks like there
> has been a lot of work in that (https://bugzil.la/1109799). Are we giving up
> on this effort?
> 
The already existing hgtool.py and gittool.py should use external_tools as well.
I wish someone went ahead and make that progress, however, transitioning to a python package from everywhere would be a way more involved task.
With this bug, we at least unify our current set up, add try support plus make it available for developers and testers.

> I am going to f+ for now until we discuss further.
> 
> [1]
> http://mxr.mozilla.org/build/find?string=tool.
> py&tree=build&hint=%2Fpuppet%7Cmodules%7Cpackages%7Cfiles
> [2]
> http://mxr.mozilla.org/build/find?string=%28hg%7Cgit%29tool.
> py&tree=build&hint=tools
> [3] http://mxr.mozilla.org/build/search?string=hgtool.py
> http://mxr.mozilla.org/build/search?string=gittool.py
> [4]
> http://mxr.mozilla.org/build/search?string=hgtool.
> py&find=mozharness&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build
> http://mxr.mozilla.org/build/search?string=gittool.
> py&find=mozharness&findi=&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=build
Attachment #8647480 - Flags: review?(jlund)
Comment on attachment 8647480 [details] [diff] [review]
[mozharness] add gittool.py and hgtool.py into external_tools

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

thanks for the detailed reply. could you follow up or at least file a patch to move all (hg|git)tool.py refs in mozhanress to point to external_tools. I wonder if there are some scripts that won't need to checkout tools repo anymore...
Attachment #8647480 - Flags: review?(jlund) → review+
Thanks for the work to drive through some improvements here. ++

We also have: http://tc-vcs.readthedocs.org/en/latest/ which supports local and remote caching, and handles both git and hg (which makes it relatively simple to migrate projects between hg and git with minimal impact). I wonder if we should move to tc-vcs instead.

Caveat: tc-vcs is written in node.js.

Note: Despite its name, it is not bound in any way to taskcluster, other than some taskcluster jobs use it. It was perhaps an unfortunate choice of name.
CC'ing some people with active experience of tc-vcs for input...

If tc-vcs due to being written in node.js is not appropriate, we could at least consider defining a standard calling mechanism, configuration and api. Alternatively we could consider replacing tc-vcs with the python version as we generally call tc-vcs as a command rather than import it as a library. Mozharness could however call it as a command, but would required node.js to be installed.

Ideally we'd avoid duplicate effort here. On reflection, porting tc-vcs to python, renaming it (to not have "tc" in its name), and checking it into gecko might be the best way forward. I believe on TC side, we burn it into the worker type, which would still be quite possible even its canonical source was in-tree, and it was written in python.

Thoughts?
tc-vcs is already integrated into mozharness [0], but it needs nodejs installed (and, of course, taskcluster-vcs).

[0] http://hg.mozilla.org/mozilla-central/file/4e883591bb5d/testing/mozharness/mozharness/base/vcs/tcvcs.py
I've landed my change as-is since bug 1194483 will soon need it to run Firefox UI tests on test machines.
I believe that we should have a better unified story, however, I know that this type of projects are hairy ones which never end.

Please file a follow up if you believe we should spend time in this (I don't think we should).

I've filed bug 1194697 to make mhg scripts use the in-tree files.
https://hg.mozilla.org/mozilla-central/rev/f8384e17bc16
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: