Separate authors and copyright holders

[porsche-rbieniek]

In German law, the author and the copyright holder can be two seperate legal entities and therefore also need to be treated separately.

Introduce a new copyright holder field that is now the primary source for copyright holder information. Authors are still only used as copyright holders if the addAuthorsToCopyrights option is enabled.

For now, all package manager implementations set empty copyright holders. Filling the copyright holder field is left as an exercise for future actions. Right now, the only way to add copyright holders is via curations.

This change resolves https://github.com/oss-review-toolkit/ort/issues/4519.

Signed-off-by: Rainer Bieniek [email protected]

[codecov[bot]]

Codecov Report

Merging #5504 (455579c) into main (285f018) will decrease coverage by 3.00%. The diff coverage is 66.66%.

:exclamation: Current head 455579c differs from pull request most recent head 86672b1. Consider uploading reports for the commit 86672b1 to get more accurate results

@@             Coverage Diff              @@
##               main    #5504      +/-   ##
============================================
- Coverage     72.70%   69.70%   -3.01%     
- Complexity     2045     2192     +147     
============================================
  Files           267      268       +1     
  Lines         14124    14897     +773     
  Branches       2109     2412     +303     
============================================
+ Hits          10269    10384     +115     
- Misses         2773     3427     +654     
- Partials       1082     1086       +4     

Flag	Coverage Δ
funTest-analyzer-docker	73.43% <94.59%> (+1.96%)	:arrow_up:
funTest-non-analyzer	47.09% <0.00%> (-0.15%)	:arrow_down:
test	32.60% <12.28%> (-0.07%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...el/src/main/kotlin/licenses/LicenseInfoResolver.kt	84.93% <0.00%> (-6.62%)	:arrow_down:
...rc/main/kotlin/experimental/ExperimentalScanner.kt	53.16% <0.00%> (-0.31%)	:arrow_down:
analyzer/src/main/kotlin/managers/Pip.kt	55.27% <50.00%> (-16.16%)	:arrow_down:
...main/kotlin/licenses/DefaultLicenseInfoProvider.kt	76.66% <66.66%> (-0.53%)	:arrow_down:
analyzer/src/main/kotlin/managers/Carthage.kt	56.52% <75.00%> (-17.43%)	:arrow_down:
analyzer/src/main/kotlin/managers/Bower.kt	67.76% <100.00%> (-20.07%)	:arrow_down:
analyzer/src/main/kotlin/managers/Bundler.kt	56.64% <100.00%> (-15.59%)	:arrow_down:
analyzer/src/main/kotlin/managers/Cargo.kt	73.80% <100.00%> (-13.34%)	:arrow_down:
analyzer/src/main/kotlin/managers/CocoaPods.kt	76.38% <100.00%> (-16.72%)	:arrow_down:
analyzer/src/main/kotlin/managers/Composer.kt	66.48% <100.00%> (-10.59%)	:arrow_down:
... and 37 more

---

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 285f018...86672b1. Read the comment docs.

[sschuberth]

@porsche-rbieniek please have a look at the detekt issues.

[sschuberth]

@porsche-rbieniek now the DCO checks fails because of a mismatch between the author and sign-off names:

[porsche-rbieniek]

So many small details to get it right. I was really wondering why my sign-off's were failing

[sschuberth]

@porsche-rbieniek Something's got stuck with the GitHub actions apparently... please rebase once more onto origin/main and force-push to retrigger the checks.

[sschuberth]

... and please add "Resolves #4519." to the commit message, too.

[porsche-rbieniek]

Changes implemented except CarthageTest. Here I would like to keep the test cverage and augment it by testing the authors as well.

[porsche-rbieniek]

Hi guys,

could you please gimme a hint why this seems to be stuck now?

IMHO I implemented the changes requested to move forward

[fviernau]

@porsche-rbieniek will you make the changes according to the comments so that we can move it forward?

[porsche-rbieniek]

Done all requested changes, we can proceed

[sschuberth]

Dismissing approval after July 14th ORT developer meeting discussion, additional changes to this PR are needed.

While I couldn't join today's developer meeting, I believe we shouldn't try to do too many things at once. IMO the intermediate step, as done in this PR, should be to introduce copyrightHolders analog to authors, except that copyrightHolders for now can exclusively set by curations. All other changes could be done incrementally on top of that; it has already taken us too much time to get this PR into its current shape.

[fviernau]

While I couldn't join today's developer meeting, I believe we shouldn't try to do too many things at once. IMO the intermediate step, as done in this PR, should be to introduce copyrightHolders analog to authors, except that copyrightHolders for now can exclusively set by curations. All other changes could be done incrementally on top of that; it has already taken us too much time to get this PR into its current shape.

As far as I understand this statement tries to argue against the changes agreed on in the developer meeting. Maybe it's worth mentioning these here, so that we are sure to be on the same page. Do you know which these are @sschuberth ?

[sschuberth]

Do you know which these are @sschuberth ?

I didn't yet have time to read through the meeting minutes, if that's what you mean. It seems like use-cases I've never heard of before popped up, and we're back to square one 😵

[fviernau]

Do you know which these are @sschuberth ?

I didn't yet have time to read through the meeting minutes, if that's what you mean. It seems like use-cases I've never heard of before popped up, and we're back to square one dizzy_face

That's a misunderstanding on your side then. The agreement today was that only one trivial change needs to be added to the PR, which is renaming the introduced property IIRC from copyrightHolders to something like declaredCopyrights. Not sure about the exact word, but it basically should align with the name of declaredLicenses.

[porsche-rishisaxena]

Hi @fviernau

I wanted to clarify on the following:

1. There was no agreement in the developers meeting. There was an expectation set from the reviewers in this case i.e., you and Thomas who proposed by enhancing the implementation with following cases: 1.1 Implement an ability to add and remove the copyright (c) 1.2 Implement separation of copyright (c) with Authors in the same way as license is being handled 1.3 If the reporter provides copyright (c) for example: "a","b","c" and manually "d" has been identified by the analyst, the curation should only consist of "d" and not "a","b","c", and "d"

2. Thomas requested to change the property name from copyright_holders to declared_copyright_holders and he proposed to change it by himself before merging.

3. @mnonnenmacher even proposed to take this pull request to be merged and later we bring the enhancements after creating an issue in the backlog.

4. Next Steps: we all aligned that we need a separate workshop to brain-storm to have Sebastian, Thomas, Rainer, Frank, Martin and myself to discuss and conclude. Thomas to set up this meeting.

I hope this clarifies.

Regards, Rishi Saxena

CC: @porsche-rbieniek @sschuberth @tsteenbe @nikpete

[fviernau]

Hi @fviernau

I wanted to clarify on the following:

1. There was no agreement in the developers meeting. There was an expectation set from the reviewers in this case i.e., you and Thomas who proposed by enhancing the implementation with following cases:
   1.1 Implement an ability to add and remove the copyright (c)
   1.2 Implement separation of copyright (c) with Authors in the same way as license is being handled
   1.3 If the reporter provides copyright (c) for example: "a","b","c" and manually "d" has been identified by the analyst, the curation should only consist of "d" and not "a","b","c", and "d"

2. Thomas requested to change the property name from copyright_holders to declared_copyright_holders and he proposed to change it by himself before merging.

3. @mnonnenmacher even proposed to take this pull request to be merged and later we bring the enhancements after creating an issue in the backlog.

4. Next Steps: we all aligned that we need a separate workshop to brain-storm to have Sebastian, Thomas, Rainer, Frank, Martin and myself to discuss and conclude. Thomas to set up this meeting.

I hope this clarifies.

Regards, Rishi Saxena

CC: @porsche-rbieniek @sschuberth @tsteenbe @nikpete

@porsche-rishisaxena

Hm, so it seems things have been perceived differently, e.g. in your view "There was no agreement in the developers meeting" . In my view, e.g. @mnonnenmacher requested to do #2 as part of the PR, which would contradict your point #2 and #3. To my understanding #1 and #4 was discussed as future improvements (and out of scope of the PR), e.g. we've discussed on a conceptual level how we envision the configuration to work in the future.

Anyhow, what problem are you trying to solve @porsche-rishisaxena? Find an agreement how to move on with this PR?

[porsche-rbieniek]

Could somebody please explain if there are now further changes required by me or not?

I couildn't join the developer meeting due to a vacation on short notice and I am now totally lost in this discussion

[sschuberth]

Could somebody please explain if there are now further changes required by me or not?

@porsche-rbieniek, let me take over this PR of yours, I'll get it merged. So please don't push to the branch anymore.

[tsteenbe]

@sschuberth Please don't merge it without my review, I was planning to update this PR today to be aligned with what was discussed in the ORT meeting.

[mnonnenmacher]

@sschuberth and @tsteenbe, since your are taking over the PR, to me the most important point is consistency in the handling of copyrights and licenses. Both can be declared in the metadata and detected in the source code, so the properties should use similar naming and the curation options should work in a similar way.

• Similar to declaredLicense I would prefer the name declaredCopyrights over copyrightHolders in Package and Project.
• I prefer copyrights over copyrightHolders because the detected values are statements like "(C) 2020 xyz", but "holder" sounds to me like it should only be "xyz" for this example. And likely the values in the metadata are also similar in structure as the values we get from ScanCode. See for example "© Microsoft Corporation. All rights reserved." here: https://api.nuget.org/v3/catalog0/data/2018.12.13.22.08.00/system.globalization.4.3.0.json (Btw, who will actually do the work and go through all package managers and check which have dedicated fields for copyright information?)
• I don't know if it's currently possible to add declared licenses with a curation, but if this is implemented for copyrights it should work for licenses and copyrights in a similar way.

[tsteenbe]

@mnonnenmacher I looked up if the most used package manager have a declared copyright holder filed and they don't.

• Cargo https://doc.rust-lang.org/cargo/reference/manifest.html
  • authors — The authors of the package.
  • license — The package license.
  • license-file — Path to the text of the license.
• Composer: https://getcomposer.org/doc/04-schema.md
  • license
  • authors
• Maven: https://maven.apache.org/pom.html#More_Project_Information
  • license
  • organization
  • developer
  • contributor
• NPM:
  • license: SPDX license id https://docs.npmjs.com/cli/v8/configuring-npm/package-json#license
  • author: one person https://docs.npmjs.com/cli/v8/configuring-npm/package-json#people-fields-author-contributors
  • contributor: array of people
• Python: https://docs.python.org/3/distutils/setupscript.html#additional-meta-data
  • author
  • author_email
  • maintainer
  • maintainer_email
  • license

[sschuberth]

Superseded by https://github.com/oss-review-toolkit/ort/pull/5680.