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

AVRO-3936: Clean up NOTICE and LICENSE file #3245

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CalvinKirs
Copy link
Member

@CalvinKirs CalvinKirs commented Nov 20, 2024

Contribution Checklist

  • Make sure that the pull request corresponds to a JIRA issue. Exceptions are made for typos in JavaDoc or documentation files, which need no JIRA issue.

  • Name the pull request in the form "AVRO-XXXX: [component] Title of the pull request", where AVRO-XXXX should be replaced by the actual issue number.
    The component is optional, but can help identify the correct reviewers faster: either the language ("java", "python") or subsystem such as "build" or "doc" are good candidates.

https://issues.apache.org/jira/browse/AVRO-1769 The implementation of Jansson has been removed.
apache#307  The implementation of nunit.framework.dll,Newtonsoft.Json.dll and Castle.Core.dll has been removed.
apache#2756 The implementation of m4 macros has been removed.
apache@8a42cd0 Boost.hhThe implementation of Boost.hh has been removed.
@KalleOlaviNiemitalo
Copy link
Contributor

Newtonsoft.Json.dll is still being distributed within the Apache.Avro.Tools NuGet package. Perhaps that package should have a separate NOTICE file, because it includes third-party files that are not in the Apache.Avro package.

@CalvinKirs
Copy link
Member Author

Newtonsoft.Json.dll is still being distributed within the Apache.Avro.Tools NuGet package. Perhaps that package should have a separate NOTICE file, because it includes third-party files that are not in the Apache.Avro package.↳

oops, could you clarify exactly which file you're referring to? I couldn't seem to find this package.

@KalleOlaviNiemitalo
Copy link
Contributor

From https://www.nuget.org/packages/Apache.Avro.Tools/1.12.0, click Download package, and use unzip -l apache.avro.tools.1.12.0.nupkg; it shows tools/net8.0/any/Newtonsoft.Json.dll, and similar files in other directories.

Or if your Web browser supports WebAssembly, then click Open in NuGet Package Explorer, wait until it has loaded the package, and navigate to tools > net80 > any in the "Contents" pane at the top right; it shows a Newtonsoft.Json.dll file there.

The LICENSE file in that package includes the Json.NET license notice. Will that one still be there after this pull request?

@KalleOlaviNiemitalo
Copy link
Contributor

It looks like the LICENSE file in the Apache.Avro.Tools NuGet package comes from lang/csharp/LICENSE, which is not modified in this pull request. So, the Json.NET license notice will remain in that package.

lang/csharp/LICENSE also includes legal notices for Castle Core and log4net. I don't know whether those are still relevant.

@CalvinKirs
Copy link
Member Author

It looks like the LICENSE file in the Apache.Avro.Tools NuGet package comes from lang/csharp/LICENSE, which is not modified in this pull request. So, the Json.NET license notice will remain in that package.↳

lang/csharp/LICENSE also includes legal notices for Castle Core and log4net. I don't know whether those are still relevant.↳

I'm not familiar with C#, but this appears to be a binary package (which includes DLL files). Where does its source code come from? Also, this change only involves the LICENSE file in the source package.

@KalleOlaviNiemitalo
Copy link
Contributor

lang/csharp/src/apache/codegen/Avro.codegen.csproj is the C# project file for the Apache.Avro.Tools NuGet package. It gets the Newtonsoft.Json.dll files from the Newtonsoft.Json NuGet package, instead of building them from source.

The changes in this pull request look all right to me, but it might be appropriate to also remove the Castle Core and log4net notices from lang/csharp/LICENSE, if Avro no longer references those libraries.

In contrast, the Newtonsoft.Json notices should not be removed from lang/csharp/LICENSE. And this PR does not remove them.

@CalvinKirs
Copy link
Member Author

lang/csharp/src/apache/codegen/Avro.codegen.csproj is the C# project file for the Apache.Avro.Tools NuGet package. It gets the Newtonsoft.Json.dll files from the Newtonsoft.Json NuGet package, instead of building them from source.↳

The changes in this pull request look all right to me, but it might be appropriate to also remove the Castle Core and log4net notices from lang/csharp/LICENSE, if Avro no longer references those libraries.↳

In contrast, the Newtonsoft.Json notices should not be removed from lang/csharp/LICENSE. And this PR does not remove them.

This tool is something we released? It seems to lack a NOTICE file, and the binary distribution still needs to comply with the AL2 requirements. I'm not familiar with C#, so I'm unable to determine whether the LICENSE file is correct, specifically if it includes the licenses for the bundled components.

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.

2 participants