AmadeusITGroup / sonar-stash

Stash (BitBucket) plugin, a pull-request decorator which allows to integrate SonarQube violations directly into your pull-request
MIT License
165 stars 82 forks source link

Comment "xxx" cannot be pushed to Stash like it does not belong to diff view #25

Closed racodond closed 7 years ago

racodond commented 8 years ago

Hi,

Sometimes, I face the following issue: Comment "squid:S1134" cannot be pushed to Stash like it does not belong to diff view

16:52:58.751 INFO  - ANALYSIS SUCCESSFUL
16:52:58.752 INFO  - Executing post-job class org.sonar.plugins.stash.StashIssueReportingPostJob
16:52:59.157 INFO  - Comment "squid:S1134" cannot be pushed to Stash like it does not belong to diff view - nc3cockpit/src/com/nespresso/nc3/cockpit/components/header/Nc3LeftSectionHeaderComponent.java (line: 14)
16:52:59.157 INFO  - New SonarQube issues have been reported to Stash.
16:52:59.269 INFO  - SonarQube analysis overview has been reported to Stash.

But line 14 seems to be in the diff view:

image

Thank you

Regards,

David

acopet commented 8 years ago

Hello David,

We have difficulties to reproduce this issue...

Could you please confirm SQ analysis is published to the expected pull-request (by checking sonar.stash.pullrequest.id property value in SQ command line)? And could you please retry this analysis in debug mode? We will check among other things if we will find the following log from your debug run:

Update Stash comment "xxx": set comment line to destination diff line (xxx)

Cheers, Antoine

racodond commented 8 years ago

Could you please confirm SQ analysis is published to the expected pull-request (by checking sonar.stash.pullrequest.id property value in SQ command line)?

I confirm.

And could you please retry this analysis in debug mode? We will check among other things if we will find the following log from your debug run

To which email address can I privately send the log file?

We manually define modules on this project. Couldn't there be an issue regarding the full path of the file as the root of the Stash repository is not the root of the SonarQube analysis?

image

11:51:00.865 INFO  - Comment "squid:S1134" cannot be pushed to Stash like it does not belong to diff view - nc3cockpit/src/com/nespresso/nc3/cockpit/components/header/Nc3LeftSectionHeaderComponent.java (line: 14)
acopet commented 8 years ago

Hello David,

Yes that's it! Path contains in addition "hybris/bin/custom": due to this, the plugin does not manage to attach the comment in the "Diff" view. To check this, could you please tell me if the issue appear in the "Overview" tab of the pull-request?

racodond commented 8 years ago

Hello,

To check this, could you please tell me if the issue appear in the "Overview" tab of the pull-request?

Yes, they are displayed in the Overview tab but quite 'useless' as you don't know on which files and where exactly is the issue.

Yes that's it! Path contains in addition "hybris/bin/custom": due to this, the plugin does not manage to attach the comment in the "Diff" view.

How do you plan to overcome this limitation?

Thank you

David

acopet commented 8 years ago

I do agree: this display is really useless. Basically we check if an issue belongs to the Diff view, not to have this kind of display. We need to check why this filter did not work with this use case...

Anyway, if I well understand, your SQ configuration is so not in the root folder of your project? How is the structure of your project? And how do you launch your SQ analysis? Sorry for all these additional questions, just to be able to quickly reproduce this issue ;)

racodond commented 8 years ago

The root folder of the SonarQube analysis is hybris/bin/custom. Each folder below is a module: sonar.modules=nc3businessprocess,nc3cockpit,.... We run the analysis with Ant.

racodond commented 8 years ago

Hi,

Any feedback on this? Would be adding a "root folder" property an option?

Thanks

David

racodond commented 8 years ago

See See https://github.com/AmadeusITGroup/sonar-stash/pull/30

re-schneider commented 8 years ago

Linked to Pull-request #24

kirkor commented 8 years ago

This problem can be solved by proper configuration. I've describe it in PR #56 and here: https://github.com/kirkor/sonar-stash-test-project/blob/master/README.md

t-8ch commented 8 years ago

@racodond Does the change proposed @kirkor also solve your issue?

maaltan commented 7 years ago

First off, this discussion is spread over so many pull requests I'm not sure where to post this. this one is still open so i chose here.

I cannot accept the "bad configuration" solution for this. Our project is complicated with arbitrary paths. The assumptions made by the plugin are invalid for our project.

1: our sonar-project.properties file CANNOT be in GIT-ROOT directory.
2: our projects do not have a fixed subpath. 3: We have external processes that depend on the module keys so adding a full path to the sonar.modules list will be very difficult to adapt to..

I feel the plugin should pay attention to the projectBaseDir of each subproject.

Here is a snippet of our project properties to give you an idea of what we need.

subproject.sonar.java.binaries=absolute path to bins subproject.sonar.projectBaseDir=absolute path to subproject

sonar.sources=src/main/java sonar.modules=subproject, <20-50 more>

to be fair: my error is slightly different: [sonar:sonar] 13:51:21.890 INFO - Comment "0159D1D153DF18CE2D" cannot be pushed to Stash like it does not belong to diff view - null (line: 36)

kingoleg commented 7 years ago

Hi, All

How to investigate it and fix?

I have a multi modules and multi language mvn project.

And for one of PR consists with only java classes changes it returnes:

Comment "javascript:TrailingWhitespace" cannot be pushed to Stash like it does not belong to diff view - null (line: 25)

t-8ch commented 7 years ago

I am looking into this. Honestly I am not a frequent user of the multi module project. Any full example with source code and the call to the sonar scanner that can reproduce this would accelerate this.

kingoleg commented 7 years ago

I'm trying to create the example. Is there an option to ignore any issue related to not visible code and approve PR?

t-8ch commented 7 years ago

Only issues in the new code and visible in the diff are used by the plugin. sonar.stash.reviewer.approval is documented as doing this.

Edit: new issues are used, not ignored

kingoleg commented 7 years ago

That's an issue:

sonar.stash.include.overview shows there is an issue, and sonar.stash.reviewer.approval doesn't approve PR

t-8ch commented 7 years ago

Can you please open dedicated issues for those

kingoleg commented 7 years ago

Em, wait, I will try it on master, not on 1.2

t-8ch commented 7 years ago

So I could find a reproduction case for the situation @maaltan described. This issue seems to be fixed by some changes we did on a development branch which is currently not on GitHub. This branch is currently in our internal beta test and should be available soon, I will probably push it tomorrow.

Fergus-Murray commented 7 years ago

In our scenario we have the solution file (mysolution.sln) inside the \git\src folder with 2 sub project folders, in this structure. \git +\src ++\vsproj1 ++\vsproj2

This is a C# solution analysed using the sonar msbuild scanner which automatically creates the .sonarqube folder in the project solution folder (\git\src.sonarqube) regardless of where the working directory is set. The output of the sonarqube scanner is put in this folder and once the plugin completes it doesn't contain any references to my "src" directory at all, and I see this reported: INFO: Comment "csharpsquid:S1144" cannot be pushed to Stash like it does not belong to diff view - vsproj1/Utils/mycode.cs (line: 27)

on the BB server the diff source file is shown as /src/vsproj1/Utils/mycode.cs

What we need is a way to tell the plugin that all references need to be prefixed with the /src folder, otherwise we will never see the issues in amongst the diff in BBS.

t-8ch commented 7 years ago

@Fergus-Murray I am still trying to wrap my head around this. Can you confirm that the root of the sonarqube project is the same as the root of the git repo? If so this should be made to work out of the box.

Fergus-Murray commented 7 years ago

No, the root of the sonarqube project is ALWAYS set to the path containing the vs solution .sln file. This happens due to using the msbuild runner and cannot be overridden.

In my example regardless of what I set the working directory to, the sonarqube root is always \git\src\ (and I see the .sonarqube always being created in there. Our git root is \git so the two are not the same.

t-8ch commented 7 years ago

@Fergus-Murray so I should probably try to reproduce this. However I have never used the msbuild runner. To accelerate that, could you provide me with an example repo and the commands to execute the scanner?

Fergus-Murray commented 7 years ago

@t-8ch unfortunately I'm unable to share any code, however I guess you could use: https://github.com/SonarSource/sonar-scanning-examples/tree/master/sonarqube-scanner-msbuild/CSharpProject as a base

t-8ch commented 7 years ago

@Fergus-Murray It works for me with the example project. Could you create a minimal test case like the example project to reproduce it?

t-8ch commented 7 years ago

@Fergus-Murray I think the next version of the msbuild runner will fix this: https://github.com/SonarSource-VisualStudio/sonar-scanner-msbuild/blob/69431958d43a95d9c6b70f607f20fd67d12f49f6/SonarScanner.Shim/PropertiesFileGenerator.cs#L115 The official roadmap ist to release the 2.3.0 version on 2017-03-31

Could you try this new version, configure the basedir manually and check if everythin will work then?

t-8ch commented 7 years ago

The new versions sonar-stash allow the setting of a repository root directory. This should fix this issue. Please verify.

tonyfalabella commented 5 years ago

The property sonar.stash.repository.root worked for MSBuild .sln project that had the solution file under dir "Trading\Solutions\Source\xxx.sln". These are the 2 properties I had to set to get it to publish the comments.

/d:sonar.stash.repository.root=${WORKSPACE}
/d:sonar.modules=Trading/Solutions/Source