clang-format march command does not gracefully handle failure to fetch/locate binaries

RESOLVED FIXED in Firefox 55

Status

RESOLVED FIXED
2 years ago
6 months ago

People

(Reporter: bryce, Assigned: bryce)

Tracking

Trunk
mozilla55

Firefox Tracking Flags

(firefox53 wontfix, firefox55 fixed)

Details

MozReview Requests

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

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
Note, all the code mentioned here resides in `tools/mach_commands.py`.

When running `mach clang-format` two attempts are made to fetch/locate the clang format binaries. One of these looks to be redundant and can be removed.

When fetching/locating the binaries fails the code looks like it intends to early return, however due to the return values of `locate_or_fetch` the early return is not hit, so we get a nasty error later. The return values can be altered so we get the graceful failure behaviour that is intended.
(Assignee)

Updated

2 years ago
Assignee: nobody → bvandyk
(Assignee)

Comment 1

2 years ago
Derp. Realised the fetches happening are actually fetching 2 different files, so it's only the graceful failure code which is not being hit correctly and needs fixing.
(Assignee)

Updated

2 years ago
Summary: clang-format march command attempts to fetch/locate binaries twice and does not gracefully handle failure to do so → clang-format march command does not gracefully handle failure to fetch/locate binaries
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Thanks!
Is there an easy way to reproduce the bug that you are fixing?
(Assignee)

Comment 5

a year ago
Yeah! If you run `./mach clang-format` and accept the default of N/no to the download prompt, you will receive "download aborted" but also a mach error and a stack trace. With the change, you will just see "download aborted" and a clean exit.

Comment 6

a year ago
mozreview-review
Comment on attachment 8821330 [details]
Bug 1325481 - Update clang-format fetch/locate codepath to gracefully fail.

https://reviewboard.mozilla.org/r/100644/#review127894
Attachment #8821330 - Flags: review?(sledru) → review+

Comment 7

a year ago
Pushed by bvandyk@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a03d0d06a809
Update clang-format fetch/locate codepath to gracefully fail. r=sylvestre

Comment 8

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/a03d0d06a809
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
status-firefox53: affected → wontfix

Updated

6 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.