Pip filter phony packages #4184

sschuberth · 2021-06-17T11:20:24Z

Please have a look at the individual commit messages for the details.

sschuberth · 2021-06-17T11:20:41Z

FYI @mmurto.

mmurto · 2021-06-17T11:22:01Z

FYI @mmurto.

I'll test this asap!

fviernau · 2021-06-17T11:31:43Z

analyzer/src/main/kotlin/managers/Pip.kt

@@ -679,7 +679,7 @@ class Pip(
        }

        val declaredLicenses = sortedSetOf<String>()
-        getLicenseFromLicenseField(map["License"]?.single())?.let { declaredLicenses += it }
+        map["License"]?.mapNotNullTo(declaredLicenses) { getLicenseFromLicenseField(it) }


commit: Do you have a reference to the case (package) where this problem appeared?

No. According to @mmurto's report in the chat it should be one of dill Django psycopg2 pymongo python-dateutil pytz PyYAML sqlparse (unclear in which version), but I cannot spend the time to find out exactly now.

I was just wondering whether the issue could be a bug in lines 660 - 678. Maybe we just wait for @mmurto 's test results.

Here's @mmurto's feedback from the chat:

Found the probable culprit: one of the packages has the whole license text of MIT in setup.py's setup() functions license field 🙂

So given it's a clear misuse of the "License" field, should I drop this change again?

Or maybe we should not fail hard but only warn if the "License" contains multiple fields?

I've added a commit. Please have a look @fviernau. And @mmurto you're welcome to test once more 😉

Analyzer succeeds, but declared licenses contain it all:

declared_licenses: - "AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER" - <redacted> - "FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL\ \ THE" - "IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY," - "LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING\ \ FROM," - "MIT License" - "OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS\ \ IN" - "Permission is hereby granted, free of charge, to any person obtaining a\ \ copy" - "THE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS\ \ OR" - "THE SOFTWARE." - "The MIT License" - "The above copyright notice and this permission notice shall be included\ \ in" - "all copies or substantial portions of the Software." - "copies of the Software, and to permit persons to whom the Software is" - "furnished to do so, subject to the following conditions:" - "in the Software without restriction, including without limitation the rights" - "of this software and associated documentation files (the \"Software\"),\ \ to deal" - "to use, copy, modify, merge, publish, distribute, sublicense, and/or sell" declared_licenses_processed: spdx_expression: "MIT" mapped: MIT License: "MIT" The MIT License: "MIT" unmapped: - "AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER" - <redacted> - "FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL\ \ THE" - "IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY," - "LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING\ \ FROM," - "OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS\ \ IN" - "Permission is hereby granted, free of charge, to any person obtaining\ \ a copy" - "THE SOFTWARE IS PROVIDED \"AS IS\", WITHOUT WARRANTY OF ANY KIND, EXPRESS\ \ OR" - "THE SOFTWARE." - "The above copyright notice and this permission notice shall be included\ \ in" - "all copies or substantial portions of the Software." - "copies of the Software, and to permit persons to whom the Software is" - "furnished to do so, subject to the following conditions:" - "in the Software without restriction, including without limitation the\ \ rights" - "of this software and associated documentation files (the \"Software\"\ ), to deal" - "to use, copy, modify, merge, publish, distribute, sublicense, and/or\ \ sell"

The analyzer run succeeded with the following result:

- id: "PIP::<redacted>:5743024ffd8a42b806a13060104d861146d4e5ba" definition_file_path: "<redacted>/requirements.txt" authors: - "<redacted>" declared_licenses: - "\nThe MIT License \nCopyright (c) 2021- <redacted>\

@mmurto, is the "<redacted>/requirements.txt" part in here really correct? Shouldn't it be "<redacted>/setup.py"?

Because above you wrote

the packages has the whole license text of MIT in setup.py

and requirements.txt does not actually support declaring a license AFAIK.

The analyzer run succeeded with the following result:

- id: "PIP::<redacted>:5743024ffd8a42b806a13060104d861146d4e5ba" definition_file_path: "<redacted>/requirements.txt" authors: - "<redacted>" declared_licenses: - "\nThe MIT License \nCopyright (c) 2021- <redacted>\

@mmurto, is the "<redacted>/requirements.txt" part in here really correct? Shouldn't it be "<redacted>/setup.py"?

Because above you wrote

the packages has the whole license text of MIT in setup.py

and requirements.txt does not actually support declaring a license AFAIK.

The part with requirements.txt is correct, the project's requirements.txt contains only ., which leads to pip getting the dependencies from setup.py, some info here.

The part with requirements.txt is correct, the project's requirements.txt contains only ., which leads to pip getting the dependencies from setup.py, some info here.

Wow, subtle!

analyzer/src/main/kotlin/managers/Pip.kt

So far phony packages have only been filtered out from the dependency tree, but they should also be removed from the list of installed packages. Signed-off-by: Sebastian Schuberth <[email protected]>

There was a case reported where the "single()" failed due to multiple licenses. While that might have been an issue with "pip", simply support that case as it is easy to do so, like we already do it for the classifiers one line below. Signed-off-by: Sebastian Schuberth <[email protected]>

There are projects which put the full license text instead of just the license name into a license field. Omit such texts from the list of declared licenses by only accepting licenses that do not contain a newline character. Signed-off-by: Sebastian Schuberth <[email protected]>

The field is specified to be a "short string" which is "a single line of text, not more than 200 characters" [1]. Respect that limit, which also filters out cases where people add full license texts to the field. [1] https://docs.python.org/3/distutils/setupscript.html#additional-meta-data Signed-off-by: Sebastian Schuberth <[email protected]>

sschuberth · 2022-05-04T13:02:32Z

Superseded by #5319.

sschuberth requested a review from a team as a code owner June 17, 2021 11:20

fviernau previously approved these changes Jun 17, 2021

View reviewed changes

sschuberth dismissed fviernau’s stale review via 996479a June 17, 2021 13:00

fviernau previously approved these changes Jun 17, 2021

View reviewed changes

analyzer/src/main/kotlin/managers/Pip.kt Show resolved Hide resolved

sschuberth added 3 commits July 9, 2021 12:28

Pip: Filter out phony packages from the list of installed packages

29dbf80

So far phony packages have only been filtered out from the dependency tree, but they should also be removed from the list of installed packages. Signed-off-by: Sebastian Schuberth <[email protected]>

sschuberth dismissed fviernau’s stale review via ee75aac July 9, 2021 10:29

sschuberth force-pushed the pip-filter-phony-packages branch from 996479a to ee75aac Compare July 9, 2021 10:29

sschuberth requested review from fviernau and a team July 9, 2021 11:00

sschuberth enabled auto-merge (rebase) July 9, 2021 11:00

sschuberth force-pushed the pip-filter-phony-packages branch from 8940a6f to 8c7b86e Compare July 9, 2021 11:06

sschuberth closed this May 4, 2022

auto-merge was automatically disabled May 4, 2022 13:02
Pull request was closed

sschuberth deleted the pip-filter-phony-packages branch May 4, 2022 13:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Pip filter phony packages #4184

Pip filter phony packages #4184

Uh oh!

sschuberth commented Jun 17, 2021

Uh oh!

sschuberth commented Jun 17, 2021

Uh oh!

mmurto commented Jun 17, 2021

Uh oh!

fviernau Jun 17, 2021

Uh oh!

sschuberth Jun 17, 2021

Uh oh!

fviernau Jun 17, 2021 •

edited

Loading

Uh oh!

sschuberth Jun 17, 2021 •

edited

Loading

Uh oh!

sschuberth Jun 17, 2021

Uh oh!

sschuberth Jun 17, 2021

Uh oh!

mmurto Jun 17, 2021 •

edited

Loading

Uh oh!

sschuberth Jul 9, 2021

Uh oh!

mmurto Jul 9, 2021

Uh oh!

sschuberth Jul 9, 2021

Uh oh!

Uh oh!

sschuberth commented May 4, 2022

Uh oh!

Uh oh!

Pip filter phony packages #4184

Pip filter phony packages #4184

Uh oh!

Conversation

sschuberth commented Jun 17, 2021

Uh oh!

sschuberth commented Jun 17, 2021

Uh oh!

mmurto commented Jun 17, 2021

Uh oh!

fviernau Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

sschuberth Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

fviernau Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sschuberth Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sschuberth Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

sschuberth Jun 17, 2021

Choose a reason for hiding this comment

Uh oh!

mmurto Jun 17, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sschuberth Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

mmurto Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

sschuberth Jul 9, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sschuberth commented May 4, 2022

Uh oh!

Uh oh!

fviernau Jun 17, 2021 •

edited

Loading

sschuberth Jun 17, 2021 •

edited

Loading

mmurto Jun 17, 2021 •

edited

Loading