✨ Don't use full file paths for violations #548
Conversation
Signed-off-by: Emily McMullan <emcmulla@redhat.com>
There was a problem hiding this comment.
I think you have to fix the dependency test as well:
https://github.com/konveyor/analyzer-lsp/actions/runs/8394658124/job/22992283193?pr=548#step:4:569
But other than that, this looks great and makes a ton of sense.
Signed-off-by: Emily McMullan <emcmulla@redhat.com>
eemcmullan
changed the title
Signed-off-by: Emily McMullan <emcmulla@redhat.com>
dymurray
added
the
cherry-pick/release-0.3
Signed-off-by: Emily McMullan <emcmulla@redhat.com>
provider/internal/java/dependency.go
Outdated
| @@ -411,7 +411,7 @@ func (p *javaServiceClient) parseDepString(dep, localRepoPath, pomPath string) ( | |||
| fp := resolveDepFilepath(&d, p, group, artifact, localRepoPath) | |||
|
|
|||
| d.Labels = addDepLabels(p.depToLabels, d.Name) | |||
| d.FileURIPrefix = fmt.Sprintf("file://%v", filepath.Dir(fp)) | |||
| d.FileURIPrefix = fmt.Sprintf("file://%v", filepath.Dir(strings.TrimPrefix(fp, localRepoPath))) | |||
There was a problem hiding this comment.
I don't think we should be doing this,
There was a problem hiding this comment.
this will break the dependency filtering logic, either we fix it in both places or don't change the fileURIPrefix at all, because this is mainly just an internal field
There was a problem hiding this comment.
Ahh, that makes sense. I am a +1 on changing this too then :)
There was a problem hiding this comment.
@eemcmullan this could be causing the failures too then maybe?
There was a problem hiding this comment.
Think so. Probably best then to trim this prefix in https://github.com/konveyor/analyzer-lsp/blob/main/cmd/analyzer/main.go#L423
Signed-off-by: Emily McMullan <emcmulla@redhat.com>
cmd/analyzer/main.go
Outdated
| var outputDepDag []konveyor.DepDAGItem | ||
| // trim m2 repo prefix for java deps | ||
| if name == "java" { | ||
| outputDepDag = trimDagItemPrefixes(ds) |
There was a problem hiding this comment.
sorry one small request...I think hardcoding for java provider here is unnecessary. there is no need to change prefix fields on dependencies as it isn't output to users in any incidents.
There was a problem hiding this comment.
I think this is for the --dep-output-file for the user. Just checking for the java provider here as the trim() function it's calling is only used to trim the local m2 repo prefix. Wanted to avoid issue with other dep providers. However, if I'm off-track, please let me know.
There was a problem hiding this comment.
I think it is fine to leave the prefixes as-is in the output @shawn-hurley wdyt?
There was a problem hiding this comment.
I don't know what users are doing with the dep output right now to have a strong opinion; I think because of that, I am ok with keeping the behavior, and if we need to change it later, we can come back. Thoughts?
Signed-off-by: Emily McMullan <emcmulla@redhat.com>
Closes #322