Upgrade `spdx-tools` to v0.8.1 #3455 by armintaenzertng · Pull Request #3456 · aboutcode-org/scancode-toolkit

armintaenzertng · 2023-07-17T09:21:00Z

As the spdx-tools library has been refactored, this requires some code adaption of the current output_spdx.py and regeneration of the test files.
Also, the SPDX license list is now handled by the license-expression library, which is why we don't need the corresponding code anymore that adapts this list.

Fixes #3455

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 📁
Looked for possible updates in documentation and added updates if applicable
Updated CHANGELOG.rst

pombredanne

Thanks!
See some comments for your review.

src/formattedcode/output_spdx.py

pombredanne · 2023-07-17T14:53:42Z

tests/formattedcode/data/spdx/license_ref/expected_with_text.tv

 ScanCode includes datasets (e.g. for license detection) that are dedicated
 to the Public Domain using the Creative Commons CC0 1.0 Universal (CC0 1.0)
 Public Domain Dedication: http://creativecommons.org/publicdomain/zero/1.0/</text>
-LicenseID: LicenseRef-scancode-other-permissive


Why change the order? Id, name then comment is the proper order here.

The order in the official spec example and implemented in the serializer is: Id, extracted text, name, cross reference, comment.
So, not different from your proposal?

tests/formattedcode/data/spdx/license_ref/expected_with_text.tv

pombredanne · 2023-07-17T14:55:35Z

tests/formattedcode/data/spdx/license_ref/expected_with_text.tv

-Public Domain Dedication: http://creativecommons.org/publicdomain/zero/1.0/</text>
+Public Domain Dedication: http://creativecommons.org/publicdomain/zero/1.0/</text>
+LicenseName: Other Copyleft Licenses
+LicenseComment: <text>See details at https://github.com/nexB/scancode-toolkit/blob/develop/src/licensedcode/data/licenses/other-copyleft.yml


Same as above. This is incorrect

As you reference the LicenseComment here, I'm not sure I know what you mean.
In case you meant the LicenseName, that corresponds to the Id "LicenseRef-scancode-other-copyleft", which seems fine to me.

pombredanne · 2023-07-17T14:57:38Z

tests/formattedcode/data/spdx/unicode/expected.tv

 * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
-* DAMAGE.</text>
+* DAMAGE.</text>
+LicenseName: Agere BSD


Let's not reorder these

setup-mini.cfg

armintaenzertng · 2023-07-25T10:28:52Z

@pombredanne, I believe I addressed all your comments above (apart from the reordering, for which we will continue to adhere to the specification examples) and implemented them or opened an issue if that problem already existed prior to this PR.
Also, we have officially released spdx-tools v0.8.0, I updated the version in this PR.

The CI currently fails for macos12 only. The failing test test_scan_does_scan_php_composer succeeds for all other platforms and also locally for me. This seems like an issue that's unrelated to this PR.

AyanSinhaMahapatra · 2023-07-25T11:14:56Z

we have officially released spdx-tools v0.8.0, I updated the version in this PR.

Thanks @armintaenzertng !

The CI currently fails for macos12 only. The failing test test_scan_does_scan_php_composer succeeds for all other platforms and also locally for me. This seems like an issue that's unrelated to this PR.

These are flukes we see sometimes, I've reran the tests and all green now.

maxhbr · 2023-07-27T16:01:56Z

Hey @pombredanne and @AyanSinhaMahapatra , just was checking in, if I can help to get it merged. Are there still open concerns?

src/formattedcode/output_spdx.py

maxhbr · 2023-08-23T13:15:45Z

@pombredanne , does #3456 (comment) resolve the issue?

as the library has been refactored, this requires some code adaption of the current output_spdx.py and regeneration of the test files Signed-off-by: Armin Tänzer <[email protected]>

armintaenzertng · 2023-08-24T13:39:45Z

Just a small update: I rebased the code on the current state of the repo and updated the spdx-tools dependency to 0.8.1 (the update has no impact on the code in this PR).

armintaenzertng · 2023-08-29T10:13:40Z

Hi @AyanSinhaMahapatra,
could you please have a look at #3456 (comment) and see if that solves the open issue with this PR?

maxhbr · 2023-10-12T07:12:44Z

hey, any updates?

pombredanne

Thanks... I have added a new nit for your review wrt. the field ordering problem but I am goimng to merge this as-is now.

pombredanne · 2023-10-13T09:19:20Z

tests/formattedcode/data/spdx/license_known/expected.tv

@@ -1,35 +1,36 @@
-# Document Information
+## Document Information


It does not make any sense: ## makes the file bigger for no good reason. The fact that spec examples use this weird way does not make it part of the spec FWIW. The spec states that a # is a comment not that you should use a double ##. Now this is not critical so I can live with this change but this feels like cargo culting.

Also the logical order from most significant to least significant is namespace then name. This is BTW also the order of the spec examples you reference https://github.com/spdx/spdx-spec/blob/development/v2.3.1/examples/SPDXTagExample-v2.3.spdx#L3

I do not care too much for the change so I can live with it but there is no rationale for this change that I can understand.

pombredanne requested changes Jul 17, 2023

View reviewed changes

AyanSinhaMahapatra reviewed Jul 17, 2023

View reviewed changes

setup-mini.cfg Outdated Show resolved Hide resolved

armintaenzertng force-pushed the update-spdx-tools-version branch from 26f26c5 to dd38a70 Compare July 18, 2023 10:07

armintaenzertng mentioned this pull request Jul 18, 2023

Preserve the original order of copyrights in SPDX output #3457

Open

armintaenzertng force-pushed the update-spdx-tools-version branch 2 times, most recently from bb1f70c to 4918b04 Compare July 25, 2023 08:28

pombredanne requested changes Aug 7, 2023

View reviewed changes

src/formattedcode/output_spdx.py Show resolved Hide resolved

pombredanne requested changes Aug 7, 2023

View reviewed changes

src/formattedcode/output_spdx.py Show resolved Hide resolved

tobim mentioned this pull request Aug 10, 2023

python3Packages.spdx-tools: 0.7.1 -> 0.8.0 NixOS/nixpkgs#248286

Merged

12 tasks

armintaenzertng force-pushed the update-spdx-tools-version branch from 4918b04 to a4d0541 Compare August 24, 2023 13:37

upgrade spdx-tools to v0.8

6e8d2f9

as the library has been refactored, this requires some code adaption of the current output_spdx.py and regeneration of the test files Signed-off-by: Armin Tänzer <[email protected]>

armintaenzertng force-pushed the update-spdx-tools-version branch from a4d0541 to 6e8d2f9 Compare August 24, 2023 13:38

fabaff mentioned this pull request Aug 25, 2023

python311Packages.spdx-tools: 0.8.0 -> 0.8.1 NixOS/nixpkgs#251250

Merged

12 tasks

armintaenzertng changed the title ~~Upgrade spdx-tools to v0.8 #3455~~ Upgrade spdx-tools to v0.8.1 #3455 Aug 28, 2023

pombredanne approved these changes Oct 13, 2023

View reviewed changes

pombredanne merged commit f3e52e1 into aboutcode-org:develop Oct 13, 2023

armintaenzertng deleted the update-spdx-tools-version branch October 16, 2023 05:40

		@@ -1,35 +1,36 @@
		# Document Information
		## Document Information

Uh oh!

Conversation

armintaenzertng commented Jul 17, 2023 • edited by AyanSinhaMahapatra Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Tasks

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pombredanne Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

armintaenzertng Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pombredanne Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

armintaenzertng Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

pombredanne Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

armintaenzertng commented Jul 25, 2023

Uh oh!

AyanSinhaMahapatra commented Jul 25, 2023

Uh oh!

maxhbr commented Jul 27, 2023

Uh oh!

Uh oh!

Uh oh!

maxhbr commented Aug 23, 2023

Uh oh!

armintaenzertng commented Aug 24, 2023

Uh oh!

armintaenzertng commented Aug 29, 2023

Uh oh!

maxhbr commented Oct 12, 2023

Uh oh!

pombredanne left a comment

Choose a reason for hiding this comment

Uh oh!

pombredanne Oct 13, 2023 • 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

armintaenzertng commented Jul 17, 2023 •

edited by AyanSinhaMahapatra

Loading

pombredanne Oct 13, 2023 •

edited

Loading