Repository: pypa/setuptools
Pull request #1767 (2019-05-20 23:04:56)

Add support for `license_files` option in metadata
## Summary of changes

Support for `license_file` was added to `bdist_wheel` in pypa/wheel#47 / pypa/wheel@0acfb4c19dc2d2f2d067eead3d59a5f3415c5c20 and then to setuptools in #1536. Last year, wheel also added support for a `license_files` option in pypa/wheel#138 / pypa/wheel@59976ab294e1b118f42cab404d95df66ed55f7e4 to enable including more than one license. At the same time, `license_file` was deprecated in that project, and using it leads to a `DeprecationWarning` if warnings are enabled. The [docs](https://wheel.readthedocs.io/en/stable/user_guide.html#including-license-files-in-the-generated-wheel-file) for wheel also mention that `license_files` is the favored option.

Currently, setuptools only supports `license_file`, which leads to the `DeprecationWarning` code path if wheel is also used when building distributions. 

This PR adds the `license_files` functionality to setuptools, which enables the more versatile form to work across both tools and allows users to take advantage of the functionality without hitting deprecated code.

A few notes about the implementation:

- If `license_file` is specified, the file is also added to the set (same behavior as wheel), which preserves backwards compatibility. I wasn't sure if any sort of deprecation was appropriate here, so I left that part out

- I added `license_files` to `Distribution` and `ConfigMetadataHandler` because it seemed to be the best way to reuse the list option parser, but please advise if it should be done a different way

- Leaving the option empty in wheel disables license auto-including, which AFAICT matches the default behavior of setuptools if either option is empty (and MANIFEST.in doesn't include them)

### Pull Request Checklist
- [x] Changes have tests
- [x] News fragment added in changelog.d. See [documentation](http://setuptools.readthedocs.io/en/latest/developer-guide.html#making-a-pull-request) for details


jakirkham: Would it be possible to get a review?
techdragon: @jakirkham I cant put proper review comments as I'm not on the setuptools team, but it looks like @kchmck has a pretty good PR here. 👍 
jaraco: An important reference to this effort is [this thread](https://discuss.python.org/t/improving-license-clarity-with-better-package-metadata/2154), where many in the PyPA settle on supporting license files.

Patch:
diff --git a/changelog.d/1767.change.rst b/changelog.d/1767.change.rst
new file mode 100644
index 000000000..5d42aedc8
--- /dev/null
+++ b/changelog.d/1767.change.rst
@@ -0,0 +1,2 @@
+Add support for the ``license_files`` option in ``setup.cfg`` to automatically
+include multiple license files in a source distribution.
diff --git a/docs/setuptools.txt b/docs/setuptools.txt
index 344ea5bc3..22025f61c 100644
--- a/docs/setuptools.txt
+++ b/docs/setuptools.txt
@@ -2276,6 +2276,7 @@ maintainer_email                maintainer-email   str
 classifiers                     classifier         file:, list-comma
 license                                            str
 license_file                                       str
+license_files                                      list-comma
 description                     summary            file:, str
 long_description                long-description   file:, str
 long_description_content_type                      str                38.6.0
diff --git a/setuptools/command/sdist.py b/setuptools/command/sdist.py
index dc2539814..55ecdd978 100644
--- a/setuptools/command/sdist.py
+++ b/setuptools/command/sdist.py
@@ -5,7 +5,7 @@ import sys
 import io
 import contextlib
 
-from setuptools.extern import six
+from setuptools.extern import six, ordered_set
 
 from .py36compat import sdist_add_defaults
 
@@ -200,10 +200,12 @@ class sdist(sdist_add_defaults, orig.sdist):
         manifest.close()
 
     def check_license(self):
-        """Checks if license_file' is configured and adds it to
-        'self.filelist' if the value contains a valid path.
+        """Checks if license_file' or 'license_files' is configured and adds any
+        valid paths to 'self.filelist'.
         """
 
+        files = ordered_set.OrderedSet()
+
         opts = self.distribution.get_option_dict('metadata')
 
         # ignore the source of the value
@@ -211,11 +213,19 @@ class sdist(sdist_add_defaults, orig.sdist):
 
         if license_file is None:
             log.debug("'license_file' option was not specified")
-            return
+        else:
+            files.add(license_file)
 
-        if not os.path.exists(license_file):
-            log.warn("warning: Failed to find the configured license file '%s'",
-                    license_file)
-            return
+        try:
+            files.update(self.distribution.metadata.license_files)
+        except TypeError:
+            log.warn("warning: 'license_files' option is malformed")
+
+        for f in files:
+            if not os.path.exists(f):
+                log.warn(
+                    "warning: Failed to find the configured license file '%s'",
+                    f)
+                files.remove(f)
 
-        self.filelist.append(license_file)
+        self.filelist.extend(files)
diff --git a/setuptools/config.py b/setuptools/config.py
index 2d50e25e8..9b9a0c45e 100644
--- a/setuptools/config.py
+++ b/setuptools/config.py
@@ -483,6 +483,7 @@ class ConfigMetadataHandler(ConfigHandler):
             'obsoletes': parse_list,
             'classifiers': self._get_parser_compound(parse_file, parse_list),
             'license': exclude_files_parser('license'),
+            'license_files': parse_list,
             'description': parse_file,
             'long_description': parse_file,
             'version': self._parse_version,
diff --git a/setuptools/dist.py b/setuptools/dist.py
index 2e5ad4bd6..0f3f7322d 100644
--- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -409,6 +409,7 @@ class Distribution(_Distribution):
         'long_description_content_type': None,
         'project_urls': dict,
         'provides_extras': ordered_set.OrderedSet,
+        'license_files': ordered_set.OrderedSet,
     }
 
     _patched_dist = None


Test patch:
diff --git a/setuptools/tests/test_egg_info.py b/setuptools/tests/test_egg_info.py
index 316eb2edd..0db204baa 100644
--- a/setuptools/tests/test_egg_info.py
+++ b/setuptools/tests/test_egg_info.py
@@ -524,27 +524,27 @@ class TestEggInfo:
                               [metadata]
                               license_file = LICENSE
                               """),
-            'LICENSE': DALS("Test license")
+            'LICENSE': "Test license"
         }, True), # with license
         ({
             'setup.cfg': DALS("""
                               [metadata]
                               license_file = INVALID_LICENSE
                               """),
-            'LICENSE': DALS("Test license")
+            'LICENSE': "Test license"
         }, False), # with an invalid license
         ({
             'setup.cfg': DALS("""
                               """),
-            'LICENSE': DALS("Test license")
+            'LICENSE': "Test license"
         }, False), # no license_file attribute
         ({
             'setup.cfg': DALS("""
                               [metadata]
                               license_file = LICENSE
                               """),
-            'MANIFEST.in': DALS("exclude LICENSE"),
-            'LICENSE': DALS("Test license")
+            'MANIFEST.in': "exclude LICENSE",
+            'LICENSE': "Test license"
         }, False) # license file is manually excluded
     ])
     def test_setup_cfg_license_file(
@@ -567,6 +567,204 @@ class TestEggInfo:
             assert 'LICENSE' not in sources_text
             assert 'INVALID_LICENSE' not in sources_text # for invalid license test
 
+    @pytest.mark.parametrize("files, incl_licenses, excl_licenses", [
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files =
+                                  LICENSE-ABC
+                                  LICENSE-XYZ
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-ABC', 'LICENSE-XYZ'], []), # with licenses
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files = LICENSE-ABC, LICENSE-XYZ
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-ABC', 'LICENSE-XYZ'], []), # with commas
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files =
+                                  LICENSE-ABC
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-ABC'], ['LICENSE-XYZ']), # with one license
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files =
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, [], ['LICENSE-ABC', 'LICENSE-XYZ']), # empty
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files = LICENSE-XYZ
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-XYZ'], ['LICENSE-ABC']), # on same line
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files =
+                                  LICENSE-ABC
+                                  INVALID_LICENSE
+                              """),
+            'LICENSE-ABC': "Test license"
+        }, ['LICENSE-ABC'], ['INVALID_LICENSE']), # with an invalid license
+        ({
+            'setup.cfg': DALS("""
+                              """),
+            'LICENSE': "Test license"
+        }, [], ['LICENSE']), # no license_files attribute
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files = LICENSE
+                              """),
+            'MANIFEST.in': "exclude LICENSE",
+            'LICENSE': "Test license"
+        }, [], ['LICENSE']), # license file is manually excluded
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_files =
+                                  LICENSE-ABC
+                                  LICENSE-XYZ
+                              """),
+            'MANIFEST.in': "exclude LICENSE-XYZ",
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-ABC'], ['LICENSE-XYZ']) # subset is manually excluded
+    ])
+    def test_setup_cfg_license_files(
+            self, tmpdir_cwd, env, files, incl_licenses, excl_licenses):
+        self._create_project()
+        build_files(files)
+
+        environment.run_setup_py(
+            cmd=['egg_info'],
+            pypath=os.pathsep.join([env.paths['lib'], str(tmpdir_cwd)])
+        )
+        egg_info_dir = os.path.join('.', 'foo.egg-info')
+
+        with open(os.path.join(egg_info_dir, 'SOURCES.txt')) as sources_file:
+            sources_lines = list(line.strip() for line in sources_file)
+
+        for lf in incl_licenses:
+            assert sources_lines.count(lf) == 1
+
+        for lf in excl_licenses:
+            assert sources_lines.count(lf) == 0
+
+    @pytest.mark.parametrize("files, incl_licenses, excl_licenses", [
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_file =
+                              license_files =
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, [], ['LICENSE-ABC', 'LICENSE-XYZ']), # both empty
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_file =
+                                  LICENSE-ABC
+                                  LICENSE-XYZ
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-XYZ': "XYZ license"
+        }, [], ['LICENSE-ABC', 'LICENSE-XYZ']), # license_file is still singular
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_file = LICENSE-ABC
+                              license_files =
+                                  LICENSE-XYZ
+                                  LICENSE-PQR
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-PQR': "PQR license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-ABC', 'LICENSE-PQR', 'LICENSE-XYZ'], []), # combined
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_file = LICENSE-ABC
+                              license_files =
+                                  LICENSE-ABC
+                                  LICENSE-XYZ
+                                  LICENSE-PQR
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-PQR': "PQR license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-ABC', 'LICENSE-PQR', 'LICENSE-XYZ'], []), # duplicate license
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_file = LICENSE-ABC
+                              license_files =
+                                  LICENSE-XYZ
+                              """),
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-PQR': "PQR license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-ABC', 'LICENSE-XYZ'], ['LICENSE-PQR']), # combined subset
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_file = LICENSE-ABC
+                              license_files =
+                                  LICENSE-XYZ
+                                  LICENSE-PQR
+                              """),
+            'LICENSE-PQR': "Test license"
+        }, ['LICENSE-PQR'], ['LICENSE-ABC', 'LICENSE-XYZ']), # with invalid licenses
+        ({
+            'setup.cfg': DALS("""
+                              [metadata]
+                              license_file = LICENSE-ABC
+                              license_files =
+                                LICENSE-PQR
+                                LICENSE-XYZ
+                              """),
+            'MANIFEST.in': "exclude LICENSE-ABC\nexclude LICENSE-PQR",
+            'LICENSE-ABC': "ABC license",
+            'LICENSE-PQR': "PQR license",
+            'LICENSE-XYZ': "XYZ license"
+        }, ['LICENSE-XYZ'], ['LICENSE-ABC', 'LICENSE-PQR']) # manually excluded
+    ])
+    def test_setup_cfg_license_file_license_files(
+            self, tmpdir_cwd, env, files, incl_licenses, excl_licenses):
+        self._create_project()
+        build_files(files)
+
+        environment.run_setup_py(
+            cmd=['egg_info'],
+            pypath=os.pathsep.join([env.paths['lib'], str(tmpdir_cwd)])
+        )
+        egg_info_dir = os.path.join('.', 'foo.egg-info')
+
+        with open(os.path.join(egg_info_dir, 'SOURCES.txt')) as sources_file:
+            sources_lines = list(line.strip() for line in sources_file)
+
+        for lf in incl_licenses:
+            assert sources_lines.count(lf) == 1
+
+        for lf in excl_licenses:
+            assert sources_lines.count(lf) == 0
+
     def test_long_description_content_type(self, tmpdir_cwd, env):
         # Test that specifying a `long_description_content_type` keyword arg to
         # the `setup` function results in writing a `Description-Content-Type`