-
-
Notifications
You must be signed in to change notification settings - Fork 7
Update OCLint setup to use native tools instead of xcpretty #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zippy1978
approved these changes
Nov 14, 2022
davidy4ng
approved these changes
Nov 14, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
gaelfoppolo
added a commit
that referenced
this pull request
Nov 16, 2022
* develop: Remove build warnings (local and CI) (#42) Refact code coverage to use common Xcode bundle result (#41) Update OCLint setup to use native tools instead of xcpretty (#40) # Conflicts: # objc-lang/src/main/java/fr/insideapp/sonarqube/objc/issues/oclint/OCLintReportParser.java # objc-lang/src/main/java/fr/insideapp/sonarqube/objc/issues/oclint/OCLintSensor.java # objc-lang/src/test/java/fr/insideapp/sonarqube/objc/issues/oclint/OCLintJSONDatabaseBuilderTest.java # objc-lang/src/test/java/fr/insideapp/sonarqube/objc/issues/oclint/OCLintReportParserTest.java # objc-lang/src/test/java/fr/insideapp/sonarqube/objc/issues/oclint/OCLintSensorTest.java # sonar-apple-plugin/src/main/java/fr/insideapp/sonarqube/apple/ApplePlugin.java
gaelfoppolo
added a commit
that referenced
this pull request
Nov 28, 2022
* Add sonar.apple.jsonCompilationDatabasePath parameter * Fix AppleTestsSensorTest * Add JSON Compilation Database merge logic * Upgrade JProc version to 2.8.0 * Export some logic to a separate object OCLintJSONDatabaseBuilder * Adding OCLintJSONDatabaseBuilderTest * Create OCLintExtractor to run oclint-json-compilation-database * Adding parsing step to OCLintExtractor * Update OCLintReportParser * Merging all the piece into OCLintSensor * Some fixes * Update documentation * Updating documentation * Update OCLintReportParserTest * Cleaning * Add headers * Improve OCLint regex for the include parameter * Update README * Fix error in unit tests * Fix bug (from Sonar Cloud) * Fix issues (from Sonar Cloud) * Fix runtime failure, the file wasn't properly closed before it was read by another ressource
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Current OCLint implementation suffers from two limitations:
a) the need to run a second build (with
xcodebuild
)b) the need to use an external tool (other than OCLint itself)
This PR aims to solve both of these issues:
a) performing only one build
b) removing xcpretty, which produces the JSON Compilation Database
Details
Performing only one build
Currently we need execute two builds, with some minor changes in their parameters.
One is used for Periphery and the other is used for OCLint.
If we try to use the result of the first build for OCLint, we'll got the following issue:
oclint: error: one compiler command contains multiple jobs
.To fix this we supply a flag,
COMPILER_INDEX_STORE_ENABLE
with the valueNO
. It seems OCLint internal logic (withclang
) does not work with index store.But, if we add this flag, Periphery will then not work, since the tool relies on the index store to work.
So if we want both tools to work we need two builds, one with index store enabled and the other disabled.
This is not ideal, we should be able to perform only one build.
By diffing the two
compile_commands.json
file generated from each build, we see only two lines were added when index store is enabled.Removing these two lines from
compile_commands.json
allows toOCLint
to work.We can now use only one build, with index store enabled.
Right before launching OClint, we "clean"
compile_commands.json
and that good.Removing
xcpretty
Currently we use
xcpretty
to generate thecompile_commands.json
, used by OCLint.This file is in fact a JSON Compilation Database, a format for specifying how to replay single compilations independently of the build system.
Many tools provide a way to generate this JSON Compilation Database, so why not Xcode?
Well, it is possible for
clang
, which is used by Xcode behind the scene, to generate fragments of JSON Compilation Database, using the-MJ
. But using this flag is complicated and would require many flag additions.Fortunately, there is a (poorly documented) flag in LLVM:
-gen-cdb-fragment-path
, which will do this work for us. We only need to supply an output path and we're done! Mostly done.Since clang produce fragments of JSON Compilation Database (one for each file), we need to concatenate those fragments together to create a compilation database.
We can now remove the use of
xcpretty
since Xcode will do the work for us.The first issue (index store enabled) is still here, so we still need to perform the cleaning of the file.
DoD
Links
oclint/oclint#611
https://thisiskyle.me/posts/using-oclint-without-rebuilding-your-project.html
https://gist.github.com/drumnkyle/b4328e7447b63af88e6f9a9daecc918d
https://gist.github.com/T1T4N/f4d63a44476eb5c7046cc561cb8c7f77
https://clang.llvm.org/docs/JSONCompilationDatabase.html