Fix reporter inclusion of subdir file licenses #10772
Conversation
Signed-off-by: Sasa Boros <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #10772 +/- ##
============================================
+ Coverage 57.38% 57.41% +0.02%
- Complexity 1671 1674 +3
============================================
Files 346 346
Lines 12759 12764 +5
Branches 1209 1208 -1
============================================
+ Hits 7322 7328 +6
Misses 4972 4972
+ Partials 465 464 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Sasa Boros <[email protected]>
| @@ -89,7 +89,7 @@ fun FileArchiverConfiguration?.createFileArchiver(): FileArchiver? { | |||
| ) | |||
| } | |||
|
|
|||
| val patterns = LicenseFilePatterns.getInstance().allLicenseFilenames | |||
| val patterns = LicenseFilePatterns.getInstance().allLicenseFilenames.map { "/**/$it" } | |||
There was a problem hiding this comment.
To be honest, I'm unsure whether this goes against the original purpose of the license file archiver. It could be that we intentionally only wanted to archive license files in the root. I guess we need to discuss this with @oss-review-toolkit/tsc.
There was a problem hiding this comment.
My two cents here. The LICENSE and NOTICE files are hardly ever located in the root of the source artifacts, but in the META-INF folder, and can be at different depths inside. When the source code origin is VCS, then the root is the most common place, yes. So one idea might be to handle these two cases differently.
There was a problem hiding this comment.
@sschuberth, is there any progress regarding the review and is there any way I can contribute further? Might be the least affecting option to look at root for VCS, and META-INF for ARTIFACT source code origin. Let me know what you think.
There was a problem hiding this comment.
and META-INF for ARTIFACT source code origin.
However, that would only work for Maven artifacts, so it's not really a generic solution.
Unfortunately, none of @oss-review-toolkit/tsc have chimed in so far to this discussion. I can put it onto the agenda for today's meeting, but the agenda already is quite full. So no promises about any progress, sorry 😞
There was a problem hiding this comment.
Maybe we chould have a (hard-coded or configurable) list of allowed parent directories. For RepositoryProvenance this would always include the VCS path, but also a list of other directories like META-INF, and possible more for non-Maven source artifacts.
There was a problem hiding this comment.
@sschuberth thanks for answering. That is also an interesting choice that involves a bit more changes, but overall doesn't break the structure. I will take a look, but I am also looking forward to tsc joining the discussion. :)
There was a problem hiding this comment.
After discussing this during our meeting, we decided to go for a more "minimally-invasive" approach that does not change the number of files getting archives. Please see #10828 for an alternative approach.
Signed-off-by: Sasa Boros <[email protected]>
Fix the reported issue that the reporter doesn't include the license files found in the subdirectories, which happens as a given when using the
ARTIFACTsource code origin.