BLD: create a LICENSE_BUNDLED.txt file from third_party licenses by mattip · Pull Request #50745 · pytorch/pytorch

mattip · 2021-01-19T18:00:11Z

Rather than maintain a LICENSE_BUNDLED.txt by hand, this build it out of the subrepos.

I ~~copied and adapted the sdist handling from Numpy~~ added a separate file, so the LICENSE.txt file of the repo remains in pristine condition and the GitHub website still recognizes it. If we modify the file, the website will no longer recognize the license.

This is not enough, since the license in the ~~wheel~~ wheel and sdist is not modified. Numpy has a separate step when preparing the wheel to concatenate the licenses. I am not sure where/if the conda-forge numpy-feedstock also fixes up the license.

~~Should~~ I ~~commit~~ commited the artifact to the repo and ~~add~~ added a test that reproducing the file is consistent.

Edit: now the file is part of the repo.

Edit: rework the mention of sdist. After this is merged another PR is needed to make the sdist and wheel ship the proper merged license.

mattip · 2021-01-19T18:02:37Z

Here is the file the new command creates

LICENSES_BUNDLED.txt

Edit: link to latest version of the file

mattip · 2021-01-19T18:28:03Z

There are a few quirks in the file. There are some third_party projects PyTorch included multiple times (benchmark has 4 copies, clog has 3, googletest has 10, there are more), I included each one separately since they are not all on the same version. Most of the licenses are copies of the original BSD/MIT/Boost/Apache, except:

neon2sse removes one of the BSD clause "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." Strange, and it makes the license unapproved. uses a BSD-Source-Code license that I had not seen before.
nccl removes the phrase "AND CONTRIBUTORS" from the last clause's "BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS ``AS IS'', which ~~also makes the license non-compliant.~~ is a bit different but is fine.
fmt adds an additional exception after the license.
eigen offers a numer of license files COPYING*. I chose the BSD one.

mattip · 2021-01-21T16:28:53Z

@rgommers do you have an idea how/where to merge the LICENSE files when generating a wheel? I guess I could expand the bdist_wheel command like this does by wrapping the sdist command?

codecov · 2021-01-21T18:11:47Z

Codecov Report

Merging #50745 (6f77914) into master (a469336) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #50745      +/-   ##
==========================================
- Coverage   80.65%   80.65%   -0.01%     
==========================================
  Files        1912     1912              
  Lines      208048   208048              
==========================================
- Hits       167810   167808       -2     
- Misses      40238    40240       +2

walterddr

do we need to modify the NOTICE file too since it also contains paragraphs discussing thrid-party license

mattip · 2021-01-21T19:36:37Z

do we need to modify the NOTICE file too since it also contains paragraphs discussing third-party license

Each snippet taken from somewhere else and merged into the pytorch sources (not the third party directory) should be attributed in the source. I think another file should be used to maintain the required attributions for such code. Is that the NOTICE file? The point of the current PR is to add attributions for third party libraries in the third_parties directory which are maintained separately from pytorch itself. If things are missing from NOTICES maybe it can be fixed in a different PR?

rgommers

Overall looks good, thanks @mattip. Main comment on dealing with sdist/wheels in other PRs.

mattip · 2021-01-26T07:36:03Z

Rebased and squashed. Ready for review, assuming CI passes.

facebook-github-bot

@walterddr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

walterddr

looks good to me. Thanks for the contribution @mattip!

walterddr · 2021-01-25T18:30:22Z

I dont know if we have a guidance but was wondering if we should put this in tools/ or scripts/ instead of third_party/. CC @seemethere @malfet

No preference, but it would be good to have a shebang at the head of the file

malfet · 2021-01-26T22:12:12Z

No preference, but it would be good to have a shebang at the head of the file

Co-authored-by: Nikita Shulga <nikita.shulga@gmail.com>

facebook-github-bot · 2021-01-26T22:59:08Z

@walterddr merged this pull request in 24eab1d.

This script was hanging up on a couple of licenses not being parsed correctly / not accounted for in the [original pass through](#50745) Should resolve issues with tests not running 100% correctly. (originally reported in #76128) Also adds an option to run the script and specify a different output file: * CLI Argument `--out-file` * Environment Variable: `PYTORCH_THIRD_PARTY_BUNDLED_LICENSE_FILE` Signed-off-by: Eli Uriegas <eliuriegasfb.com> [ghstack-poisoned]

…orch#50745) Summary: Fixes pytorch#50695. Rather than maintain a LICENSE_BUNDLED.txt by hand, this build it out of the subrepos. I ~copied and adapted the sdist handling from Numpy~ added a separate file, so the LICENSE.txt file of the repo remains in pristine condition and the GitHub website still recognizes it. If we modify the file, the website will no longer recognize the license. This is not enough, since the license in the ~wheel~ wheel and sdist is not modified. Numpy has a [separate step](https://github.com/MacPython/numpy-wheels/blob/master/patch_code.sh) when preparing the wheel to concatenate the licenses. I am not sure where/if the [conda-forge numpy-feedstock](https://github.com/conda-forge/numpy-feedstock/) also fixes up the license. ~Should~ I ~commit~ commited the artifact to the repo and ~add~ added a test that reproducing the file is consistent. Edit: now the file is part of the repo. Edit: rework the mention of sdist. After this is merged another PR is needed to make the sdist and wheel ship the proper merged license. Pull Request resolved: pytorch#50745 Reviewed By: seemethere, heitorschueroff Differential Revision: D26074974 Pulled By: walterddr fbshipit-source-id: bacd5d6870e9dbb419a31a3e3d2fdde286ff2c94

facebook-github-bot added the cla signed label Jan 19, 2021

pytorchbot added the open source label Jan 19, 2021

seemethere self-requested a review January 19, 2021 18:51

seemethere added this to the 1.8.0 milestone Jan 19, 2021

seemethere added the module: build Build system issues label Jan 19, 2021

isuruf reviewed Jan 21, 2021

View reviewed changes

Comment thread third_party/LICENSES_BUNDLED.txt Outdated

walterddr reviewed Jan 21, 2021

View reviewed changes

Comment thread third_party/LICENSES_BUNDLED.txt Outdated

rgommers reviewed Jan 21, 2021

View reviewed changes

Comment thread third_party/LICENSES_BUNDLED.txt Outdated

Comment thread setup.py Outdated

isuruf reviewed Jan 24, 2021

View reviewed changes

Comment thread third_party/LICENSES_BUNDLED.txt Outdated

mattip force-pushed the license branch from ecd41c3 to 8580002 Compare January 26, 2021 07:26

BLD: create a LICENSE_BUNDLED.txt file from third_party licenses

96a4bf3

mattip force-pushed the license branch from 8580002 to 96a4bf3 Compare January 26, 2021 07:28

mattip changed the title ~~WIP: create a LICENSE_BUNDLED.txt file from third_party licenses~~ BLD: create a LICENSE_BUNDLED.txt file from third_party licenses Jan 26, 2021

rebuild LICENSES_BUNDLED.txt

cd04322

facebook-github-bot reviewed Jan 26, 2021

View reviewed changes

walterddr approved these changes Jan 26, 2021

View reviewed changes

malfet approved these changes Jan 26, 2021

View reviewed changes

Update third_party/build_bundled.py

6058db1

Co-authored-by: Nikita Shulga <nikita.shulga@gmail.com>

facebook-github-bot closed this in 24eab1d Jan 26, 2021

facebook-github-bot added the Merged label Jan 26, 2021

mattip mentioned this pull request Feb 3, 2021

License file in wheels and conda packages does not contain any third-party licenses #50695

Closed

This was referenced Apr 25, 2022

third_party: Fix build_bundled script #76336

Closed

test_license_for_wheel always fails on my local dev copy #76128

Open

mattip mentioned this pull request Jul 10, 2022

LICENSE file in the wheel package references files that are not included in the wheel #81181

Closed

Conversation

mattip commented Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattip commented Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattip commented Jan 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattip commented Jan 21, 2021

Uh oh!

Uh oh!

codecov Bot commented Jan 21, 2021

Codecov Report

Uh oh!

walterddr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mattip commented Jan 21, 2021

Uh oh!

rgommers left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattip commented Jan 26, 2021

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr left a comment

Choose a reason for hiding this comment

Uh oh!

walterddr Jan 25, 2021

Choose a reason for hiding this comment

Uh oh!

malfet Jan 26, 2021

Choose a reason for hiding this comment

Uh oh!

mattip Jan 26, 2021

Choose a reason for hiding this comment

Uh oh!

malfet Jan 26, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

facebook-github-bot commented Jan 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

mattip commented Jan 19, 2021 •

edited

Loading

mattip commented Jan 19, 2021 •

edited

Loading

mattip commented Jan 19, 2021 •

edited

Loading