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

Support license #32

Closed
diazona opened this issue Jul 17, 2023 · 16 comments · Fixed by #78
Closed

Support license #32

diazona opened this issue Jul 17, 2023 · 16 comments · Fixed by #78
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers setuptools-fields Fields in the pyproject data structure that this project needs to support

Comments

@diazona
Copy link
Owner

diazona commented Jul 17, 2023

This issue entails adding support for writing the license field in pyproject.toml when it is defined statically in the setuptools configuration.

@diazona diazona added enhancement New feature or request good first issue Good for newcomers setuptools-fields Fields in the pyproject data structure that this project needs to support labels Jul 17, 2023
@diazona diazona added this to the Initial release milestone Jul 17, 2023
@sjlongland sjlongland self-assigned this Aug 19, 2023
@sjlongland
Copy link
Collaborator

So there are two mechanisms for including a license, inline licenses and external files. pyproject.toml only supports the latter, so much like the long_description (issue #33), we'll need to write a file out and reference it in the pyproject.toml.

This suggests in the medium-term, we should also emit pyproject.toml to a file rather than dumping it to standard output, and/or provide a command switch to specify which file we output to standard output… but this will require careful thought.

@sjlongland
Copy link
Collaborator

The inline case looks like this:

setup.py source:

import setuptools

setuptools.setup(
    name="mypackage",
    version="1.2.3",
    license="""Mock License 1.0

This is a dummy license text to check how setuptools represents
it in the metadata so it can be exposed to pyproject.toml."""
)

dumpmeta output:

RC=0 stuartl@rikishi ~/projects/setuptools-pyproject-migration/experiments/license/inline $ python setup.py dumpmeta
running dumpmeta
{
    "methods": {
…snip…
        "get_license": "Mock License 1.0\n\nThis is a dummy license text to check how setuptools represents\nit in the metadata so it can be exposed to pyproject.toml.",
…snip…
    },
    "properties": {
…snip…
        "metadata": "<distutils.dist.DistributionMetadata object at 0x7f1c18a81410>",
…snip…
    }
}

PR#58 actually inspects that metadata structure, so might tweak dumpmeta to inspect that more closely.

@diazona
Copy link
Owner Author

diazona commented Aug 19, 2023

Oh I actually made some local improvements on the dumpmeta branch while working on readme support, along very similar lines to what you mentioned - I can push those when I get home later today.

@diazona
Copy link
Owner Author

diazona commented Aug 19, 2023

This suggests in the medium-term, we should also emit pyproject.toml to a file rather than dumping it to standard output, and/or provide a command switch to specify which file we output to standard output… but this will require careful thought.

Yeah agreed. I had a feeling for a while that we might wind up going in that direction, but I thought it's something we didn't necessarily need to figure out until later. Printing to stdout was just easier 😛

Of course, one of the complications with writing to pyproject.toml is that it might already exist and we'd have to merge the existing contents with whatever we get from setuptools. Hopefully that won't be too hard, but it is some extra complexity. I think it would be fair to leave that out of an initial alpha/beta release.

@sjlongland
Copy link
Collaborator

Yep, that's fair enough. Probably also when we do write out a new pyproject.toml, we should perhaps rename the old one so they don't lose it (yes, they should be using a SCM, there are devs that do not or don't know how to use it properly).

But… that can come later. :-)

metadata so far though, looks like what the top-level method getters return, which isn't overly surprising, they are exactly that: getters.

        "metadata": {
            "author": null,
            "author_email": null,
            "classifiers": null,
            "description": null,
            "download_url": null,
            "keywords": null,
            "license": "Mock License 1.0\n\nThis is a dummy license text to check how setuptools represents\nit in the metadata so it can be exposed to pyproject.toml.",
            "long_description": null,
            "maintainer": null,
            "maintainer_email": null,
            "name": "mypackage",
            "obsoletes": null,
            "platforms": null,
            "provides": null,
            "requires": null,
            "url": null,
            "version": "1.2.3"
        },

The source actually reads distribution.command_options["metadata"], a fun fact: this is seemingly omitted if the metadata is embedded in setup.py like above.

RC=0 stuartl@rikishi ~/projects/setuptools-pyproject-migration/experiments/license/inline $ python setup.py dumpmeta
running dumpmeta
Traceback (most recent call last):
  File "/home/stuartl/projects/setuptools-pyproject-migration/experiments/license/inline/setup.py", line 3, in <module>
    setuptools.setup(
  File "/home/stuartl/projects/setuptools-pyproject-migration/env/lib/python3.11/site-packages/setuptools/__init__.py", line 107, in setup
    return distutils.core.setup(**attrs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/stuartl/projects/setuptools-pyproject-migration/env/lib/python3.11/site-packages/setuptools/_distutils/core.py", line 185, in setup
    return run_commands(dist)
           ^^^^^^^^^^^^^^^^^^
  File "/home/stuartl/projects/setuptools-pyproject-migration/env/lib/python3.11/site-packages/setuptools/_distutils/core.py", line 201, in run_commands
    dist.run_commands()
  File "/home/stuartl/projects/setuptools-pyproject-migration/env/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 969, in run_commands
    self.run_command(cmd)
  File "/home/stuartl/projects/setuptools-pyproject-migration/env/lib/python3.11/site-packages/setuptools/dist.py", line 1244, in run_command
    super().run_command(command)
  File "/home/stuartl/projects/setuptools-pyproject-migration/env/lib/python3.11/site-packages/setuptools/_distutils/dist.py", line 988, in run_command
    cmd_obj.run()
  File "/home/stuartl/projects/setuptools-pyproject-migration/env/lib/python3.11/site-packages/setuptools_pyproject_migration/__init__.py", line 145, in run
    metadata["metadata"] = self.distribution.command_options["metadata"]
                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^
KeyError: 'metadata'

However, if I put it in setup.cfg:

RC=0 stuartl@rikishi ~/projects/setuptools-pyproject-migration/experiments/license/setupcfg-inline $ tail -n +0 setup.py setup.cfg 
==> setup.py <==
import setuptools

setuptools.setup()

==> setup.cfg <==
[metadata]
name = mypackage
version = 1.2.3
license = Mock License 1.0

  This is a dummy license text to see how setuptools represents such content
  stored inline in setup.cfg

… I do see that metadata key:

RC=0 stuartl@rikishi ~/projects/setuptools-pyproject-migration/experiments/license/setupcfg-inline $ python3 setup.py dumpmeta
running dumpmeta
{
    "metadata": {
        "license": [
            "setup.cfg",
            "Mock License 1.0\n\nThis is a dummy license text to see how setuptools represents such content\nstored inline in setup.cfg"
        ],
        "name": [
            "setup.cfg",
            "mypackage"
        ],
        "version": [
            "setup.cfg",
            "1.2.3"
        ]
    },

@diazona
Copy link
Owner Author

diazona commented Aug 20, 2023

Ohhh that's interesting. Among other things, it probably means we should be more particular about adding tests that use both setup.cfg and setup.py.

@sjlongland
Copy link
Collaborator

Indeed… I think with the "inline" case, we may have to assume it is truly "inlined" (and not an open("file.txt","r").read() call) when it's embedded in setup.py like that, as setuputils/distutils won't tell us where the value came from (it likely doesn't know).

@diazona
Copy link
Owner Author

diazona commented Aug 20, 2023

Yeah but setuptools should at least know whether it receives a string value that starts with file:. (If licenses act the same way that readme files do)

@diazona
Copy link
Owner Author

diazona commented Aug 20, 2023

BTW I saw that you pushed to debug/dumpmeta so I pushed my own version of that branch to diazona/dumpmeta - feel free to take anything you like (cherry-pick, copy/paste, whatever) and incorporate it into debug/dumpmeta. I don't need to keep my version of the branch around.

@sjlongland
Copy link
Collaborator

BTW I saw that you pushed to debug/dumpmeta so I pushed my own version of that branch to diazona/dumpmeta - feel free to take anything you like (cherry-pick, copy/paste, whatever) and incorporate it into debug/dumpmeta. I don't need to keep my version of the branch around.

Tried both versions, I think yours does a better job and is cleaner to boot. :-)

@sjlongland
Copy link
Collaborator

Okay… so there's several ways a license can be included.

  • In setup.py:
    • Inline via license keyword argument
      • (1) as a bare license string
      • (2) as a file reference with file: prefix
    • (3) Referenced via license_file keyword argument (raises deprecation warning)
    • (4) Referenced via license_files keyword argument
  • In setup.cfg:
    • Inline via license property
      • (5) as a bare license string
      • (6) as a file reference with file: prefix
    • (7) Referenced via license_file property (raises deprecation warning)
    • (8) Referenced via license_files property

(Yep… and now for something completely different.)

@sjlongland
Copy link
Collaborator

https://gist.github.com/sjlongland/1b79ac2f87a2c9ac5a947a27c99777a8 ← that basically describes the behaviour of setuptools for different project configurations.

Using file: prefixes in setup.cfg is forbidden for the license property (setuptools will throw an exception).

Using file: prefixes in setup.py seems to add the file to license_files.

license_file is deprecated in both places and raises warnings, but some projects may still use it. license_files is the replacement.

Unlike using license; license_files does not cause the license to be displayed in setup.py --license (or make it visible via get_license()). Nor does using a file: prefix in setup.py.

@diazona
Copy link
Owner Author

diazona commented Aug 20, 2023

Oh that is maddeningly inconsistent 😆

Personally, and this is most definitely just an opinion, I would follow a TDD approach here: start by writing simple test cases for all eight configurations. (assuming setuptools "officially" claims to support all of them - obviously anything that setuptools doesn't claim to support, we don't have to care about) The tests can be decorated with pytest.mark.xfail to indicate that they won't pass, and they could even be merged to main in that state. Then you can go ahead and work on an implementation, or even a partial implementation that only handles the easy cases, knowing that the tests will track the progress and make it explicitly clear which versions are working. I find that following TDD takes the pressure off me to have to do the whole thing correctly.

In fact, I think we could even make our initial release without having an implementation that covers all the cases. (i.e. there's really no pressure to do this completely correctly now) As far as I'm concerned, just getting something that works for some of the simpler projects would be a totally fine chunk of functionality to post on PyPI and start getting some feedback.

@diazona
Copy link
Owner Author

diazona commented Sep 4, 2023

How's it going with this feature? I think we're getting pretty close to the initial release milestone and this is one of the few things left

@sjlongland
Copy link
Collaborator

I've been absolutely hammered by work… so it's taking waaay longer than I had hoped. I have some time this week-end, so should try and get it done then.

@sjlongland
Copy link
Collaborator

So, having a look at this… also in relation to the library mentioned in #74, seems license is the "official" way in Python projects, license_files is an extension described in PEP-639.

https://peps.python.org/pep-0639/

That PEP down further describes what it'd look like in the TOML:

[project]
license = "MIT AND (Apache-2.0 OR BSD-2-Clause)"
license-files.globs = [
    "LICENSE*",
    "setuptools/_vendor/LICENSE*",
]

Or specified explicitly:

[project]
license = "MIT AND (Apache-2.0 OR BSD-2-Clause)"
license-files.paths = [
    "LICENSE",
    "setuptools/_vendor/LICENSE",
    "setuptools/_vendor/LICENSE.APACHE",
    "setuptools/_vendor/LICENSE.BSD",
]

The core specification defines license as a string, a plain string:
https://packaging.python.org/en/latest/specifications/core-metadata/#license

So it appears once the decision is made on PEP-639; if that's adopted it'll basically be a 1-1 mapping license_fileslicense-files.globs or license_fileslicense-files.paths, depending on whether all the files "exist" or not. os.path.isfile() should be enough to figure out which of these two we add a file to.

@diazona diazona linked a pull request Sep 10, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers setuptools-fields Fields in the pyproject data structure that this project needs to support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants