tc-vcs makes it pretty easy to wrap vcs of choice (this already works with caching for all of our gecko build jobs except for emulator/phone) so we should also make it work for phones/emulators then supporting use cases like "git try" and "user repos" will work easily (both git/hg variants)

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r c54a5679edf6710f1169d53f3b3908d57e097d41

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=catlee Pull down this commit: hg pull review -r 24d8c5a7c745b01025a0e135375f773727567847

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=catlee Pull down this commit: hg pull review -r ea513cf1433654a9e1b5ab49d66834debc2a8563

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=catlee Pull down this commit: hg pull review -r e9dd43e05a8b4b91b97ee980099f4f5440e98fbf

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=catlee Pull down this commit: hg pull review -r e9dd43e05a8b4b91b97ee980099f4f5440e98fbf

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r a2917ffa1fa01dff2ae7edf6d069b87279aad319

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 612c93dddb3ca8dc9c4abb5052b7eb3734290b71

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 612c93dddb3ca8dc9c4abb5052b7eb3734290b71

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 612c93dddb3ca8dc9c4abb5052b7eb3734290b71

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 612c93dddb3ca8dc9c4abb5052b7eb3734290b71

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 612c93dddb3ca8dc9c4abb5052b7eb3734290b71

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 612c93dddb3ca8dc9c4abb5052b7eb3734290b71

https://reviewboard.mozilla.org/r/1533/#review1075 ::: testing/docker/phone-builder/bin/validate_task.py (Diff revision 4) > - if 'REPOSITORY' not in payload['env']: > + if 'GECKO_HEAD_REPOSITORY' not in payload['env']: We should check both base and HEAD I think

Comment on attachment 8537810 [details] MozReview Request: bz://1109346/wcosta one comment to address otherwise lgtm

Comment on attachment 8538154 [details] MozReview Request: bz://1109346/wcosta tc-vcs bits lgtm

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=jlund Pull down this commit: hg pull review -r ce9ce960cda87f2417e1c087ffd5d626b9c5b9e1

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 1b003db06c5196cf75eb5446d1c072e148aa9faa

/r/1535 - Bug 1109346: Use tc-vcs for cloning repos. r=lightsofapollo Pull down this commit: hg pull review -r 1b003db06c5196cf75eb5446d1c072e148aa9faa

https://reviewboard.mozilla.org/r/1553/#review1111 looks really good to me :D this is great to see more vcs tools getting snapped into VCSMixin. I have some nits/comments in the review and so before I r+, I'd like to address those and also see this patch being actually used with log output. as for the tc-vcs cmd iteslf and valid args, I'll trust that jlal r+ was verified that. ::: mozharness/base/vcs/tcvcs.py (Diff revision 5) > + self.tc_vcs = self.query_exe('tc-vcs', return_type='list') looks like you never add this to self.config['exes'], so I'm assuming you will always have 'tc-vcs' in your path? ::: mozharness/base/vcs/tcvcs.py (Diff revision 5) > + if retval != 0: IIUC you can s/if retval != 0:/if retval:/ everywhere in this file. or, if you want, simply, `if self.run_command(cmd):` ::: mozharness/base/vcs/tcvcs.py (Diff revision 5) > + parser.got_revision unlike ruby, I don't think python will return the last statement in a function. IIRC, you must explicitly use `return X` ::: mozharness/base/vcs/tcvcs.py (Diff revision 5) > + retval = self.run_command(cmd, output_parser=parser) IIUC you are only expecting the revision as output from this command. instead of adding a custom OutputParser that assigns self.got_revision to the latest line of output (in this case, we only expect one line so it works), you can also use this[1]. e.g. return self.get_output_from_command(cmd) [1] http://mxr.mozilla.org/build/source/mozharness/mozharness/base/script.py#780 ::: scripts/b2g_build.py (Diff revision 5) > + "dest": "base_repo", I'm confused why we need --repo[1] and --base-repo in TcVCS, you have: base_repo = c.get('base_repo', c['repo']) are you expecting c['base_repo'] to be different than c['repo'] if c['base_repo'] exists? [1] http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildb2gbase.py#48

:jlund, Here is piece of log related to the patch 18:23:49 INFO - retry: Calling <bound method B2GBuild._get_revision of <__main__.B2GBuild object at 0x7f0d8f82ea90>> with args: (<mozharness.base.vcs.tcvcs.TcVCS object at 0x7f0d8f6e1f10>, '/home/wander/work/build/gecko'), kwargs: {}, attempt #1 18:23:49 INFO - Running command: ['tc-vcs', 'checkout-revision', '/home/wander/work/build/gecko', 'https://hg.mozilla.org/try', 'master', 'c355549c9381'] 18:23:49 INFO - Copy/paste: tc-vcs checkout-revision /home/wander/work/build/gecko https://hg.mozilla.org/try master c355549c9381 18:23:49 INFO - [tc-vcs] run start : hg pull -r c355549c9381 https://hg.mozilla.org/try 18:23:53 INFO - trazendo revisões de https://hg.mozilla.org/try 18:23:53 INFO - nenhuma alteração encontrada 18:23:53 INFO - [tc-vcs] run end : hg pull -r c355549c9381 https://hg.mozilla.org/try (0) in 4468 ms 18:23:53 INFO - [tc-vcs] run start : hg update -C c355549c9381 18:23:54 INFO - 0 arquivos atualizados, 0 arquivos mesclados, 0 arquivos removidos, 0 arquivos não resolvidos 18:23:54 INFO - [tc-vcs] run end : hg update -C c355549c9381 (0) in 959 ms 18:23:54 INFO - Return code: 0

https://reviewboard.mozilla.org/r/1553/#review1113 > looks like you never add this to self.config['exes'], so I'm assuming you will always have 'tc-vcs' in your path? That's correct, at least in current situation, do you think would be better to add it to config['exes']? > IIUC you can s/if retval != 0:/if retval:/ everywhere in this file. or, if you want, simply, `if self.run_command(cmd):` Sure, just copy paste from other module. I'll fix that. > IIUC you are only expecting the revision as output from this command. instead of adding a custom OutputParser that assigns self.got_revision to the latest line of output (in this case, we only expect one line so it works), you can also use this[1]. > > e.g. > return self.get_output_from_command(cmd) > > [1] http://mxr.mozilla.org/build/source/mozharness/mozharness/base/script.py#780 Ah, I didn't know that, great, I'll use it. > unlike ruby, I don't think python will return the last statement in a function. IIRC, you must explicitly use `return X` Ops, you're right. > I'm confused why we need --repo[1] and --base-repo > > in TcVCS, you have: > base_repo = c.get('base_repo', c['repo']) > > are you expecting c['base_repo'] to be different than c['repo'] if c['base_repo'] exists? > > > [1] http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildb2gbase.py#48 Yep, this is for try support, we use mozilla-central as base repo, and pull latest commits from try. So we have faster cloning because m-c is cached.

thanks for the log snippet. based on your last comment, it sounds like you are going to remove/change a few things: 1) retval conditions 2) output parser 3) return statement in ensure_repo_and_rev. I'll clear the review out of my queue for now. feel free to either interdiff with a new patch or else submit the whole thing again :) (In reply to Wander Lairson Costa [:wcosta] from comment #23) > https://reviewboard.mozilla.org/r/1553/#review1113 > > That's correct, at least in current situation, do you think would be better > to add it to config['exes']? just asserting that it will always be there as I don't know how tc works and sets things like PATH. your way wfm :) > > I'm confused why we need --repo[1] and --base-repo > > > > in TcVCS, you have: > > base_repo = c.get('base_repo', c['repo']) > > > > are you expecting c['base_repo'] to be different than c['repo'] if c['base_repo'] exists? > > > > > > [1] http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildb2gbase.py#48 > > Yep, this is for try support, we use mozilla-central as base repo, and pull > latest commits from try. So we have faster cloning because m-c is cached. oic, thanks for clarifying

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=jlund Pull down this commit: hg pull review -r 978d1df59e9855471b201a7908c1d187a7fb946e

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=jlund Pull down this commit: hg pull review -r 749bd24a1208f076a926fe17cd4b55938c818e56

(In reply to Jordan Lund (:jlund) from comment #24) > thanks for the log snippet. based on your last comment, it sounds like you > are going to remove/change a few things: 1) retval conditions 2) output > parser 3) return statement in ensure_repo_and_rev. > > I'll clear the review out of my queue for now. feel free to either interdiff > with a new patch or else submit the whole thing again :) > > (In reply to Wander Lairson Costa [:wcosta] from comment #23) > > https://reviewboard.mozilla.org/r/1553/#review1113 > > > > > That's correct, at least in current situation, do you think would be better > > to add it to config['exes']? > > just asserting that it will always be there as I don't know how tc works and > sets things like PATH. your way wfm :) > > > > I'm confused why we need --repo[1] and --base-repo > > > > > > in TcVCS, you have: > > > base_repo = c.get('base_repo', c['repo']) > > > > > > are you expecting c['base_repo'] to be different than c['repo'] if c['base_repo'] exists? > > > > > > > > > [1] http://mxr.mozilla.org/build/source/mozharness/mozharness/mozilla/building/buildb2gbase.py#48 > > > > Yep, this is for try support, we use mozilla-central as base repo, and pull > > latest commits from try. So we have faster cloning because m-c is cached. > > oic, thanks for clarifying Just pushed the patch again with the requested changes :)

https://reviewboard.mozilla.org/r/1553/#review1175 ::: mozharness/base/vcs/tcvcs.py (Diff revision 7) > + return self.get_output_from_command(cmd, output_parser=parser) parser no longer exists. I think you forgot to remove the reference.

r+ if you remove output_parser=parser and the changes are tested :)

/r/1555 - Bug 1109346: Add taskcluster-vcs support to mozharness. r=jlund Pull down this commit: hg pull review -r 5802e03817a97d5026be6b253d21291b5774b40f

(In reply to Jordan Lund (:jlund) from comment #29) > r+ if you remove output_parser=parser and the changes are tested :) Ops, fixed now, I attached a new build log generated after the changes.

https://reviewboard.mozilla.org/r/1553/#review1201 Ship It!

checking-needed only for mozharness part, gecko part will be pushed to alder (perhaps a should have filed two bugs).

This has broken the unit tests in travis, e.g. see: https://travis-ci.org/mozilla/build-mozharness/builds/46234845 1) ERROR: Failure: ImportError (No module named tcvcs) Traceback (most recent call last): .tox/py27/lib/python2.7/site-packages/nose/loader.py line 414 in loadTestsFromName addr.filename, addr.module) .tox/py27/lib/python2.7/site-packages/nose/importer.py line 47 in importFromPath return self.importFromDir(dir_path, fqname) .tox/py27/lib/python2.7/site-packages/nose/importer.py line 94 in importFromDir mod = load_module(part_fqname, fh, filename, desc) mozharness/mozilla/testing/gaia_test.py line 20 in <module> from mozharness.base.vcs.vcsbase import MercurialScript mozharness/base/vcs/vcsbase.py line 22 in <module> from mozharness.base.vcs.tcvcs import TcVCS ImportError: No module named tcvcs Hopefully this should be reproducible locally with: pip install tox tox Thanks, Pete

/r/1555 - Bug 1109346: Add the tcvcs.py script. Pull down this commit: hg pull review -r 3029a11dec0e413295077e8ccfd3686150a25820

I don't know how, but the tcvcs.py was not included in the commit pushed. Found a minor typo in taskcluster-phone.py file also.

Comment on attachment 8538154 [details] MozReview Request: bz://1109346/wcosta I landed a bustage fix for this: https://hg.mozilla.org/build/mozharness/rev/073d426649ee https://hg.mozilla.org/build/mozharness/rev/3029a11dec0e I didn't do this via reviewboard as I was nervous it might roll back the other files.

This change is affecting my try pushes: http://hg.mozilla.org/build/mozharness/rev/5371983056ce with: > Traceback (most recent call last): > File "scripts/scripts/android_emulator_unittest.py", line 23, in <module> > from mozharness.base.vcs.vcsbase import VCSMixin > File "/builds/slave/test/scripts/mozharness/base/vcs/vcsbase.py", line 22, in <module> > from mozharness.base.vcs.tcvcs import TcVCS > ImportError: No module named tcvcs http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/armenzg@mozilla.com-ecdb1c21ae76/try-android-api-9/try_ubuntu64_vm_mobile_test-mochitest-gl-1-bm116-tests1-linux64-build153.txt.gz I've now re-merge to pick 3029a11dec0e up. Let's see if I hit the issue again or not.

In production: https://hg.mozilla.org/build/mozharness/rev/5371983056ce

In production: https://hg.mozilla.org/build/mozharness/rev/3029a11dec0e

In production: https://hg.mozilla.org/build/mozharness/rev/073d426649ee

https://reviewboard.mozilla.org/r/1555/#review6673 This has already landed in https://bugzilla.mozilla.org/show_bug.cgi?id=1109346#c40, following up with formal review in reviewboard.