Turned on static analyser after build

[maratal]

Closes #1202

[maratal]

Analyzing turned on (screenshot from "View raw logs"):

[lawrence-forooghian]

Will this fail the CI build if the static analyser spots a problem? It seems necessary to me — I don't think anybody's going to come and look at the build logs. (@QuintinWillison, what do you think? Was this implicit in #1202?)

[lukasz-szyszkowski]

Will this fail the CI build if the static analyser spots a problem? It seems necessary to me — I don't think anybody's going to come and look at the build logs. (@QuintinWillison, what do you think? Was this implicit in #1202?)

agree, but analyze can't fail xcodebuild so it's a bit tricky, there should be some output other than console log, something "human readable" like HTML files, to achieve this you need to pass config params to xcodebuild command and then check the existence of files from the analyzer, you can read about this here .
Additionally, you should add artifact step to upload the result of the analysis.

[QuintinWillison]

analyze can't fail xcodebuild

Really? It's been quite a number of years since I set this up in a project but my memory is that this was possible. Maybe things have changed significantly, but regardless, if CI doesn't fail when analyze fails then it's impotent and meaningless in reality.

[maratal]

Will this fail the CI build if the static analyser spots a problem?

We should turn on "Treat warnings as errors" to achieve this, as was pointed out by @QuintinWillison in one of our calls.

Additionally, you should add artifact step to upload the result of the analysis.

Isn't it in the same log file? @lukasz-szyszkowski

[lawrence-forooghian]

My memory of setting this up a long time ago is that out of the box it doesn't fail a build which produces analyzer warnings, and I think that's true even with "treat warnings as errors" enabled. My rough memory is that you need to do what @lukasz-szyszkowski said — set up something separate to check the output of xcodebuild.

[lukasz-szyszkowski]

Isn't it in the same log file?

it produces something like this
report-2b2517.html.zip

[maratal]

Guys, do you have any idea where "Ably_vers.c" file comes from? It fails while trying to analyze it. The project contains nothing like that. Also any feedback on the approach taken in this PR is very appreciated. @lukasz-szyszkowski @lawrence-forooghian @QuintinWillison @ben-xD

UPDATE:
Turned out it's generated _vers.c

[QuintinWillison]

Sorry, @maratal - perhaps I should have spotted this on first pass but I still think this isn't doing what you expect it might.

[jamienewcomb]

@maratal what is the status of this one? are you wanting on another review from Quintin. If so, please re-request

[QuintinWillison]

Sorry, @jamienewcomb - my bad - I was somewhat overloaded last few weeks so didn't come back to this. Reviewing again now.

[QuintinWillison]

Looks sensible from a workflows perspective, though I'll admit the solution as a whole feels a little fragile (i.e. failing due to second hand information, being presence of reports output). Either way, presumably this is better than what we had before so I do think it'll move the codebase forwards (even though I predict we'll probably end up looking at this again in the future... time will tell).