./mach try doesn't work if hg isn't installed

RESOLVED FIXED in Firefox 57

Status

Testing
General
RESOLVED FIXED
4 months ago
4 months ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

unspecified
mozilla57
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Comment hidden (empty)
Comment hidden (mozreview-request)

Comment 2

4 months ago
mozreview-review
Comment on attachment 8896090 [details]
Bug 1389366 - Fix |mach try| when hg is not installed. -

https://reviewboard.mozilla.org/r/167376/#review172724

::: tools/tryselect/vcs.py:55
(Diff revision 1)
> -            proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> -            output = proc.communicate()[0].strip()
> +            try:
> +                output = subprocess.check_output(cmd).strip()
> +            except (subprocess.CalledProcessError, OSError):
> +                continue

Thanks, good catch!

::: tools/tryselect/vcs.py:142
(Diff revision 1)
> -        if not find_executable('git-cinnabar'):
> +        try:
> +            subprocess.check_output('git cinnabar --version'.split(' '), stderr=subprocess.STDOUT)
> +        except subprocess.CalledProcessError:

Strange that find_executable wasn't working for you :/. There's also the `which` module that's checked into the tree, but I guess this is just as good.

nit: just use ['git', 'cinnabar', '--version']
Attachment #8896090 - Flags: review?(ahalberstadt) → review+
Comment hidden (mozreview-request)

Comment 4

4 months ago
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e24e342dbd3a
Fix |mach try| when hg is not installed. - r=ahal
https://hg.mozilla.org/mozilla-central/rev/e24e342dbd3a
Status: NEW → RESOLVED
Last Resolved: 4 months ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Blocks: 1390141
You need to log in before you can comment on or make changes to this bug.