-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Update THIRD-PARTY-NOTICES for 8.0 release #91933
Conversation
License for remote stack unwind (https://github.com/llvm/llvm-project/blob/main/lldb/source/Symbol/CompactUnwindInfo.cpp) | ||
-------------------------------------- | ||
|
||
Copyright 2019 LLVM Project |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
llvm is referenced above
License notice for The LLVM Compiler Infrastructure |
Can we fold it into a single entry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could remove the new entry, if old one covers the remote stack unwind
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are using several LLVM components across coreclr and mono; combined. I think one license is enough (since the wordings remain the same), and we are not being specific about other usages of LLVM in runtime either.
cc @jkotas
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LLVM relicensed their sources.
The old sources are available under legacy LLVM License terms. The existing LLVM entry has the legacy LLVM License terms.
The current sources are available under the current LLVM License terms (Apache with LLVM Exception - https://llvm.org/LICENSE.txt).
We have a mix of both sources licensed under the old terms and the new terms, so I think we should keep both old and new notices. We may want to change the section titles to "License notice for The LLVM Compiler Infrastructure (Legacy License)" and "License notice for The LLVM Project" to make it clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with dcda10a
|
||
--------------------------------------------------------- | ||
|
||
Newtonsoft.Json 13.0.1 - MIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is third entry for Newtonsoft.Json. Do we need all three?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with dcda10a
Aspects of Date/Time processing based on algorithm described in "Euclidean Affine Functions and Applications to Calendar | ||
Algorithms", Cassio Neri and Lorenz Schneider. https://arxiv.org/pdf/2102.06959.pdf | ||
|
||
Notice for Library of Congress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this used? I expect that this is going to raise red flags. It does not look like an approved OSS license.
If this is only used in tests, it should be a local 3rd party notice next to the test and omitted in the shipping 3rd party notice file.
(Here is an example of 3rd party notice that is local to the test: https://github.com/dotnet/runtime/blob/main/src/tests/JIT/Performance/CodeQuality/Burgers/THIRD-PARTY-NOTICES)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is coming from runtime
repo's TPN file, and the root cause might be this:
// Euclidean Affine Functions Algorithm (EAF) constants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My comment is for "Notice for Library of Congress"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aah, I see - looking...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed with 71f97eb
This was a test-only notice, I've created a tracking issue in contributing repo: dotnet/winforms#9905
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tell-mode. Approving.
@NikolaMilosavljevic are all the suggestions addressed?
@jeffschwMSFT / @artl93 would you like to give it a final look before I merge it?
@jeffschwMSFT - I don't have context on the license inclusion / changes to comment, so I defer to you if you have more context. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
All suggestions have been addressed. |
@carlossanlop this can be merged now. |
Update of THIRD-PARTY-NOTICES file for 8.0 release.
Generated using infra, command:
build -subset RegenerateThirdPartyNotices
Full list of repos that contribute to .NET THIRD-PARTY-NOTICES.TXT is here: https://github.com/dotnet/runtime/blob/main/eng/regenerate-third-party-notices.proj