1405602 - mach static-analyzer check --fix is failing (binary missing)

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

./mach static-analyzer check --check google-readability-braces-around-statements --fix dom/presentation/Presentation.cpp 


 0:07.39 
 0:07.39 Applying fixes ...
 0:07.39 Traceback (most recent call last):
 0:07.39     main()
 0:07.39     apply_fixes(args, tmpdir)
 0:07.39     subprocess.call(invocation)
 0:07.39   File "/usr/lib/python2.7/subprocess.py", line 168, in call
 0:07.39     return Popen(*popenargs, **kwargs).wait()
 0:07.39   File "/usr/lib/python2.7/subprocess.py", line 390, in __init__
 0:07.39     errread, errwrite)
 0:07.39   File "/usr/lib/python2.7/subprocess.py", line 1025, in _execute_child
 0:07.39     raise child_exception
 0:07.39 OSError: [Errno 2] No such file or directory
Error running mach:

probably because clang-apply-replacement is not installed.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

As a workaround on Debian
sudo ln -s clang-apply-replacements-5.0 clang-apply-replacements
or
sudo apt-get install clang

Comment 2

[Andi [:andi]]

This should come in the toolchain binary that we use.

Comment 3

[Andi [:andi]]

Attachment: Bug 1405602 - add clang-apply-replacements to path used by mach static-analysis.

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

Comment 4

[Sylvestre Ledru [:Sylvestre]]

We will need people to download clang-tidy again to get the version with clang-apply-replacement. Do you manage that?

Comment 5

[Andi [:andi]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> We will need people to download clang-tidy again to get the version with
> clang-apply-replacement. Do you manage that?

The process is automatic, this patch adds the integrity check to see if it has all of the binaries needed. If it sees missing binaries it will re-download clang-tidy. So this change is not transparent to the end-user.

Comment 6

[Andi [:andi]]

This change should not be landed before the binaries are built on toolchain.

Comment 7

[Andi [:andi]]

Attachment: add clang-apply-replacements to the clang-tidy build generated by toolchains

MozReview-Commit-ID: 1BT2ukzLoC3

Comment 8

[Andi [:andi]]

(In reply to Andi-Bogdan Postelnicu [:andi] from comment #7)
> Created attachment 8915163 [details] [diff] [review]
> add clang-apply-replacements to the clang-tidy build generated by toolchains
> 
> MozReview-Commit-ID: 1BT2ukzLoC3

This addresses the issue where the clang-apply-replacements binary was not present in the archive from artefact build.

Comment 9

[Andi [:andi]]

Attachment: add clang-apply-replacements to the clang-tidy build generated by toolchains

MozReview-Commit-ID: 1BT2ukzLoC3

Comment 10

[Andi [:andi]]

I've marked this issue to remain open since we need to also land the issue from review board after will be reviewed.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cca8c45c7a34
Add clang-apply-replacements to the clang-tidy build generated by toolchains. r=ehsan

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/cca8c45c7a34

Comment 13

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8915127 [details]
Bug 1405602 - add clang-apply-replacements to path used by mach static-analysis.

https://reviewboard.mozilla.org/r/186380/#review192218

Looks like the right stuff to do, bravo

Comment 14

[Andi [:andi]]

Note to self: remove leave-open flag after the two patches land in m-c.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/45f4d7722995
add clang-apply-replacements to path used by mach static-analysis. r=sylvestre

Comment 16

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

https://hg.mozilla.org/mozilla-central/rev/45f4d7722995

Comment 17

[Andi [:andi]]

This ca-n be closed now since all of the parchet Landes.