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(python): add license fields #217

Merged
merged 7 commits into from
Jun 27, 2023

Conversation

afdesk
Copy link
Contributor

@afdesk afdesk commented May 16, 2023

License field is deprecated and replaced by the new License-Expression field, and there was added License-File field.
Read more here PEP 639 – Improving License Clarity with Better Package .

also there are legacy packages that use license classifiers (Classifier field), so we have to check it too.

@afdesk afdesk marked this pull request as ready for review May 17, 2023 06:15
@afdesk afdesk marked this pull request as draft May 17, 2023 06:28
@afdesk afdesk marked this pull request as ready for review May 17, 2023 06:36
Comment on lines 30 to 42
license := h.Get("License-Expression")
if license == "" {
license = h.Get("License")
}
if license == "" {
for _, classifier := range h.Values("Classifier") {
if strings.HasPrefix(classifier, "License :: ") {
values := strings.Split(classifier, " :: ")
license = values[len(values)-1]
break
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may want to flatten the conditions.

Suggested change
license := h.Get("License-Expression")
if license == "" {
license = h.Get("License")
}
if license == "" {
for _, classifier := range h.Values("Classifier") {
if strings.HasPrefix(classifier, "License :: ") {
values := strings.Split(classifier, " :: ")
license = values[len(values)-1]
break
}
}
}
// "License-Expression" takes precedence as "License" is deprecated.
// cf. https://peps.python.org/pep-0639/#deprecate-license-field
var license string
if l := h.Get("License-Expression"); l != "" {
license = l
else if l := h.Get("License"); l != "" {
license = l
else {
for _, classifier := range h.Values("Classifier") {
if strings.HasPrefix(classifier, "License :: ") {
values := strings.Split(classifier, " :: ")
license = values[len(values)-1]
break
}
}
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, sure

Name: h.Get("Name"),
Version: h.Get("Version"),
License: license,
LicenseFile: h.Get("License-File"),
Copy link
Collaborator

@knqyf263 knqyf263 May 17, 2023

Choose a reason for hiding this comment

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

It could be a bit hacky, but I'm not keen on adding a new field only for python. What if adding file://license.txt to License if the license is empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's try this way.

@afdesk afdesk requested a review from knqyf263 May 18, 2023 07:50
@afdesk
Copy link
Contributor Author

afdesk commented Jun 27, 2023

@knqyf263 could you take a look at this PR again?
@nikpivkin will finish this task.

Copy link
Collaborator

@knqyf263 knqyf263 left a comment

Choose a reason for hiding this comment

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

LGTM!

@knqyf263 knqyf263 merged commit fb7eb31 into aquasecurity:main Jun 27, 2023
rishabh-arya95 added a commit to LambdaTest/go-dep-parser that referenced this pull request Jun 28, 2023
feat(python): add license fields (aquasecurity#217)
Sq34sy pushed a commit to Sq34sy/go-dep-parser that referenced this pull request Jul 28, 2023
Sq34sy pushed a commit to Sq34sy/go-dep-parser that referenced this pull request Jul 28, 2023
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.

None yet

2 participants