Closed Bug 1270317 Opened 4 years ago Closed 4 years ago

Replace HgtoolVCS with raw and modern Mercurial commands

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set

Tracking

(firefox49 fixed)

RESOLVED FIXED
Tracking Status
firefox49 --- fixed

People

(Reporter: gps, Assigned: gps)

References

(Blocks 2 open bugs)

Details

Attachments

(7 files, 4 obsolete files)

58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
58 bytes, text/x-review-board-request
jlund
: review+
Details
Many of the things that hgtool does can now be performed by Mercurial itself.

In this bug, I'll be overhauling the MercurialVCS class to make it conform the the VCSMixin interface and will make it use modern and efficient hg commands without going through an additional abstraction layer. I'll also start removing usage of HgtoolVCS from mozharness, as we'll be able to use a modern MercurialVCS.

If I'm ambitious, I may remove all uses of hgtool from mozharness.
We had a test environment running on Python 2.6 and an ancient version
of Mercurial. AFAICT we run Python 2.7 everywhere, so this environment
can be dropped.

We also upgrade to Mercurial 3.7.3, as that is what automation now runs.

Review commit: https://reviewboard.mozilla.org/r/50633/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50633/
Attachment #8748947 - Flags: review?(jlund)
Attachment #8748948 - Flags: review?(jlund)
Attachment #8748949 - Flags: review?(jlund)
Attachment #8748950 - Flags: review?(jlund)
Attachment #8748950 - Flags: review?(catlee)
Attachment #8748951 - Flags: review?(jlund)
We currently have a "clone_by_revision" property that indicates to
perform a `hg clone -r`. We use it for cloning from Try.

Cloning single revisions undermines the benefits of clone bundles. So,
I'll be replacing "clone_by_revision" with a feature that clones from
another "upstream" repo then does a `hg pull -r` on the wanted revision.

This commit starts that work by introducing a "clone_upstream_url"
property. We define it on Try. It is currently unused.

Review commit: https://reviewboard.mozilla.org/r/50635/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50635/
MercurialVCS doesn't currently implement the VCSMixin interface.

This commit copies the implementation of query_pushinfo() from
HgtoolVCS to MercurialVCS so it implements the interface.

Review commit: https://reviewboard.mozilla.org/r/50637/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50637/
The code for ensure_repo_and_revision() has been completely overhauled.

We now require shared repos using auto pooled storage via the share
extension. This ensures that only a single copy of a logical
repository's history is stored on disk. e.g. if you clone
mozilla-central, inbound, and try, they'll all automatically share the
same storage.

The new code ensures the destination repo is using modern conventions
and will delete the destination repo if it isn't. So once this gets
deployed to production, machines will slowly start using optimal
storage. This should make VCS operations significantly faster.

Another optimization that is now in here is we check for presence of the
wanted revision before doing `hg pull`. This saves some communication
with the server if the revision is already present locally.

This change effectively requires a modern version of Mercurial to be
installed to run ensure_repo_and_revision(). Since Mercurial <3.7.3 has
security vulnerabilities, we shouldn't be running <3.7.3 in automation.
So I think this will be OK. If not, it will certainly be easy to
identify which machines aren't updated!

Review commit: https://reviewboard.mozilla.org/r/50639/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50639/
Now that the MercurialVCS VCS tool does things optimally, we no longer
need to use hgtool!

Again, this will effectively require a modern Mercurial version or
things will fail.

Review commit: https://reviewboard.mozilla.org/r/50641/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50641/
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/1-2/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/1-2/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/1-2/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/1-2/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/1-2/
CC glandium and smacleod in case you want to do a drive-by.
https://reviewboard.mozilla.org/r/50639/#review47395

This commit breaks some tests by introducing the requirement that the VCS share base directory be defined (it isn't defined in some tests, it appears).

Also, there's a Try push at https://treeherder.mozilla.org/#/jobs?repo=try&revision=9f6e42077b55.
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/2-3/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/2-3/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/2-3/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/2-3/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/2-3/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/3-4/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/3-4/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/4-5/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/4-5/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/5-6/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/5-6/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/6-7/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/6-7/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/7-8/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/7-8/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/8-9/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/8-9/
Attachment #8748950 - Flags: review?(catlee)
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

https://reviewboard.mozilla.org/r/50639/#review47501

::: testing/mozharness/mozharness/base/vcs/mercurial.py:10
(Diff revision 9)
>  # You can obtain one at http://mozilla.org/MPL/2.0/.
>  # ***** END LICENSE BLOCK *****
>  """Mercurial VCS support.
>  
>  Largely copied/ported from
>  https://hg.mozilla.org/build/tools/file/cf265ea8fb5e/lib/python/util/hg.py .

this probably isn't true any more

::: testing/mozharness/mozharness/base/vcs/mercurial.py:336
(Diff revision 9)
> -            # Need better fallback
> -            self.error("Error updating %s from shared_repo (%s): " % (dest, shared_repo))
> -            self.exception(level='error')
> -            self.rmtree(dest)
> -
>      def share(self, source, dest, branch=None, revision=None):

is this method used any more?

::: testing/mozharness/mozharness/base/vcs/mercurial.py:423
(Diff revision 9)
> +        # well-formed shared storage or we have nothing.
> +        created = False
> +
> +        # Create the destination if necessary.
> +        if not os.path.exists(dest):
> +            # We do a full `hg clone` (no `hg clone -r`) because it will use

is there a way to guarantee this happens? if we try and clone try without a bundle, things will fail.

::: testing/mozharness/mozharness/base/vcs/mercurial.py:424
(Diff revision 9)
> +        created = False
> +
> +        # Create the destination if necessary.
> +        if not os.path.exists(dest):
> +            # We do a full `hg clone` (no `hg clone -r`) because it will use
> +            # clone bundles, which are the most effecient way to transfer data.

s/effecient/efficient/

::: testing/mozharness/mozharness/base/vcs/mercurial.py:460
(Diff revision 9)
> +                have_wanted_rev = wanted_rev in output
> +
> +        if not have_wanted_rev:
> +            self.info('pulling to obtain %s' % wanted_rev)
> +            args = self.hg + ['pull', '-r', wanted_rev, repo_url]
> +            # TODO we may want to wrap this in a retry().

We should leave it up to the caller to retry this operation.

::: testing/mozharness/mozharness/base/vcs/mercurial.py:469
(Diff revision 9)
> +        # Now we should have the wanted revision in the store. Perform
> +        # working directory manipulation.
> +
> +        # Do a purge first, if requested. We purge first because this way we're
> +        # guaranteed to not have conflicts on `hg update`.
> +        if purge and not created:

I think mozharness will need to be taught how to purge properly on windows. See https://bugzilla.mozilla.org/show_bug.cgi?id=1157704 for how we did it in hgtool
https://reviewboard.mozilla.org/r/50639/#review47501

> is this method used any more?

Nope. I'll nuke it in the next submission.

> is there a way to guarantee this happens? if we try and clone try without a bundle, things will fail.

I'm fine with failing fast in this scenario, as it means there is a configuration problem.

> I think mozharness will need to be taught how to purge properly on windows. See https://bugzilla.mozilla.org/show_bug.cgi?id=1157704 for how we did it in hgtool

I'll add the purgelong extension to mozharness.
The build/tools repo has a "purgelong" extension that is used to delete
long filenames on Windows. Without this extension, the APIs Mercurial
uses may run into path length issues and `hg purge` will fail.

This commit is a straight import of the purgelong extension
from https://hg.mozilla.org/build/tools minus the shebang line, which
isn't needed.

Review commit: https://reviewboard.mozilla.org/r/50851/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50851/
Attachment #8749323 - Flags: review?(jlund)
Attachment #8749324 - Flags: review?(jlund)
Attachment #8749325 - Flags: review?(jlund)
Attachment #8748950 - Flags: review?(catlee)
We no longer use <3.7 in automation. So drop support for <3.2. While I
was here, I also added magic variables to the extension so Mercurial
can react intelligently to version compatibility issues.

Review commit: https://reviewboard.mozilla.org/r/50855/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50855/
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/3-4/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/3-4/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/3-4/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/9-10/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/9-10/
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/4-5/
Comment on attachment 8749323 [details]
MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50851/diff/1-2/
Comment on attachment 8749324 [details]
MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50853/diff/1-2/
Comment on attachment 8749325 [details]
MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50855/diff/1-2/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/4-5/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/4-5/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/10-11/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/10-11/
https://reviewboard.mozilla.org/r/50631/#review47587

so much cleaner! I'm not up to date with current hg versions across all our infra slaves. If we are up to date with the version expected/defined in requirements.txt, ++ to this patch. tyvm for this

::: testing/mozharness/configs/builds/branch_specifics.py:187
(Diff revision 10)
>      },
>      'try': {
>          'repo_path': 'try',
>          'clone_by_revision': True,
>          'clone_with_purge': True,
> +        # FUTURE this should be a unified repo because Try pushes may e.g.for

looks like typo: `may e.g.for be`

::: testing/mozharness/mozharness/base/vcs/mercurial.py:364
(Diff revision 10)
> +        share_base = c.get('vcs_share_base', os.environ.get('HG_SHARE_BASE_DIR', None))
> +
> +        # We require shared storage is configured because it guarantees we
> +        # only have 1 local copy of logical repo stores.
> +        if not share_base:
> +            self.fatal('vcs share base not defined; refusing to operate sub-optimally',

hmm, I wonder how this will affect Taskcluster tasks and developer local runs..

::: testing/mozharness/mozharness/base/vcs/mercurial.py:397
(Diff revision 10)
> -            if not c.get('allow_unshared_local_clones'):
> -                # Re-raise the exception so it gets caught below.
> -                # We'll then clobber dest, and clone from original
> -                # repo
> +
> +            # FUTURE when we require generaldelta, this is where we can check
> +            # for that.
> +        except IOError as e:
> +            if e.errno != errno.ENOENT:
>                  raise

wondering if we could hit a situation where dest_hg_sharedpath raises ENOENT and we silently continue without running self.rmtree(dest)

... might be wrong or could be overkill

::: testing/mozharness/mozharness/base/vcs/mercurial.py:408
(Diff revision 10)
> -        # OS supports it
> -        try:
> -            self.clone(shared_repo, dest, update_dest=False)
> -            return self.update(dest, branch=branch, revision=revision)
> -        except VCSException:
> -            # Need better fallback
> +        # Create the destination if necessary.
> +        if not os.path.exists(dest):
> +            # We do a full `hg clone` (no `hg clone -r`) because it will use
> +            # clone bundles, which are the most efficient way to transfer data.
> +            # `hg clone` with pooled storage automatically performs a
> +            # share under the hood. So no additional share magic is necessary.

\o/

::: testing/mozharness/mozharness/base/vcs/mercurial.py:409
(Diff revision 10)
> -        try:
> -            self.clone(shared_repo, dest, update_dest=False)
> -            return self.update(dest, branch=branch, revision=revision)
> -        except VCSException:
> -            # Need better fallback
> -            self.error("Error updating %s from shared_repo (%s): " % (dest, shared_repo))
> +        if not os.path.exists(dest):
> +            # We do a full `hg clone` (no `hg clone -r`) because it will use
> +            # clone bundles, which are the most efficient way to transfer data.
> +            # `hg clone` with pooled storage automatically performs a
> +            # share under the hood. So no additional share magic is necessary.
> +            clone_url = upstream if upstream else repo_url

not even a nit: I tend to `a = b or c` rather than `a = b if b else c`. ooc, why do you prefer the latter? readibility?

::: testing/mozharness/mozharness/base/vcs/purgelong.py:2
(Diff revision 10)
> +'''
> +Mercurial extension to enable purging long filenames on Windows

being independant of mozharness, I feel like this file should be in http://hg.mozilla.org/mozilla-central/file/e5a10bc7dac4/testing/mozharness/external_tools
https://reviewboard.mozilla.org/r/50631/#review47587

> hmm, I wonder how this will affect Taskcluster tasks and developer local runs..

TC uses its own version control management (tc-vcs). Many (all?) of the TC tasks skip the checkout-sources mozharness step. TC is a separate beast (which I may tackle after this lands).

Good point regarding developer local runs. I'll have to think about this a bit more...
Comment on attachment 8749323 [details]
MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50851/diff/2-3/
Comment on attachment 8749324 [details]
MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50853/diff/2-3/
Comment on attachment 8749325 [details]
MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50855/diff/2-3/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/5-6/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/5-6/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/11-12/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/11-12/
hgtool printed the hg version info when running. This is useful data
when debugging Mercurial failures. Add it back in.

We also add `hg debuginstall`, which prints useful bits about the
install, including the Python path and version.

Review commit: https://reviewboard.mozilla.org/r/50971/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50971/
Attachment #8749466 - Flags: review?(jlund)
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

https://reviewboard.mozilla.org/r/50633/#review47787
Attachment #8748947 - Flags: review?(jlund) → review+
Comment on attachment 8749323 [details]
MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r=jlund

https://reviewboard.mozilla.org/r/50851/#review47789
Attachment #8749323 - Flags: review?(jlund) → review+
Attachment #8749324 - Flags: review?(jlund) → review+
Comment on attachment 8749324 [details]
MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r=jlund

https://reviewboard.mozilla.org/r/50853/#review47793
Attachment #8749325 - Flags: review?(jlund) → review+
Comment on attachment 8749325 [details]
MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r=jlund

https://reviewboard.mozilla.org/r/50855/#review47795

looks like we probably still use purgelong from its original location (tools repo) https://dxr.mozilla.org/build-central/source/tools/lib/python/util/hg.py#180

I suppose we should apply the same patch in the tools repo?
Attachment #8748948 - Flags: review?(jlund) → review+
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

https://reviewboard.mozilla.org/r/50635/#review47797
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

https://reviewboard.mozilla.org/r/50637/#review47803

it would be great to kill hgtool.py (from base/vcs/ and external_tools/) and replace usage soley with mercurial.py :)
Attachment #8748949 - Flags: review?(jlund) → review+
https://reviewboard.mozilla.org/r/50637/#review47803

That is my eventual goal. I'm inclined to defer to a follow-up bug: there is a long tail of mozharness scripts and configs still using hgtool and I want to get the big fish (Firefox builds) out of the way first.
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

https://reviewboard.mozilla.org/r/50639/#review47805

this looks good to me. one small nit

::: testing/mozharness/mozharness/base/vcs/mercurial.py:323
(Diff revisions 10 - 12)
>  
>      @property
>      def purgelong_path(self):
>          """Path to the purgelong extension."""
>          here = os.path.abspath(os.path.dirname(__file__))
> -        ext = os.path.join(here, 'purgelong.py')
> +        ext = os.path.join(here, '..', '..', '..', 'external_tools',

I prefer this abspath style of finding external_tools to .. rel path.

https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/mozilla/purge.py#14
Attachment #8748950 - Flags: review?(jlund)
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

https://reviewboard.mozilla.org/r/50641/#review47809

\o/
Attachment #8748951 - Flags: review?(jlund) → review+
Attachment #8749466 - Flags: review?(jlund) → review+
Comment on attachment 8749466 [details]
MozReview Request: Bug 1270317 - Record hg version and install info; r=jlund

https://reviewboard.mozilla.org/r/50971/#review47811
https://reviewboard.mozilla.org/r/50855/#review47795

If I have my way, we'll soon kill hgtool, which is the only place purgelong is used. So I'm inclined to not do this.
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/5-6/
Attachment #8748947 - Attachment description: MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r?jlund → MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund
Attachment #8749323 - Attachment description: MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r?jlund → MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r=jlund
Attachment #8749324 - Attachment description: MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r?jlund → MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r=jlund
Attachment #8749325 - Attachment description: MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r?jlund → MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r=jlund
Attachment #8748948 - Attachment description: MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r?jlund → MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund
Attachment #8748949 - Attachment description: MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r?jlund → MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund
Attachment #8748951 - Attachment description: MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r?jlund → MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund
Attachment #8749466 - Attachment description: MozReview Request: Bug 1270317 - Record hg version and install info; r?jlund → MozReview Request: Bug 1270317 - Record hg version and install info; r=jlund
Attachment #8748950 - Flags: review?(jlund)
Comment on attachment 8749323 [details]
MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50851/diff/3-4/
Comment on attachment 8749324 [details]
MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50853/diff/3-4/
Comment on attachment 8749325 [details]
MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50855/diff/3-4/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/6-7/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/6-7/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/12-13/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/12-13/
Comment on attachment 8749466 [details]
MozReview Request: Bug 1270317 - Record hg version and install info; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50971/diff/1-2/
Attachment #8748950 - Flags: review?(jlund) → review+
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

https://reviewboard.mozilla.org/r/50639/#review47817
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/6-7/
Attachment #8748950 - Attachment description: MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r?jlund, catlee → MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund
Attachment #8748950 - Flags: review?(catlee)
Comment on attachment 8749323 [details]
MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50851/diff/4-5/
Comment on attachment 8749324 [details]
MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50853/diff/4-5/
Comment on attachment 8749325 [details]
MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50855/diff/4-5/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/7-8/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/7-8/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/13-14/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/13-14/
Comment on attachment 8749466 [details]
MozReview Request: Bug 1270317 - Record hg version and install info; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50971/diff/2-3/
Blocks: 1232442
Blocks: 1270951
Blocks: 1271137
seems some builders were not happy about this, after landing and periodic clobber this broke it seems nightly builds like https://treeherder.mozilla.org/logviewer.html#?job_id=3847233&repo=mozilla-central

with errors like:

Copy/paste: hg --config ui.merge=internal:merge --config extensions.share= --config share.pool=C:/builds/hg-shared clone -U https://hg.mozilla.org/build/tools c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\tools

 03:07:15     INFO -  (sharing from new pooled repository 7ae7fb134bf7aec6ec96a062ff47a69053dd2973)
 03:07:15     INFO -  applying clone bundle from https://s3-external-1.amazonaws.com/moz-hg-bundles-us-east-1/build/tools/b030af490d9395958c3dc17443204037faf6a991.packed1.hg
 03:07:15     INFO -  2005 files to transfer, 26.7 MB of data
 03:07:15     INFO -  transferred 26.7 MB in 5.5 seconds (4.88 MB/sec)
 03:07:15     INFO -  finished applying clone bundle
 03:07:15     INFO -  searching for changes
 03:07:15     INFO -  no changes found
 03:07:15     INFO -  Traceback (most recent call last):
 03:07:15     INFO -    File "mercurial\dispatch.pyc", line 191, in _runcatch
 03:07:15     INFO -    File "mercurial\dispatch.pyc", line 924, in _dispatch
 03:07:15     INFO -    File "mercurial\dispatch.pyc", line 681, in runcommand
 03:07:15     INFO -    File "mercurial\extensions.pyc", line 195, in closure
 03:07:15     INFO -    File "hgext\color.pyc", line 518, in colorcmd
 03:07:15     INFO -    File "mercurial\dispatch.pyc", line 1055, in _runcommand
 03:07:15     INFO -    File "mercurial\dispatch.pyc", line 1015, in checkargs
 03:07:15     INFO -    File "mercurial\dispatch.pyc", line 921, in <lambda>
 03:07:15     INFO -    File "mercurial\util.pyc", line 991, in check
 03:07:15     INFO -    File "mercurial\extensions.pyc", line 195, in closure
 03:07:15     INFO -    File "mercurial\util.pyc", line 991, in check
 03:07:15     INFO -    File "hgext\share.pyc", line 122, in clone
 03:07:15     INFO -    File "mercurial\util.pyc", line 991, in check
 03:07:15     INFO -    File "mercurial\commands.pyc", line 1563, in clone
 03:07:15     INFO -    File "mercurial\hg.pyc", line 489, in clone
 03:07:15     INFO -    File "mercurial\hg.pyc", line 372, in clonewithshare
 03:07:15     INFO -    File "mercurial\hg.pyc", line 223, in share
 03:07:15     INFO -    File "mercurial\scmutil.pyc", line 364, in mkdir
 03:07:15     INFO -  WindowsError: [Error 3] The system cannot find the path specified: 'c:\\builds\\moz2_slave\\m-cen-w32-ntly-000000000000000\\build\\tools'
 03:07:15     INFO -  abort: The system cannot find the path specified: 'c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build\tools'
03:07:15 ERROR - Return code: 255 

so backedout this out to hopefully unbreak the builds :)
Status: RESOLVED → REOPENED
Flags: needinfo?(gps)
Resolution: FIXED → ---
Looks like clobberer removes the c:\builds\moz2_slave\m-cen-w32-ntly-000000000000000\build parent directory. We just need to ensure the parent directory exists before doing the clone. Should be a trivial fix-up.
Flags: needinfo?(gps)
Status: REOPENED → ASSIGNED
Duplicate of this bug: 1271137
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/7-8/
Comment on attachment 8749323 [details]
MozReview Request: Bug 1270317 - Add the purgelong Mercurial extension; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50851/diff/5-6/
Comment on attachment 8749324 [details]
MozReview Request: Bug 1270317 - Use modern exception syntax in purgelong; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50853/diff/5-6/
Comment on attachment 8749325 [details]
MozReview Request: Bug 1270317 - Remove purgelong compatibility with ancient Mercurial versions; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50855/diff/5-6/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/8-9/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/8-9/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50639/diff/14-15/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/14-15/
Comment on attachment 8749466 [details]
MozReview Request: Bug 1270317 - Record hg version and install info; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50971/diff/3-4/
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

Please look at the interdiff of just this one to see the fixes for missing parent directory and abandoned transaction issues.
Attachment #8748950 - Flags: review+ → review?(jlund)
Comment on attachment 8748950 [details]
MozReview Request: Bug 1270317 - Use modern, optimal cloning in MercurialVCS.ensure_repo_and_revision(); r=jlund

https://reviewboard.mozilla.org/r/50639/#review48541

looks good. couple questions below

::: testing/mozharness/mozharness/base/vcs/mercurial.py:40
(Diff revisions 13 - 15)
>  # build/tools/scripts to MercurialVCS -- generic tagging logic belongs here.
>  REVISION, BRANCH = 0, 1
>  
>  
> -class RepositoryUnrelatedParser(OutputParser):
> -    """Parse `hg pull` output for "repository unrelated" errors."""
> +class MercurialErrorParser(OutputParser):
> +    """Parse cloen and pull output for various errors."""

s/cloen/clone

::: testing/mozharness/mozharness/base/vcs/mercurial.py:399
(Diff revisions 13 - 15)
>                  self.rmtree(dest)
>  
>              # FUTURE when we require generaldelta, this is where we can check
>              # for that.
>  
> +        def recover_from_abandoned_transaction(parser):

wondering if this should be defined outside of ensure_repo_and_revision impl

::: testing/mozharness/mozharness/base/vcs/mercurial.py:405
(Diff revisions 13 - 15)
> +            # We can run `hg recover` in the destination repo because it is
> +            # shared and `hg recover` always operates on the shared store.
> +            if not self.run_command(self.hg + ['recover'], cwd=dest):
> +                return
> +
> +            with open(os.path.join(dest, '.hg', 'sharedpath'), 'rb') as fh:

now that we are opening and reading from this twice, maybe we should have a helper method for this. Or better yet, use this one each time we read sharedpath:

self.read_from_file()

https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/script.py#773

::: testing/mozharness/mozharness/base/vcs/mercurial.py:421
(Diff revisions 13 - 15)
>  
>          # Create the destination if necessary.
>          if not os.path.exists(dest):
> +            # Ensure parent directory exists.
> +            try:
> +                os.makedirs(os.path.dirname(dest))

fyi for next time, you should be able to safely call: self.mkdir_p(os.path.dirname(dest))

https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/script.py#146

::: testing/mozharness/test/test_base_vcs_mercurial.py:375
(Diff revisions 13 - 15)
> +            'branch': 'default',
> +            'vcs_share_base': os.path.join(self.tmpdir, 'share'),
> +        }
> +        m.ensure_repo_and_revision()
> +
> +        with open(os.path.join(self.wc, '.hg', 'sharedpath'), 'rb') as fh:

in addition to whereever we read sharedpath in the mercurial.py script, we can use m.read_from_file() anywhere in these tests too

::: testing/mozharness/test/test_base_vcs_mercurial.py:386
(Diff revisions 13 - 15)
> +
> +        # Touching the journal file simulates an abondoned transaction.
> +        with open(os.path.join(shared_path, 'store', 'journal'), 'a'):
> +            pass
> +
> +        m.vcs_config['dest'] = os.path.join(self.tmpdir, 'dest2')

sorry, could you explain what's happening here and how this test works?
Attachment #8748950 - Flags: review?(jlund)
Depends on: 1273305
Functionality for doing an optimal clone/pull+share+purge+update is now
implemented in the robustcheckout extension so it can be implemented in one
place and used by all the various tools needing to perform a "robust"
checkout using optimal practices.

This commit switches the MercurialVCS to use it.

Functionality for interfacing with shared repos and associated tests have
been removed because this is all implemented and tested in robustcheckout.

Various other tests have also been removed because they are redundant with
tests in the robustcheckout extension.

Review commit: https://reviewboard.mozilla.org/r/53254/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53254/
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/8-9/
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/9-10/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/9-10/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/15-16/
Comment on attachment 8749466 [details]
MozReview Request: Bug 1270317 - Record hg version and install info; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50971/diff/4-5/
Attachment #8749323 - Attachment is obsolete: true
Attachment #8749324 - Attachment is obsolete: true
Attachment #8749325 - Attachment is obsolete: true
Attachment #8748950 - Attachment is obsolete: true
https://reviewboard.mozilla.org/r/53254/#review50048

New plan here.

Instead of implementing the "do a checkout" logic in multiple places (namely mozharness, hgtool, and [eventually] tc-vcs), I've created a Mercurial extension that performs a clone+share+checkout using best practices. This is a standalone file that provides a command that we can call into. This makes the code in mozharness and hgtool simpler and mostly minimal.
Comment on attachment 8748947 [details]
MozReview Request: Bug 1270317 - Upgrade to Mercurial 3.7.3 in the mozharness test environment; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50633/diff/9-10/
Attachment #8753414 - Attachment description: MozReview Request: Bug 1270317 - Add robustcheckout.py Mercurial extension → MozReview Request: Bug 1270317 - Add robustcheckout.py Mercurial extension; r?jlund
Attachment #8753414 - Flags: review?(jlund)
Comment on attachment 8748948 [details]
MozReview Request: Bug 1270317 - Define a clone_upstream_url property; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50635/diff/10-11/
Comment on attachment 8748949 [details]
MozReview Request: Bug 1270317 - Add query_pushinfo to MercurialVCS; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50637/diff/10-11/
Comment on attachment 8753414 [details]
MozReview Request: Bug 1270317 - Add robustcheckout.py Mercurial extension; r?jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53252/diff/1-2/
Comment on attachment 8753415 [details]
MozReview Request: Bug 1270317 - Use robustcheckout extension for checking out Mercurial repos; r?jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/53254/diff/1-2/
Comment on attachment 8748951 [details]
MozReview Request: Bug 1270317 - Stop using hgtool for Firefox builds; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50641/diff/16-17/
Comment on attachment 8749466 [details]
MozReview Request: Bug 1270317 - Record hg version and install info; r=jlund

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50971/diff/5-6/
ran out of time to review this today. will look tomorrow
Comment on attachment 8753414 [details]
MozReview Request: Bug 1270317 - Add robustcheckout.py Mercurial extension; r?jlund

https://reviewboard.mozilla.org/r/53252/#review50430

stamping purely as support to adding this extension, not a review of the extention itself as it has been reviewed elsewhere.
Attachment #8753414 - Flags: review?(jlund) → review+
Comment on attachment 8753415 [details]
MozReview Request: Bug 1270317 - Use robustcheckout extension for checking out Mercurial repos; r?jlund

https://reviewboard.mozilla.org/r/53254/#review50426

before I stamp:

iiuc,

in mozharness: we use and enforce the use of robustcheckout everywhere that currently calls MercurialVCS.ensure_repo_and_revision()?

in tools repo: we use but not enforce robustcheckout from anywhere that calls lib/python/hg.py's mecurial() which include hgtool.py release-runner, l10n.py, etc. we don't enforce because of the variety of ways each of these call mercurial()?

is that correct?

this leaves me wondering what this means for mozharness's copy of tools' hgtool.py and mozharness's own version of hgtool, HgtoolVCS
  * https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/vcs/hgtool.py
      * used:
          * https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/mozharness/base/vcs/vcsbase.py#27
          * https://dxr.mozilla.org/mozilla-central/search?q=path%3Amozharnes+path%3Aconfigs+hgtool&redirect=true&case=false
  * https://dxr.mozilla.org/mozilla-central/source/testing/mozharness/external_tools/hgtool.py
      * used:
          *  https://dxr.mozilla.org/mozilla-central/search?q=path%3Amozharnes+external_tools+hgtool.py&redirect=false&case=false

we have too many hg "tools" :(

::: testing/mozharness/mozharness/base/vcs/mercurial.py:347
(Diff revision 2)
> +        # The API here is kind of bad because we're relying on state in
> +        # self.vcs_config instead of passing arguments. This confuses
> +        # scripts that have multiple repos. This includes the clone_tools()
> +        # step :(

hm, can't each repo have its own vcs_config defined in self.config["repos"] so that we can isolate prefs in vcs_checkout_repos()[1] or even add custom configs (kwargs) to vcs_checkout() call directly?

Or are you saying that scripts are not using that correctly?

[1] https://dxr.mozilla.org/mozilla-central/search?q=vcs_checkout_repos&=mozilla-central&redirect=true

::: testing/mozharness/mozharness/base/vcs/mercurial.py:362
(Diff revision 2)
> +
> +        # We require shared storage is configured because it guarantees we
> +        # only have 1 local copy of logical repo stores.
> +        if not share_base:
> +            raise VCSException('vcs share base not defined; '
> +                               'refusing to operate sub-optimally')

lolz. I suppose its not too bad to enforce this even for devs

::: testing/mozharness/mozharness/base/vcs/mercurial.py:384
(Diff revision 2)
> -            share_base = None
>  
> -        if share_base:
> -            return self._ensure_shared_repo_and_revision(share_base)
> +        parser = RepositoryUpdateRevisionParser(config=self.config,
> +                                                log_obj=self.log_obj)
> +        if self.run_command(args, output_parser=parser):
> +            raise VCSException('repo checkout failed!')

++ to using extension/external_tool to control the complexity
Attachment #8753415 - Flags: review?(jlund)
https://reviewboard.mozilla.org/r/53254/#review50426

That assessment is correct.

I wasn't willing to change all consumers to use robustcheckout yet. The main reason is I'm a bit scared of random processes breaking. I'd rather take baby steps.

I'd like to eventually switch everything in mozharness to use MercurialVCS and get rid of HgtoolVCS. I'd also like all consumers of hgtool (if there are any remaining) to use robustcheckout. These can be follow-up bugs.

I also plan to land the hgtool changes a day or two after these mozharness changes. I feel like hgtool changes are higher risk and want the dust to settle before I land them.

> hm, can't each repo have its own vcs_config defined in self.config["repos"] so that we can isolate prefs in vcs_checkout_repos()[1] or even add custom configs (kwargs) to vcs_checkout() call directly?
> 
> Or are you saying that scripts are not using that correctly?
> 
> [1] https://dxr.mozilla.org/mozilla-central/search?q=vcs_checkout_repos&=mozilla-central&redirect=true

I don't think scripts are doing that properly. I'm basically cargo culting behavior from before the patch. I put the comment in because I figured this seemingly incorrect behavior should be documented. I'm inclined to address things as a follow-up.

> lolz. I suppose its not too bad to enforce this even for devs

We had landed this before and nobody complained. I'm inclined to land it again and see if anyone notices. I /think/ most local users of mozharness already have checkouts so they never encounter this. I could be wrong. I'm inclined to cross the bridge if we come to it.
Comment on attachment 8753415 [details]
MozReview Request: Bug 1270317 - Use robustcheckout extension for checking out Mercurial repos; r?jlund

https://reviewboard.mozilla.org/r/53254/#review50450
Attachment #8753415 - Flags: review+
https://reviewboard.mozilla.org/r/53254/#review50426

okay, this works for me too. getting follow up bugs on file and making comments under HgtoolVCS and external_tools/hgtool.py of their deprecation. :D

I'm excited for this patch's clean up and the thought of dealing with all edge cases in the future :)

> I don't think scripts are doing that properly. I'm basically cargo culting behavior from before the patch. I put the comment in because I figured this seemingly incorrect behavior should be documented. I'm inclined to address things as a follow-up.

okay, would you mind getting a bug on file to ensure proper vcs_config kwargs being passed everywhere and add the bug number in this comment?

> We had landed this before and nobody complained. I'm inclined to land it again and see if anyone notices. I /think/ most local users of mozharness already have checkouts so they never encounter this. I could be wrong. I'm inclined to cross the bridge if we come to it.

cool. wfm
Blocks: 1274059
Blocks: 1274092
https://reviewboard.mozilla.org/r/53254/#review50426

> okay, would you mind getting a bug on file to ensure proper vcs_config kwargs being passed everywhere and add the bug number in this comment?

Bug 1274059 filed to track removing usage of hgtool from mozharness.
Blocks: 1274436
No longer depends on: 1274436
Blocks: 1274488
Blocks: 1274655
Taskcluster docker images install mercurial from http://mockbuild-repos.pub.build.mozilla.org/releng/public/CentOS/6/x86_64/, is it possible to upgrade it? Or at least push a package with the newer version?
Flags: needinfo?(jlund)
Depends on: 1274765
(In reply to Wander Lairson Costa [:wcosta] from comment #130)
> Taskcluster docker images install mercurial from
> http://mockbuild-repos.pub.build.mozilla.org/releng/public/CentOS/6/x86_64/,
> is it possible to upgrade it? Or at least push a package with the newer
> version?

wcosta switched to installing hg via pip.
Flags: needinfo?(jlund)
Blocks: 1275777
Duplicate of this bug: 1097979
Blocks: 1275783
Depends on: 1277041
Blocks: 1277046
Blocks: 1279048
Blocks: 1286336
You need to log in before you can comment on or make changes to this bug.