Skip to content
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

Fixes for #50 and #52 #53

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fixes for #50 and #52 #53

wants to merge 3 commits into from

Conversation

drewcrawford
Copy link

I'm not entirely sure that there are the "best" solution, but they work and pass the tests.

@cyrilpicat
Copy link
Contributor

Thanks for your contribution.

For #50, your explication is clear, thanks. Can you make a separate pull request for it, please? I will add two tests to check (if possible) that the two plugins (obj-c, java) are installed and update the documentation.

For #52, it's not 100% clear for me:

  • for the indexing part: why this works on some projects then? What cause the indexing part not to work? Is it related to Sonar 4.x? Was it working for you in 3.7.x? Does your fix still work in 3.7.x?
  • why the coverage report works on some project then? Was it working for you in 3.7.x? Does your fix still work in 3.7.x?

Thanks,

@cyrilpicat
Copy link
Contributor

For #50 don't loose your time, I will report your fix on my own.

@drewcrawford
Copy link
Author

With regard to #52, when using SonarQube 4.0.x:

  1. At this point in execution, no source code is ever indexed, ever, for any project I've run
  2. This causes fileExists to always be false. fileExists has several method definitions.
  3. It's not clear to me what part of this behavior is the bug. Is it reasonable to expect source code to be indexed in 4.0? I say it is unreasonable, because Sonar-5006 which lands in 4.2 makes a change so that "resource indexing is done by the core". I'm guessing, in releases earlier than 4.2, resource indexing is not done by the core, so that would be the responsibility of this project. However I can't find any documentation that actually speaks to this point, so my understanding could be wrong.

I can't speak to versions prior to 4.0 as I've never used them.

@cyrilpicat
Copy link
Contributor

Did you try to ask this question on sonar-dev? And check if these changes in behavior between 3.7 - > 4.0/4.1 -> 4.2 were expected?

And ask advice on how project are supposed to be handling this (and supporting the different behaviors by version of Sonar, which not very convenient) ?

Thanks,

@drewcrawford
Copy link
Author

I dipped into sonar-dev briefly about this while working on the patch but at present I have no answers. I was actually hoping that you would understand what is happening here better than I do.

I've posted again but based on the long delay on my last post additional insight may not be immediately forthcoming.

@drewcrawford
Copy link
Author

Sonar-dev reports that it's the plugin's responsibility to index resources in 4.1 and earlier. In 4.2 and later, requests to index have no effect, and indexing is no longer required.

There is some suggestion that this could represent a bug in the import stage of sonar-objectivec. #56 reports an importer bug that may be related, although, the importer is known to work at least some of the time in 4.x.

Does calling index again actually harm anything?

@cyrilpicat
Copy link
Contributor

I did update my project to Sonar 4.1.2 and I don't have your indexation issue. So this bug might not be to due to the 4.1 version, but more a project specific issue.

Do you have the same problem on different projects? Can you try on a sample projet, for example the one used in this article: http://blog.octo.com/en/track-your-ios-application-code-quality-using-sonar/

Thanks,

@cyrilpicat
Copy link
Contributor

Let me know if the fix suggested on sonar-dev is fixing your issues. That's a bit strange that it even worked before, and that is still work on all my/and other projects.

ObjectiveCPlugin.java:
public static final String FILE_SUFFIXES_DEFVALUE = ".h,.m";

@drewcrawford
Copy link
Author

There are a variety of problems that prevent me from testing with the sample project:

  • It fails to bootstrap
  • It is not actually possible to copy and paste the sonar-project.properties file from the webpage to a text editor
  • It fails to build, possibly related to its failure to bootstrap

I doubt this is project-related; it might be system-related.

@drewcrawford
Copy link
Author

With the following configuration:

  • SonarQube 4.1.2
  • sonar-objectivec snapshot 3.2
  • No other plugins

The issue (no OCLint violations reported) persists.

After adding the fix suggested on dev, the issue still persists.

So far the only fix for this issue that actually works is the patch in the PR.

@cyrilpicat
Copy link
Contributor

About the sample project:

  • I fixed the copy/paste issue, thanks
  • have you tried my fork of the repo (I modified the bootstrap.sh file)? It still fails to bootstrap?

Even if you can't try with this project, my question remains: is it (the indexation) working on another project of yours?

By the way I didn't ask but do you have a simplified version of your project that fails and that you could publish, in order for me to test it?

(btw I will test your fix to see if it has any bad side effects on my projects)

@drewcrawford
Copy link
Author

Master of your fork fails to bootstrap

$ git log --oneline | head -n 1
5d1add1 Update dependency Expecta to 0.2.3 to fix Xcode 5 build

$ ./bootstrap.sh 
Installing dependencies...
Submodule 'Assets/highlight.js' (git://github.com/iOctocat/highlight.js.git) registered for path 'Assets/highlight.js'
Cloning into 'Assets/highlight.js'...
remote: Counting objects: 7499, done.
remote: Compressing objects: 100% (4101/4101), done.
remote: Total 7499 (delta 3388), reused 7499 (delta 3388)
Receiving objects: 100% (7499/7499), 4.31 MiB | 805.00 KiB/s, done.
Resolving deltas: 100% (3388/3388), done.
Checking connectivity... done
Submodule path 'Assets/highlight.js': checked out '4c8f9eca94857c4647ea6988491c9cec0a2f42c7'

[!] Invalid `Podfile` file: Could not find cocoapods (= 0.26.2) amongst [activesupport-3.2.17, addressable-2.3.3, aws-s3-0.6.3, bigdecimal-1.1.0, builder-3.2.0, bundler-1.3.0, chronic-0.10.2, claide-0.4.0, cocoapods-0.29.0, cocoapods-core-0.29.0, cocoapods-downloader-0.3.0, cocoapods-try-release-fix-0.1.1, colored-1.2, crack-0.3.2, domain_name-0.5.12, escape-0.0.4, faraday-0.8.8, faraday_middleware-0.8.8, fuzzy_match-2.0.4, highline-1.6.19, highline-1.6.15, hpricot-0.8.6, http-cookie-1.0.1, i18n-0.6.9, io-console-0.3, json-1.8.0, json-1.5.5, json_pure-1.8.1, mechanize-2.7.1, mime-types-1.21, minitest-2.5.1, multi_json-1.8.4, multipart-post-1.2.0, mustache-0.99.4, nap-0.6.0, net-http-digest_auth-1.3, net-http-persistent-2.8, net-sftp-2.1.1, net-ssh-2.6.5, nokogiri-1.5.6, ntlm-http-0.1.1, open4-1.3.1, papertrail-0.9.7, rake-10.0.3, rake-0.9.2.2, rdiscount-2.1.6, rdoc-3.9.5, rest-client-1.6.7, ronn-0.7.3, rturk-2.10.1, rubygems-bundler-1.1.1, rvm-1.11.3.6, typingpool-0.7.5, unf-0.1.1, unf_ext-0.0.6, vcr-2.4.0, webmock-1.9.3, webrobots-0.1.1, wwdcdownloader-20.13.1, xcodeproj-0.14.1, xml-simple-1.1.2, yajl-ruby-1.1.0]. Updating CocoaPods might fix the issue.

 #  from /Users/drew/Dropbox/Code/ioctocat/Podfile:3
 #  -------------------------------------------
 #  
 >  gem 'cocoapods', '0.26.2'
 #  
 #  -------------------------------------------

Issue persists for any project I run

@drewcrawford
Copy link
Author

This particular commit is a sample project that exhibits the issue

@cyrilpicat
Copy link
Contributor

thanks for the link, I will try it on mine computer and let you know.

For the bootstrap issue, it looks like CocoaPods is not installed on your computer, or another version than '0.26.2' is installed. You can change the version in the Pod file if this is the case. Anyway, if you tried with different projects, I don't think it is worth trying again with this sample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants