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

New Issues could not be mapped to files under review #37

Open
kstevenard opened this issue Feb 26, 2016 · 9 comments
Open

New Issues could not be mapped to files under review #37

kstevenard opened this issue Feb 26, 2016 · 9 comments

Comments

@kstevenard
Copy link

I find out a case where new issues are identified by SonarQube but where the sonar-gerrit-plugin fails to map them to files under review.
this is when in the file path there is a "src" folder. The sonar-gerrit-plugin apparently use a shortened key for those files due to the following block of code:
protected String parseFileName(@NotNull String fileName) { return fileName.replaceFirst(MAVEN_ENTRY_REGEX, "src/"); }

If gerrit returns something like this:
{"/COMMIT_MSG":{"status":"A","lines_inserted":9},"a/b/c/src/x/y/z/a.source.file":{"lines_inserted":5}}
then it will keep in memory the following:
11:17:55.564 DEBUG - [GERRIT PLUGIN] Modified files in gerrit (keys) : [src/x/y/z/a.source.file] 11:17:55.564 DEBUG - [GERRIT PLUGIN] Modified files in gerrit (values): [a/b/c/src/x/y/z/a.source.file]
Which means that when later the plugin will check if a/b/c/src/x/y/z/a.source.file is under review it will not find it and will not post any comments:

11:17:55.615 DEBUG - [GERRIT PLUGIN] Decorate: a/b/c/src/x/y/z/a.source.file 11:17:55.615 DEBUG - [GERRIT PLUGIN] Start Sonar decoration for Gerrit 11:17:55.615 DEBUG - [GERRIT PLUGIN] Look for in Gerrit if the file was under review, name=a/b/c/src/x/y/z/a.source.file 11:17:55.615 DEBUG - [GERRIT PLUGIN] File a/b/c/src/x/y/z/a.source.file is not under review

Then is this parseFileName function really needed? is there cases when it should be done?

@mildis
Copy link
Contributor

mildis commented Feb 26, 2016

Filename mapping is required as Gerrit and Sonar do not refer to the file the same way.
In which configuration did you get the file not under review message ?
Where you using git submodules or Sonar branches ?

@kstevenard
Copy link
Author

I got this issue when the source code file is under a directory structure which contains a folder named "src" in this case all the structure before "src" folder is removed at the gerrit plugin side and used as the key, while the value still contains the full path. So when the plugin cross entries from sonarqube project and from gerrit patchset it fails because the path from sonarqube is bigger and the one retrieved from gerrit is altered due to the following: fileName.replaceFirst(MAVEN_ENTRY_REGEX, "src/")

I'm not using git submodules or sonar branches. I'm checking if i can workaround this problem by changing the sonarqube project configuration.

@kstevenard
Copy link
Author

Hi Mildis,
By configuring some modules within SonarQube, I have been able to fix the issue. Could you please let me know what is the intent of doing fileName.replaceFirst(MAVEN_ENTRY_REGEX, "src/") while it may also impact other type of project (non maven or java) when there is a src directory.
I guess not doing it will not have any impact on java mvn project.

@mildis
Copy link
Contributor

mildis commented Mar 9, 2016

The regex was meant to match the name of the file being analyzed and the one returned bu Gerrit.
You are right it has been currently used only with Java Maven projects following Maven’s directories structure.
Feel free to submit a pull request for the fix you made to be included in the main code.

@kstevenard
Copy link
Author

I can't push anything reliable at this time. we will investigate more how it could be done without impact while supporting most of projects structures.
Anyway when encountering this issue we could workaround it by playing with projectBaseDir & sources parameters to expose a path which will match with the path checked by this plugin.

Thank you a lot for your great plugin :)

@mildis
Copy link
Contributor

mildis commented Mar 19, 2016

I’m trying to figure out how to solve this programmatically without adding too much overhead in the file-matching algorithm.
Is you project public ? Can you send the output of a tree command ?
BTW, which version are you using (SonarQube and sonar-gerrit-plugin) ?

@kstevenard
Copy link
Author

sorry for delay, I'm using a snapshot of your plugin ( https://github.com/tech-advantage/sonar-gerrit-plugin/tree/840201d5fd5c38008d329862a749984b914a327b ) and sonarqube 5.2
My project is not public, but a set of directory structures triggering the different behavior could be created.

By the way I have seen your recent commit, i will give them a try.

@greg7859
Copy link
Contributor

I would add some informations.
The review and the analyze must be done on the same folder else the path will be different.

If you have this project:
*
|-P1
| |- pom.xml
|-P2
| |- pom.xml
|- pom.xml

If you analyze the project from /, each file will contain the project folder (P1, P2, ..) : P1/src/...
If you run a review inside the project (like P1, P2), the file under review will be src/...

And the matching will not be done.

Could you check if the analyzes are started from the same folder ?

@kstevenard
Copy link
Author

@greg7859 most of the time its started from the same folder. Quite recently we also run into other issues with this plugin on some C# projects, because there the sonarqube client for C# projects do more or less like the maven plugin (breaking down the application solution into modules) then whatever the location from which the analysis is executed the path of the file known and stored inside sonarqube will be always shorter than the path seen in Gerrit. Then it makes the mapping failing. In addition to that with those C# projects we don't have like under maven regular structure a "src/" folder to help the parsing and then the mapping.

As we were running into a lot of issues on various type of projects, we added two additional ways of doing mapping into getFileNameFromInputPath.

  1. for each gerrit modified files check if the path is contained inside the full path of the resource (complete path of the file analyzed by sonarqube on the file system, then the modified file under gerrit should always be a subpart of the full path of the file on the file system)
  2. for each gerrit modified files check if it contains the resource relative path (i admit this one could generate wrong mapping) but as 1. should solve all our issues we should never go inside 2.

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

No branches or pull requests

3 participants