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

Fix incorrect license detection #2777 #2811

Merged
merged 3 commits into from
Feb 1, 2022

Conversation

KevinJi22
Copy link
Collaborator

@KevinJi22 KevinJi22 commented Jan 25, 2022

Adds a false positive rule for GNU tar. Also removes the public domain license as one of the expected outputs of the Debian slim copyright license test because the test has the same bug as the linked issue. Fixes #2777.

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁

@KevinJi22
Copy link
Collaborator Author

@pombredanne the one test that fails after applying this change is the exact same bug reported in #2777. The expected output for the test is located on line 3 of tests/packagedcode/data/debian/copyright/debian-slim-2021-04-07/usr/share/doc/tar/copyright-detailed.expected.yml, and you can see that the test also expects a public-domain license.

Does this mean the test's expected output is incorrect? If it is, I can also fix the test. Or is #2777 actually not a bug?

@pombredanne
Copy link
Member

@KevinJi22 This is not a clear cut... See #2777 (comment)

Adds a false positive rule for GNU tar. Fixes nexB#2777

Signed-off-by: Kevin Ji <kyji1011@gmail.com>
This removes the public domain license that was previously one of the
expected outputs of the Debian Slim Copyright license detection test.
The public domain license is not an actual license for this GNU tar
file.

Signed-off-by: Kevin Ji <kyji1011@gmail.com>
@pombredanne
Copy link
Member

@KevinJi22 this is a bug alright as documented in #2777 (comment)

@KevinJi22
Copy link
Collaborator Author

@pombredanne Yup, I fixed the bug in the second commit by removing the public domain license as one of the test's expected outputs.

Copy link
Member

@pombredanne pombredanne left a comment

Choose a reason for hiding this comment

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

the failure in https://github.com/nexB/scancode-toolkit/runs/5010476784?check_suite_focus=true is not your making. Looking all good... Merging now!
Thank you ++

@pombredanne pombredanne merged commit bebbcf7 into nexB:develop Feb 1, 2022
@KevinJi22 KevinJi22 deleted the false-positive-2777 branch February 1, 2022 21:34
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.

Incorrect license detection
2 participants