Skip to content

new check: LICENSE should not contain variables#368

Closed
laumann wants to merge 1 commit intopkgcore:masterfrom
laumann:check-license
Closed

new check: LICENSE should not contain variables#368
laumann wants to merge 1 commit intopkgcore:masterfrom
laumann:check-license

Conversation

@laumann
Copy link
Copy Markdown
Contributor

@laumann laumann commented Apr 30, 2022

Extend MetadataVarCheck with a new method checking the LICENSE variable
for other variables. The only exception is LICENSE itself. The check
accepts occurrences of $LICENSE and ${LICENSE}.

Closes: #366
Signed-off-by: Thomas Bracht Laumann Jespersen t@laumann.xyz

@arthurzam
Copy link
Copy Markdown
Member

@laumann
Also, as our discussion at #gentoo-qa, please also add check for LICENSE="${LICENSE} ${LICENSE/M/G}-2", so we know we disallow it.
In case we decide to allow it in future, we can test the behavior change.

@laumann
Copy link
Copy Markdown
Contributor Author

laumann commented Apr 30, 2022

Hmm, seems i broke some other tests.

please also add check for LICENSE="${LICENSE} ${LICENSE/M/G}-2", so we know we disallow it.

I'll add it. I really doubt that it would be allowed though :-)

Extend MetadataVarCheck with a new method checking the LICENSE variable
for other variables. The only exception is LICENSE itself. The check
accepts occurrences of $LICENSE and ${LICENSE}.

Closes: #366
Signed-off-by: Thomas Bracht Laumann Jespersen <t@laumann.xyz>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 1, 2022

Codecov Report

Merging #368 (4351176) into master (76ea79d) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #368   +/-   ##
=======================================
  Coverage   95.50%   95.51%           
=======================================
  Files          54       54           
  Lines        7366     7376   +10     
  Branches     1785     1788    +3     
=======================================
+ Hits         7035     7045   +10     
  Misses        204      204           
  Partials      127      127           
Impacted Files Coverage Δ
src/pkgcheck/checks/codingstyle.py 96.68% <100.00%> (+0.07%) ⬆️

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 76ea79d...4351176. Read the comment docs.

Copy link
Copy Markdown
Contributor

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

Could you also paste the current matches as a comment on this PR?

@laumann
Copy link
Copy Markdown
Contributor Author

laumann commented May 1, 2022

Could you also paste the current matches as a comment on this PR?

There aren't any! I think @ulm fixed them not too long ago.

@laumann
Copy link
Copy Markdown
Contributor Author

laumann commented May 1, 2022

but if I check out for example d07e7ff26348a277be0d4d18fc63f6bdfea94038 and run the scan I get the following hits:

dev-lang/mmix
  ReferenceInMetadataVar: version 20160804-r1: LICENSE includes variable: ${PN}

games-emulation/dolphin
  ReferenceInMetadataVar: version 5.0_p20210506-r3: LICENSE includes variable: KEEP_BUNDLED[*]
  ReferenceInMetadataVar: version 9999: LICENSE includes variable: KEEP_BUNDLED[*]

games-rpg/comi
  ReferenceInMetadataVar: version 1: LICENSE includes variable: ${PN}

Copy link
Copy Markdown
Member

@arthurzam arthurzam left a comment

Choose a reason for hiding this comment

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

Asking for last blocker from QA team or other devs before merging it.

@arthurzam arthurzam closed this in 70ab010 May 12, 2022
@laumann laumann deleted the check-license branch August 1, 2022 10:03
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.

New check: LICENSE doesn't contain variables

4 participants