search

raimon49 / pip-licenses

Dump the license list of packages installed with pip.
MIT License
314 stars 45 forks source link

Verify multiple licenses individually #102

Closed khatkar closed 3 years ago

khatkar commented 3 years ago
codecov[bot] commented 3 years ago

Codecov Report

Merging #102 (e329d34) into master (b6bfeac) will increase coverage by 0.01%. The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #102      +/-   ##
==========================================
+ Coverage   98.96%   98.97%   +0.01%     
==========================================
  Files           1        1              
  Lines         385      390       +5     
==========================================
+ Hits          381      386       +5     
  Misses          4        4
Impacted Files Coverage Δ
piplicenses.py 98.97% <100.00%> (+0.01%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b6bfeac...e329d34. Read the comment docs.

khatkar commented 3 years ago

@cdce8p Based on @raimon49 's suggestions, I am now aware that license names can contain , Using a ; delimiter when combining licenses into a string can fix this issue. I tried to preserve the current behavior of this lib and that's why I translated multiple licenses into an AND expression where all licenses must appear on the allow list. I can update this PR to consider only a single match (OR expression). For example all of the the following checks will pass:

keyring     21.4.0     Python Software Foundation License, MIT License

pip-licenses --allow-only="Python Software Foundation License"
pip-licenses --allow-only="MIT License"
pip-licenses --allow-only="Python Software Foundation License;MIT License"
dkarp0 commented 3 years ago

This would be a great contribution! If you're allowing either of the licenses then I don't think that will work with your current implementation using set.difference() though. Consider the following:

keyring     21.4.0     Python Software Foundation License, MIT License
fastly      0.5.1        MIT License

pip-licenses --allow-only="Python Software Foundation License" - Fail
pip-licenses --allow-only="MIT License" - Succeed
pip-licenses --allow-only="Python Software Foundation License;MIT License" - Succeed
khatkar commented 3 years ago

@dkarp0 Yes Daniel, the current implementation checks for all. I was proposing the idea, and didn't made change until now. It should work with the new commit.

khatkar commented 3 years ago

This PR is ready for another review. I've addressed concerns and tests pass. Let me know if I should squash into a single commit.

raimon49 commented 3 years ago

@khatkar You don't have to squash commits into one.