tonic icon indicating copy to clipboard operation
tonic copied to clipboard

Package Apache-2.0 license properly

Open LecrisUT opened this issue 1 year ago • 12 comments

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.

LecrisUT avatar Sep 19 '24 11:09 LecrisUT

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

djc avatar Sep 19 '24 11:09 djc

I have edited the top-comment

LecrisUT avatar Sep 19 '24 12:09 LecrisUT

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?

djc avatar Sep 19 '24 14:09 djc

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.

LecrisUT avatar Sep 19 '24 14:09 LecrisUT

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.

djc avatar Sep 20 '24 08:09 djc

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 avatar Sep 20 '24 09:09 LecrisUT

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.

LecrisUT avatar Sep 20 '24 12:09 LecrisUT

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 avatar Sep 23 '24 13:09 djc

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.

djc avatar Sep 25 '24 08:09 djc

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?

  • 251aba414fef953e0db283a60e34c06c9501ee96 removes license and metadata form codegen and interop, and removes metadata from examples keeping the license info
  • 017730450a5a61a91e9cf5d83fcd1ccffe845e66 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
  • 79dc612160c369260cb873c3b275d95a37a67554 removes benches-disabled therefore reducing the necessary license information
  • 7d78aecf6370248bbb4006e0eb1a0c3f07aa6805 removes license and metadata of all test crates

LecrisUT avatar Sep 25 '24 08:09 LecrisUT

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.

djc avatar Sep 25 '24 08:09 djc

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

LecrisUT avatar Sep 25 '24 08:09 LecrisUT

I am going to close this for now, but I would accept a version of this change that is updated. Closing for now, feel free to re-open.

LucioFranco avatar Jun 20 '25 15:06 LucioFranco

I don't think there was any open issues for this one, a simple rebase should also fix the CI issues?

LecrisUT avatar Jun 20 '25 15:06 LecrisUT