Closed Bug 1393242 Opened 7 years ago Closed 7 years ago

Misc mozversioncontrol improvements

Tracking

(firefox57 fixed)

Status:

RESOLVED FIXED

Milestone:

mozilla57

Tracking Flags:

Tracking

Status

firefox57

---

fixed

People

(Reporter: gps, Assigned: gps)

Details

Attachments

(7 files)

Bug 1393242 - Helper function to obtain a repository from a build config; 7 years ago Gregory Szorc [:gps] 59 bytes, text/x-review-board-request	mshal : review+	Details
Bug 1393242 - Return a Repository using configure preferences; 7 years ago Gregory Szorc [:gps] 59 bytes, text/x-review-board-request	mshal : review+	Details
Bug 1393242 - Vendor python-hglib 2.4; 7 years ago Gregory Szorc [:gps] 59 bytes, text/x-review-board-request	mshal : review+	Details
Bug 1393242 - Hook up hglib to HgRepository; 7 years ago Gregory Szorc [:gps] 59 bytes, text/x-review-board-request	mshal : review+	Details
Bug 1393242 - Use hglib for get_files_in_working_directory(); 7 years ago Gregory Szorc [:gps] 59 bytes, text/x-review-board-request	mshal : review+	Details
Bug 1393242 - Make hglib tests more resilient; 7 years ago Gregory Szorc [:gps] 59 bytes, text/x-review-board-request	mshal : review+	Details
Bug 1393242 - Normalize paths in MercurialRevisionFinder; 7 years ago Gregory Szorc [:gps] 59 bytes, text/x-review-board-request	mshal : review+	Details

Gregory Szorc [:gps]

Assignee

Description

•

7 years ago

Patches forthcoming.

Comment hidden (mozreview-request)

Some callers already have a build config object. Let's not
make them call a function that imports buildconfig.

Review commit: https://reviewboard.mozilla.org/r/171906/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171906/

Comment hidden (mozreview-request)

If configure has defined VCS binaries, we should use those.

Review commit: https://reviewboard.mozilla.org/r/171908/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171908/

Comment hidden (mozreview-request)

python-hglib is a Python client for Mercurial's command server. It
facilitates querying Mercurial efficiently (using a single process)
and without having to parse output in the common case.

Let's vendor it so we can make use of it for more advanced Mercurial
scenarios.

Content vendored from changeset 820d7c1e470a without modifications
(other than deleting unwanted files).

As part of vendoring, we add the package to the virtualenv and make
it available to mach.

Review commit: https://reviewboard.mozilla.org/r/171910/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171910/

Comment hidden (mozreview-request)

Because hglib spawns a persistent process, we introduce a context
manager for Repository. It no-ops by default. On HgRepository it
controls the lifetime of the persistent hg process.

A helper method for running an hg command via hglib has been added.
We can't transition existing methods to hglib because hglib
requires a context manager, which no consumer is using yet.

Review commit: https://reviewboard.mozilla.org/r/171912/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171912/

Comment hidden (mozreview-request)

And convert consumers to context managers because hglib requires that.

Review commit: https://reviewboard.mozilla.org/r/171914/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171914/

Michael Shal [:mshal]

Comment 6

•

7 years ago

mozreview-review

Comment on attachment 8900524 [details]
Bug 1393242 - Helper function to obtain a repository from a build config;

https://reviewboard.mozilla.org/r/171906/#review177584

Attachment #8900524 - Flags: review?(mshal) → review+

Michael Shal [:mshal]

Comment 7

•

7 years ago

mozreview-review

Comment on attachment 8900525 [details]
Bug 1393242 - Return a Repository using configure preferences;

https://reviewboard.mozilla.org/r/171908/#review177586

Attachment #8900525 - Flags: review?(mshal) → review+

Michael Shal [:mshal]

Comment 8

•

7 years ago

mozreview-review

Comment on attachment 8900526 [details]
Bug 1393242 - Vendor python-hglib 2.4;

https://reviewboard.mozilla.org/r/171910/#review177588

Attachment #8900526 - Flags: review?(mshal) → review+

Michael Shal [:mshal]

Comment 9

•

7 years ago

mozreview-review

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

https://reviewboard.mozilla.org/r/171912/#review177592

::: python/mozversioncontrol/mozversioncontrol/__init__.py:129
(Diff revision 1)
> +        import hglib.client
> +
>          super(HgRepository, self).__init__(path, tool=hg)
>          self._env[b'HGPLAIN'] = b'1'
>  
> +        # Setting this is a global change. But since the tool was validated

What do you mean it's a "global change"? Would another process outside of m-c that uses hglib talk to the same server and be impacted by this setting?

Attachment #8900527 - Flags: review?(mshal) → review+

Michael Shal [:mshal]

Comment 10

•

7 years ago

mozreview-review

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

https://reviewboard.mozilla.org/r/171914/#review177598

Attachment #8900528 - Flags: review?(mshal) → review+

Gregory Szorc [:gps]

Assignee

Comment 11

•

7 years ago

mozreview-review-reply

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

https://reviewboard.mozilla.org/r/171912/#review177592

> What do you mean it's a "global change"? Would another process outside of m-c that uses hglib talk to the same server and be impacted by this setting?

Modified a global variable for the Python process. I'll update the comment.

Comment hidden (mozreview-request)

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171912/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171914/diff/1-2/

Pulsebot

Comment 14

•

7 years ago

Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/0279960d71ab
Helper function to obtain a repository from a build config; r=mshal
https://hg.mozilla.org/integration/autoland/rev/217260fb6976
Return a Repository using configure preferences; r=mshal
https://hg.mozilla.org/integration/autoland/rev/159028ef12d2
Vendor python-hglib 2.4; r=mshal
https://hg.mozilla.org/integration/autoland/rev/ffe8bef74be0
Hook up hglib to HgRepository; r=mshal
https://hg.mozilla.org/integration/autoland/rev/18d89d8ab860
Use hglib for get_files_in_working_directory(); r=mshal

Pulsebot

Comment 15

•

7 years ago

Backout by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/299de9e2cdab
Backed out changesets 18d89d8ab860, ffe8bef74be0, 159028ef12d2, 217260fb6976, 0279960d71ab

Comment hidden (mozreview-request)

These tests weren't running in automation because hglib wasn't
available. An upcoming commit will vendor hglib. This exposed that the
tests can fail if ui.username isn't set.

In this commit, we introduce a helper function to obtain an hglib
client with ui.username set. We also convert tests to use the context
manager form of the client so resources are cleaned up immediately
without relying on refcounting or garbage collection.

Review commit: https://reviewboard.mozilla.org/r/174452/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/174452/

Comment hidden (mozreview-request)

Comment on attachment 8900524 [details]
Bug 1393242 - Helper function to obtain a repository from a build config;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171906/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8900525 [details]
Bug 1393242 - Return a Repository using configure preferences;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171908/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8900526 [details]
Bug 1393242 - Vendor python-hglib 2.4;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171910/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171912/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171914/diff/2-3/

Michael Shal [:mshal]

Comment 22

•

7 years ago

mozreview-review

Comment on attachment 8902788 [details]
Bug 1393242 - Make hglib tests more resilient;

https://reviewboard.mozilla.org/r/174452/#review179982

Looks reasonable to me, but try still seems unhappy on Windows. (And Linux has some check_spidermonkey_style errors, which I can't tell if they're related)

Attachment #8902788 - Flags: review?(mshal)

Comment hidden (mozreview-request)

This ensures we use forward slashes, even if Mercurial emits
backslashes (which it can do on Windows).

Review commit: https://reviewboard.mozilla.org/r/175034/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/175034/

Comment hidden (mozreview-request)

Comment on attachment 8900524 [details]
Bug 1393242 - Helper function to obtain a repository from a build config;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171906/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8900525 [details]
Bug 1393242 - Return a Repository using configure preferences;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171908/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8902788 [details]
Bug 1393242 - Make hglib tests more resilient;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174452/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8900526 [details]
Bug 1393242 - Vendor python-hglib 2.4;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171910/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171912/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171914/diff/3-4/

Gregory Szorc [:gps]

Assignee

Comment 30

•

7 years ago

mozreview-review

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

https://reviewboard.mozilla.org/r/171914/#review180098

This series is still failing in CI. I'm unsure why. Having trouble reproducing locally :/

Comment hidden (mozreview-request)

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171912/diff/4-5/

Comment hidden (mozreview-request)

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171914/diff/4-5/

Michael Shal [:mshal]

Comment 33

•

7 years ago

mozreview-review

Comment on attachment 8902788 [details]
Bug 1393242 - Make hglib tests more resilient;

https://reviewboard.mozilla.org/r/174452/#review180102

Attachment #8902788 - Flags: review?(mshal) → review+

Gregory Szorc [:gps]

Assignee

Comment 34

•

7 years ago

mozreview-review

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

https://reviewboard.mozilla.org/r/171912/#review180104

::: python/mozversioncontrol/mozversioncontrol/__init__.py:150
(Diff revisions 4 - 5)
> +            # The cwd if the spawned process should be the repo root to ensure
> +            # relative paths are normalized to it.
> +            old_cwd = os.getcwd()
> +            try:
> +                os.chdir(self.path)
> -            self._client.open()
> +                self._client.open()
> +            finally:
> +                os.chdir(old_cwd)

I think this will fix the SpiderMonkey test failures.

We may still have a failure due to a bug in old Python not liking unicode types in an environment variable dictionary.

Michael Shal [:mshal]

Comment 35

•

7 years ago

mozreview-review

Comment on attachment 8903241 [details]
Bug 1393242 - Normalize paths in MercurialRevisionFinder;

https://reviewboard.mozilla.org/r/175034/#review180106

LGTM, as long as CI is happy :)

Attachment #8903241 - Flags: review?(mshal) → review+

Comment hidden (mozreview-request)

Comment on attachment 8902788 [details]
Bug 1393242 - Make hglib tests more resilient;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174452/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8903241 [details]
Bug 1393242 - Normalize paths in MercurialRevisionFinder;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175034/diff/1-2/

Comment hidden (mozreview-request)

Comment on attachment 8900526 [details]
Bug 1393242 - Vendor python-hglib 2.4;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171910/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171912/diff/5-6/

Comment hidden (mozreview-request)

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171914/diff/5-6/

Comment hidden (mozreview-request)

Comment on attachment 8902788 [details]
Bug 1393242 - Make hglib tests more resilient;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174452/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8903241 [details]
Bug 1393242 - Normalize paths in MercurialRevisionFinder;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175034/diff/2-3/

Comment hidden (mozreview-request)

Comment on attachment 8900526 [details]
Bug 1393242 - Vendor python-hglib 2.4;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171910/diff/4-5/

Comment hidden (mozreview-request)

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171912/diff/6-7/

Comment hidden (mozreview-request)

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171914/diff/6-7/

Comment hidden (mozreview-request)

Comment on attachment 8900524 [details]
Bug 1393242 - Helper function to obtain a repository from a build config;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171906/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8900525 [details]
Bug 1393242 - Return a Repository using configure preferences;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171908/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8902788 [details]
Bug 1393242 - Make hglib tests more resilient;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/174452/diff/4-5/

Comment hidden (mozreview-request)

Comment on attachment 8903241 [details]
Bug 1393242 - Normalize paths in MercurialRevisionFinder;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/175034/diff/3-4/

Comment hidden (mozreview-request)

Comment on attachment 8900526 [details]
Bug 1393242 - Vendor python-hglib 2.4;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171910/diff/5-6/

Comment hidden (mozreview-request)

Comment on attachment 8900527 [details]
Bug 1393242 - Hook up hglib to HgRepository;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171912/diff/7-8/

Comment hidden (mozreview-request)

Comment on attachment 8900528 [details]
Bug 1393242 - Use hglib for get_files_in_working_directory();

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171914/diff/7-8/

Pulsebot

Comment 53

•

7 years ago

Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/608f8c2125a7
Helper function to obtain a repository from a build config; r=mshal
https://hg.mozilla.org/integration/autoland/rev/318dfe168265
Return a Repository using configure preferences; r=mshal
https://hg.mozilla.org/integration/autoland/rev/fdd63a193b9d
Make hglib tests more resilient; r=mshal
https://hg.mozilla.org/integration/autoland/rev/175c99472cfd
Normalize paths in MercurialRevisionFinder; r=mshal
https://hg.mozilla.org/integration/autoland/rev/4a06dcbf5793
Vendor python-hglib 2.4; r=mshal
https://hg.mozilla.org/integration/autoland/rev/b07f70e6b32b
Hook up hglib to HgRepository; r=mshal
https://hg.mozilla.org/integration/autoland/rev/a2d3d62c0df7
Use hglib for get_files_in_working_directory(); r=mshal

Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)

Comment 54

•

7 years ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/608f8c2125a7
https://hg.mozilla.org/mozilla-central/rev/318dfe168265
https://hg.mozilla.org/mozilla-central/rev/fdd63a193b9d
https://hg.mozilla.org/mozilla-central/rev/175c99472cfd
https://hg.mozilla.org/mozilla-central/rev/4a06dcbf5793
https://hg.mozilla.org/mozilla-central/rev/b07f70e6b32b
https://hg.mozilla.org/mozilla-central/rev/a2d3d62c0df7

Status: ASSIGNED → RESOLVED

Closed: 7 years ago

status-firefox57: affected → fixed

Resolution: --- → FIXED

Target Milestone: --- → mozilla57

BMO Automation

Updated

•

6 years ago

Product: Core → Firefox Build System

You need to log in before you can comment on or make changes to this bug.