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

feat(package-configurations): Add some path excludes for commons-compress #192

Merged
merged 2 commits into from
May 30, 2024

Conversation

sschuberth
Copy link
Member

First and foremost, exclude some-900kb-text.txt 1 which just contains random text in a single line, which makes ScanCode time out.

Secondly, while at it, also exclude the .github build directory committed to VCS.

@sschuberth sschuberth requested a review from a team as a code owner May 28, 2024 17:44
@sschuberth sschuberth enabled auto-merge (rebase) May 28, 2024 17:44
Copy link
Member

@tsteenbe tsteenbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also package configuration for this package that is pending us agree on some rules. Mine was party generated with ort helper cli and a bit more elaborate/complete, propose you we use mine.

---
id: "Maven:org.apache.commons:commons-compress:1.26.2"
vcs:
  type: "Git"
  url: "https://gitbox.apache.org/repos/asf/commons-compress.git"
  revision: "95727006cac0892c654951c4e7f1db142462f22a"
path_excludes:
- pattern: ".github/**"
  reason: "BUILD_TOOL_OF"
- pattern: "CODE_OF_CONDUCT.md"
  reason: "DOCUMENTATION_OF"
- pattern: "CONTRIBUTING.md"
  reason: "DOCUMENTATION_OF"
- pattern: "SECURITY.md"
  reason: "DOCUMENTATION_OF"
- pattern: "src/changes/**"
  reason: "DOCUMENTATION_OF"
- pattern: "src/main/java/org/apache/commons/compress/archivers/examples/**"
  reason: "EXAMPLE_OF"
- pattern: "src/site/**"
  reason: "DOCUMENTATION_OF"
- pattern: "src/test/**"
  reason: "TEST_OF"
license_finding_curations:
- path: "src/main/java/org/apache/commons/compress/archivers/zip/*.java"
  detected_license: "LicenseRef-scancode-proprietary-license"
  reason: "REFERENCE"
  comment: |
    Match on 'Refer to the section in this document entitled Incorporating PKWARE Proprietary Technology 
    into Your Product" for more information', see
    https://github.com/apache/commons-compress/blob/rel/commons-compress-1.26.2/src/main/java/org/apache/commons/compress/archivers/zip/X0017_StrongEncryptionHeader.java#L251-L252
    Git blame on sentence so this is a reference to ZIP file format specification, see
    https://github.com/apache/commons-compress/commit/a433f62
  concluded_license: "NONE"

@sschuberth
Copy link
Member Author

sschuberth commented May 28, 2024

I also package configuration for this package that is pending us agree on some rules. Mine is a bit more elaborate

IMO that's not a reason to block this PR, though. I'm sorry, but we cannot hold our breath and wait for you to move things forward, see also #131 which has been pending for almost a year.

And most importantly, your version lacks the path exclude for the file that causes the ScanCode timeout. Thus I'd like to get this merged, and later add your changes to it.

Edit: Scratch that, I missed that it's covered by the broader pattern: "src/test/**".

@sschuberth sschuberth requested a review from a team May 28, 2024 18:57
@tsteenbe
Copy link
Member

tsteenbe commented May 29, 2024

IMO that's not a reason to block this PR, though. I'm sorry, but we cannot hold our breath and wait for you to move things forward, see also #131 which has been pending for almost a year.

Did I say I wan't to block this PR - maybe it wasn't clear from my comment but I simply requested this PR to be updated to include the additional line from the codeblock in my comment

The main goal of excluding the `src/test` directory is to exclude the
file at

    src/test/resources/org/apache/commons/compress/COMPRESS-649/some-900kb-text.txt

(also see [1]) which contains random text in a single line, causing
ScanCode 32.1.0 to time out.

[1]: https://github.com/apache/commons-compress/blob/rel/commons-compress-1.26.2/src/test/resources/org/apache/commons/compress/COMPRESS-649/some-900kb-text.txt

Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth added a commit that referenced this pull request May 29, 2024
Incorporate reviewed changed from @tsteenbe, see [1].

[1]: #192 (review)

Signed-off-by: Sebastian Schuberth <[email protected]>
@sschuberth sschuberth force-pushed the exclude-commons-compress-900kb-text branch from 0f7a5a2 to 5b91e14 Compare May 29, 2024 16:00
Incorporate reviewed changed from @tsteenbe, see [1].

[1]: #192 (review)

Signed-off-by: Thomas Steenbergen <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
@sschuberth sschuberth force-pushed the exclude-commons-compress-900kb-text branch from 5b91e14 to d56bbf9 Compare May 29, 2024 16:01
@sschuberth
Copy link
Member Author

Did I say I wan't to block this PR - maybe it wasn't clear from my comment but I simply requested this PR to be updated to include the additional line from the codeblock in my comment

Then this was indeed a misunderstanding. I've reviewed and incorporated your changes.

@sschuberth sschuberth dismissed fviernau’s stale review May 30, 2024 04:48

Comment addressed.

@sschuberth sschuberth merged commit d391cd0 into main May 30, 2024
2 checks passed
@sschuberth sschuberth deleted the exclude-commons-compress-900kb-text branch May 30, 2024 04:48
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.

3 participants