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

Package Apache-2.0 license properly #1946

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

LecrisUT
Copy link

@LecrisUT LecrisUT commented Sep 19, 2024

Motivation

This PR tries to address licensing issue in the Cargo.toml metadata. Both the source files and the generated files contain Apache-2.0 licensed files, and as such the Cargo.toml file should reflect that. This is not a issue because prost is already licensed as Apache-2.0 and as such any dependency would have to comply with MIT AND Apache-2.0 implicitly.

A more nuanced file are the .bin files which further includes BSD-3-Clause metadata. To me it is unclear what the license under Cargo.toml is meant to reflect and if it would include the generated artifacts at the end as well, and it is also ambiguous who should be carrying the additional BSD-3-Clause license information (tonic, prost or end-user?). This part is not addressed here.

Solution

  • Add the Apache-2.0 license file and metadata

License breakdown for the files that require the additional Apache-2.0 metadata:

  • examples: examples/proto
  • interop: interop/proto/grpc/testing
  • tonic: tonic/benches-disabled/proto/helloworld
  • tonic-health: tonic-health/proto
  • tonic-reflection: tonic-reflection/proto,
  • tonic-types: tonic-types/proto

For reference I searched for Apache License as well as the spdx form Apache-2.0.

Additionally all of the src/generated/*.bin files have the Apache-2.0 metadata in them, as well as the BSD-3-Clause

Extracted from types.bin
Protocol Buffers - Google's data interchange format
 Copyright 2008 Google Inc.  All rights reserved.
 https://developers.google.com/protocol-buffers/

 Redistribution and use in source and binary forms, with or without
 modification, are permitted provided that the following conditions are
 met:

     * Redistributions of source code must retain the above copyright
 notice, this list of conditions and the following disclaimer.
     * Redistributions in binary form must reproduce the above
 copyright notice, this list of conditions and the following disclaimer
 in the documentation and/or other materials provided with the
 distribution.
     * Neither the name of Google Inc. nor the names of its
 contributors may be used to endorse or promote products derived from
 this software without specific prior written permission.

 THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
 "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
 LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
 A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
 OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
 SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
 LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
 DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
 THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
 OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

For reference: https://spdx.org/licenses/BSD-3-Clause.html

PS: the Apache-2.0 copyright should be updated to include tonic as the copyright holder, unless it is not meant to be altered from upstream.

@djc
Copy link
Contributor

djc commented Sep 19, 2024

Would appreciate some hints in the OP or PR comments that point out for each crate why this is necessary.

@LecrisUT
Copy link
Author

I have edited the top-comment

@djc
Copy link
Contributor

djc commented Sep 19, 2024

For examples and interop, why do you care? Presumably you're not going to package these, nor is anyone distributing them. For tonic itself, IMO we should not encumber the published crate with another license only for a disabled benchmark -- could we exclude any files from that benchmark from the distribution?

@LecrisUT
Copy link
Author

For examples and interop, why do you care? Presumably you're not going to package these, nor is anyone distributing them.

In practice no, it is not necessary, I just followed that in the current version there is metadata included around it. Was also considering just stripping down the Cargo.toml there, so up to you how you want to deal with those.

For tonic itself, IMO we should not encumber the published crate with another license only for a disabled benchmark -- could we exclude any files from that benchmark from the distribution?

Feel free to strip down as much as possible for distribution. We want to avoid having benches in downstream packaging as well, and personally it seems to make more sense to run the benches on a git repo rather than a packaged crate, but I am not familiar enough with the ecosystem to know why the benches are generally packaged.

@djc
Copy link
Contributor

djc commented Sep 20, 2024

For examples and interop, why do you care? Presumably you're not going to package these, nor is anyone distributing them.

In practice no, it is not necessary, I just followed that in the current version there is metadata included around it. Was also considering just stripping down the Cargo.toml there, so up to you how you want to deal with those.

Stripping metadata for these crates and adding publish = false for them seems like the right way forward.

For tonic itself, IMO we should not encumber the published crate with another license only for a disabled benchmark -- could we exclude any files from that benchmark from the distribution?

Feel free to strip down as much as possible for distribution. We want to avoid having benches in downstream packaging as well, and personally it seems to make more sense to run the benches on a git repo rather than a packaged crate, but I am not familiar enough with the ecosystem to know why the benches are generally packaged.

Suggest just adding exclude = ["/benches/*"] to the tonic/Cargo.toml.

@LecrisUT
Copy link
Author

Stripping metadata for these crates and adding publish = false for them seems like the right way forward.

Done.

Suggest just adding exclude = ["/benches/*"] to the tonic/Cargo.toml.

Needed to use exclude = ["/benches*"] in order to capture the benches-disabled that we actually need to exclude, but I am not sure if that was the intent you were looking for?

@LecrisUT
Copy link
Author

Regarding CI/Check MSRV it seems that the rust version is <1.75 when the package.version was made optional 1. Would you want to keep the CI action at 1.71.1?

For cargo-deny I have figured how to fix it at least.

Footnotes

  1. https://doc.rust-lang.org/cargo/reference/manifest.html#the-version-field

deny.toml Show resolved Hide resolved
codegen/Cargo.toml Show resolved Hide resolved
tonic/Cargo.toml Outdated Show resolved Hide resolved
@LecrisUT LecrisUT force-pushed the packaging/license branch 2 times, most recently from ce890c8 to 9c74f03 Compare September 23, 2024 09:49
@djc
Copy link
Contributor

djc commented Sep 23, 2024

Suggest you change the order of commits here to first remove metadata from non-published crates (except the license in any examples crates) then add a commit to add licenses to the published crates where necessary.

@djc
Copy link
Contributor

djc commented Sep 25, 2024

Suggest you change the order of commits here to first remove metadata from non-published crates (except the license in any examples crates) then add a commit to add licenses to the published crates where necessary.

You still have the second commit adding back a bunch of license stuff to crates from which you removed license stuff in the first commit. This does not make sense. Please clean up the commit history so that each commit can be reviewed separately.

@LecrisUT
Copy link
Author

You still have the second commit adding back a bunch of license stuff to crates from which you removed license stuff in the first commit.

Which one are you referring to?

  • 251aba4 removes license and metadata form codegen and interop, and removes metadata from examples keeping the license info
  • 0177304 adjusts the license info of exmaples, tonic, tonic-build, tonic-health, tonic-reflection, toinc-types, tonic-web. The workspace comments that tonic-web is unpublished, but the crate exists
  • 79dc612 removes benches-disabled therefore reducing the necessary license information
  • 7d78aec removes license and metadata of all test crates

@djc
Copy link
Contributor

djc commented Sep 25, 2024

Okay, but I had asked you to do this:

Suggest you change the order of commits here to first remove metadata from non-published crates (except the license in any examples crates) then add a commit to add licenses to the published crates where necessary.

In the current setup, you're adding metadata in the second commit ("Indicate Apache-2.0 license accordingly") and then reverting a bunch of that change in the commits for benches-disabled and test crates.

@LecrisUT
Copy link
Author

Ok, didn't know you wanted that for tonic crate as well. What about now?

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