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

Update the specification with PEP 639 #1662

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Conversation

befeleme
Copy link
Contributor

@befeleme befeleme commented Nov 19, 2024

@befeleme

This comment was marked as outdated.

@befeleme befeleme marked this pull request as draft November 19, 2024 12:15
@befeleme
Copy link
Contributor Author

befeleme commented Nov 19, 2024

@befeleme befeleme force-pushed the pep639 branch 3 times, most recently from e652a7e to f5baeba Compare November 19, 2024 15:05
@webknjaz webknjaz added the type: enhancement A self-contained enhancement or new feature label Nov 20, 2024
source/glossary.rst Outdated Show resolved Hide resolved
source/glossary.rst Outdated Show resolved Hide resolved
source/glossary.rst Outdated Show resolved Hide resolved
@webknjaz
Copy link
Member

@befeleme I've added code suggestions with :file: into a few places, but I'd like to ask you to also apply the same technique in other places that I've missed.

@befeleme
Copy link
Contributor Author

I addressed the review points in the fixup commits for better readability (will squash them later once we get closer to merging). I still walk through the pages looking for the gaps. Thank you for the sphinx tips, that's very helpful!

@befeleme befeleme force-pushed the pep639 branch 2 times, most recently from 22a449c to 4c81d90 Compare November 26, 2024 11:54
@befeleme befeleme marked this pull request as ready for review November 26, 2024 11:59
@befeleme
Copy link
Contributor Author

I think it's ready.

@befeleme
Copy link
Contributor Author

Is there anything I can do to make this move forward?

@befeleme
Copy link
Contributor Author

The linkcheck reports on https://pypi.org/project/pip/23.3.1/#files (Non-existing anchor), but that's surprising: yesterday's cron job reported is as success and the link works (opens PyPI on the files tab). Also, it's not related to any changes made for this PR.

Copy link

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this @befeleme!

source/tutorials/packaging-projects.rst Outdated Show resolved Hide resolved
source/guides/writing-pyproject-toml.rst Outdated Show resolved Hide resolved
source/guides/writing-pyproject-toml.rst Outdated Show resolved Hide resolved
source/specifications/pyproject-toml.rst Show resolved Hide resolved
source/specifications/pyproject-toml.rst Outdated Show resolved Hide resolved
Comment on lines 121 to 128
Putting it all together, our :file:`setup.cfg` would be:

.. code-block:: ini

[metadata]
license_expression = MIT AND (Apache-2.0 OR BSD-2-Clause)
license_files =
LICENSE
setuptools/_vendor/packaging/LICENSE
setuptools/_vendor/packaging/LICENSE.APACHE
setuptools/_vendor/packaging/LICENSE.BSD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just remove the setup.cfg example.

Comment on lines +239 to +237
However, it is still a good idea to include ``LicenseRef-Proprietary``
as a license expression in your package configuration, and/or a
copyright statement and any legal notices in a :file:`LICENSE.txt` file
in the root of your project directory, which will be automatically
included by packaging tools.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's the case for license expressions as well but isn't there a classifier which causes PyPI to reject uploads?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may happen that package authors decide to upload a package warranted by a proprietary license to PyPI (https://pypi.org/search/?q=&o=&c=License+%3A%3A+Other%2FProprietary+License shows over 6700 packages) - this paragraph is specifically meant to cover those cases.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it's the case for license expressions as well but isn't there a classifier which causes PyPI to reject uploads?

Just found it. I meant the Private :: Do Not Upload classifier. https://pypi.org/classifiers/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"this paragraph is specifically meant to cover those cases" - but wouldn't the package be "shared publicly" in these cases if it's uploaded to PyPI? (But I can see where it might be useful to add the designation even for proprietary code that only ever touches the company's private index - maybe it's used by the company's higher-level tooling, or is otherwise relevant to their business procedures.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package is shared publicly on the base of exception granted to PyPI (https://policies.python.org/pypi.org/Terms-of-Use/) but the original license applies for any consumers of that package.
Tools scanning package metadata would have a clear indicator that there's something to be aware of. An example from the field I'm familiar with: a person trying to create a Fedora rpm package out of the Python distribution would be warned that the license doesn't allow that package to be included into Fedora Linux repositories.

Comment on lines 246 to 248
I just want to share my own work without legal restrictions
'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

While you aren't required to include a license, if you don't, no one has
`any permission to download, use or improve your work <dontchoosealicense_>`__,
so that's probably the *opposite* of what you actually want.
The `MIT license <chooseamitlicense_>`__ is a great choice instead, as it's simple,
widely used and allows anyone to do whatever they want with your work
(other than sue you, which you probably also don't want).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't Unlicense a good candidate for that?

I completely agree that MIT is a great license for OSS projects. Maybe it should be mentioned in it's own use case at the top. Something along the lines of:

I have a simple / new project, which license should I choose?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The guidelines come from the original authors of the PEP whom I trust in the matter but I'm in no way a domain expert, so I don't feel competent to change recommendations. I also don't think Python packaging should be too specific when it comes to licenses. The MIT one has already appeared in https://packaging.python.org/en/latest/guides/writing-pyproject-toml/ and I'd just left it as it is.

Comment on lines +334 to +340
As an example, if your project was licensed MIT but incorporated
a vendored dependency (say, ``packaging``) that was licensed under
either Apache 2.0 or the 2-clause BSD, your license expression would
be ``MIT AND (Apache-2.0 OR BSD-2-Clause)``. You might have a
:file:`LICENSE.txt` in your repo root, and a :file:`LICENSE-APACHE.txt` and
:file:`LICENSE-BSD.txt` in the ``_vendor`` subdirectory, so to include
all of them, you'd specify ``["LICENSE.txt", "_vendor/packaging/LICENSE*"]``
as glob patterns, or
``["LICENSE.txt", "_vendor/LICENSE-APACHE.txt", "_vendor/LICENSE-BSD.txt"]``
as literal file paths.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above about vendoring.

@webknjaz
Copy link
Member

The linkcheck reports on https://pypi.org/project/pip/23.3.1/#files (Non-existing anchor), but that's surprising: yesterday's cron job reported is as success and the link works (opens PyPI on the files tab). Also, it's not related to any changes made for this PR.

You're right. Over the past few days I noticed a quick Fastly loading screen showing up on PyPI, which then redirects to where I was going originally.
So I probed it with cURL just now and verified that this is what's happening, and HTML DOM no longer contains that in the HTTP first response (this is probably cookie-based):

$ curl -v 'https://pypi.org/project/pip/23.3.1/#files'
* Host pypi.org:443 was resolved.
* IPv6: 2a04:4e42:600::223, 2a04:4e42:200::223, 2a04:4e42:400::223, 2a04:4e42::223
* IPv4: 151.101.0.223, 151.101.192.223, 151.101.64.223, 151.101.128.223
*   Trying [2a04:4e42:600::223]:443...
* Immediate connect fail for 2a04:4e42:600::223: Network is unreachable
*   Trying [2a04:4e42:200::223]:443...
* Immediate connect fail for 2a04:4e42:200::223: Network is unreachable
*   Trying [2a04:4e42:400::223]:443...
* Immediate connect fail for 2a04:4e42:400::223: Network is unreachable
*   Trying [2a04:4e42::223]:443...
* Immediate connect fail for 2a04:4e42::223: Network is unreachable
*   Trying 151.101.0.223:443...
* ALPN: curl offers h2,http/1.1
* TLSv1.3 (OUT), TLS handshake, Client hello (1):
*  CAfile: /etc/ssl/certs/ca-certificates.crt
*  CApath: /etc/ssl/certs
* TLSv1.3 (IN), TLS handshake, Server hello (2):
* TLSv1.3 (IN), TLS handshake, Encrypted Extensions (8):
* TLSv1.3 (IN), TLS handshake, Certificate (11):
* TLSv1.3 (IN), TLS handshake, CERT verify (15):
* TLSv1.3 (IN), TLS handshake, Finished (20):
* TLSv1.3 (OUT), TLS change cipher, Change cipher spec (1):
* TLSv1.3 (OUT), TLS handshake, Finished (20):
* SSL connection using TLSv1.3 / TLS_AES_128_GCM_SHA256 / x25519 / RSASSA-PSS
* ALPN: server accepted h2
* Server certificate:
*  subject: CN=pypi.org
*  start date: Apr 23 04:22:05 2024 GMT
*  expire date: May 25 04:22:04 2025 GMT
*  subjectAltName: host "pypi.org" matched cert's "pypi.org"
*  issuer: C=BE; O=GlobalSign nv-sa; CN=GlobalSign Atlas R3 DV TLS CA 2024 Q2
*  SSL certificate verify ok.
*   Certificate level 0: Public key type RSA (2048/112 Bits/secBits), signed using sha256WithRSAEncryption
*   Certificate level 1: Public key type RSA (2048/112 Bits/secBits), signed using sha256WithRSAEncryption
*   Certificate level 2: Public key type RSA (2048/112 Bits/secBits), signed using sha256WithRSAEncryption
* TLSv1.3 (IN), TLS handshake, Newsession Ticket (4):
* Connected to pypi.org (151.101.0.223) port 443
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://pypi.org/project/pip/23.3.1/#files
* [HTTP/2] [1] [:method: GET]
* [HTTP/2] [1] [:scheme: https]
* [HTTP/2] [1] [:authority: pypi.org]
* [HTTP/2] [1] [:path: /project/pip/23.3.1/]
* [HTTP/2] [1] [user-agent: curl/8.10.1]
* [HTTP/2] [1] [accept: */*]
> GET /project/pip/23.3.1/ HTTP/2
> Host: pypi.org
> User-Agent: curl/8.10.1
> Accept: */*
> 
* Request completely sent off
< HTTP/2 200 
< set-cookie: _fs_ch_st_FSBmUei20MqUiJb9=ARwOUcLntEKxNCnL5W0on4gbZZuJgKNFAuJTwV5kqlwObPx5zOjadDLJ8iZ2jXY2v-kRpx0J1npexkvu_R75uguNU_5S13wmbTRuQr1zm4AghacYsZb2dTQG9sPxmJahlzJLe16uBKWgCnaeE4pXhqsMs77NogoTpKoqJhS6nkwgjtK2hJA3s4d8d4JnXTvMtJRqm3vtuDFWp5s6OqiT-u3N-QTbB58=; Max-Age=10; HttpOnly; Path=/
< content-type: text/html; charset=utf-8
< cache-control: no-store
< accept-ranges: bytes
< date: Wed, 11 Dec 2024 23:00:42 GMT
< x-served-by: cache-iad-kcgs7200169-IAD, cache-iad-kjyo7100141-IAD, cache-fra-eddf8230065-FRA
< x-cache: MISS, MISS
< x-cache-hits: 0, 0
< x-timer: S1733958043.869433,VS0,VE107
< strict-transport-security: max-age=31536000; includeSubDomains; preload
< x-frame-options: deny
< x-xss-protection: 1; mode=block
< x-content-type-options: nosniff
< x-permitted-cross-domain-policies: none
< permissions-policy: publickey-credentials-create=(self),publickey-credentials-get=(self),accelerometer=(),ambient-light-sensor=(),autoplay=(),battery=(),camera=(),display-capture=(),document-domain=(),encrypted-media=(),execution-while-not-rendered=(),execution-while-out-of-viewport=(),fullscreen=(),gamepad=(),geolocation=(),gyroscope=(),hid=(),identity-credentials-get=(),idle-detection=(),local-fonts=(),magnetometer=(),microphone=(),midi=(),otp-credentials=(),payment=(),picture-in-picture=(),screen-wake-lock=(),serial=(),speaker-selection=(),storage-access=(),usb=(),web-share=(),xr-spatial-tracking=()
< 
<!DOCTYPE html>
<html>
  <head>
    <meta
      http-equiv="Content-Security-Policy"
      content="default-src 'self'; img-src 'self' data:; media-src 'self' data:; object-src 'none'; style-src 'self' 'sha256-o4vzfmmUENEg4chMjjRP9EuW9ucGnGIGVdbl8d0SHQQ='; script-src 'self' 'sha256-a9bHdQGvRzDwDVzx8m+Rzw+0FHZad8L0zjtBwkxOIz4=';"
    />
    <link
      href="/_fs-ch-1T1wmsGaOgGaSxcX/assets/inter-var.woff2"
      rel="preload"
      as="font"
      type="font/woff2"
      crossorigin
    />
    <link href="/_fs-ch-1T1wmsGaOgGaSxcX/assets/styles.css" rel="stylesheet" />
    <meta
      name="viewport"
      content="width=device-width, initial-scale=1, maximum-scale=1"
    />
    <style>
      #loading-error {
        font-size: 16px;
        font-family: 'Inter', sans-serif;
        margin-top: 10px;
        margin-left: 10px;
        display: none;
      }
    </style>
  </head>
  <body>
    <noscript>
      <div class="noscript-container">
        <div class="noscript-content">
          <img
            src="/_fs-ch-1T1wmsGaOgGaSxcX/assets/errorIcon.svg"
            alt="Error Icon"
            class="error-icon"
          />
          <span class="noscript-span"
            >JavaScript is disabled in your browser.</span
          >
          Please enable JavaScript to proceed.
        </div>
      </div>
    </noscript>
    <div id="loading-error">
      A required part of this site couldn’t load. This may be due to a browser
      extension, network issues, or browser settings. Please check your
      connection, disable any ad blockers, or try using a different browser.
    </div>
    <script>
      function loadScript(src) {
        return new Promise((resolve, reject) => {
          const script = document.createElement('script');
          script.onload = resolve;
          script.onerror = (event) => {
            console.error('Script load error event:', event);
            document.getElementById('loading-error').style.display = 'block';
            reject(
              new Error(
                `Failed to load script: ${src}, Please contact the service administrator.`
              )
            );
          };
          script.src = src;
          document.body.appendChild(script);
        });
      }

      loadScript('/_fs-ch-1T1wmsGaOgGaSxcX/errors.js')
        .then(() => {
          const script = document.createElement('script');
          script.src = '/_fs-ch-1T1wmsGaOgGaSxcX/script.js?reload=true';
          script.onerror = (event) => {
            console.error('Script load error event:', event);
            const errorMsg = new Error(
              `Failed to load script: ${script.src}. Please contact the service administrator.`
            );
            console.error(errorMsg);
            handleScriptError();
          };
          document.body.appendChild(script);
        })
        .catch((error) => {
          console.error(error);
        });
    </script>
  </body>
</html>
* Connection #0 to host pypi.org left intact

Nevertheless, this would be blocking PR merges, and so we have to address it by possibly adding the URL to nitpick_ignore or adjusting the anchor checks somehow. It's best to do this in a separate PR.

Tools MUST assume that license file content is valid UTF-8 encoded text,
and SHOULD validate this and raise an error if it is not.

Literal paths (e.g. :file:`LICENSE`) are treated as valid globs which means they
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can tell, literal paths are valid glob patterns, not merely treated as such. Anyway, I'm not sure this use case needs to be highlighted explicitly... ?

Copy link
Contributor Author

@befeleme befeleme Dec 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll replace: s/are treated as/are/
The sentence comes as a result of a long discussion about the license-files value, with the earlier PEP draft defining separate paths and globs subkeys. That draft has been implemented in some build backends. Since in the end the subkeys were "merged" into one array rather that one of them abandoned, it made sense to explicitly describe that transition. I'd keep it there for full clarity.

- MUST raise an error if any individual user-specified pattern does not match
at least one file.

If the ``license-files`` key is present and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I understood this in the PEP, either - but it seems that, when there is no license-files key, the build system can do what it wants (either omit the license files entirely, or use its own logic to choose files that correspond to the license expression, etc.)? Whatever the answer, it might be worth highlighting here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your assumption is correct, I'd add the proper sentence stating that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

befeleme and others added 18 commits December 13, 2024 14:52
Copied and adapted from PEP 639.
Redefine the license key, add license-files, mention that license
classifiers are deprecated now.
…P 639

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
@@ -66,6 +66,11 @@ The ``METADATA`` file is mandatory.
All other files may be omitted at the installing tool's discretion.
Additional installer-specific files may be present.

This ``.dist-info`` directory may contain the following directory, described in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This ``.dist-info`` directory may contain the following directory, described in
This :file:`.dist-info/` directory may contain the following directory, described in

This ``.dist-info`` directory may contain the following directory, described in
detail below:

* ``licenses/``: contains license files.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* ``licenses/``: contains license files.
* :file:`licenses/`: contains license files.

@@ -259,3 +275,5 @@ History
for the full definition.
- September 2020: Various amendments and clarifications were approved through
:pep:`627`.
- December 2024: The .dist-info/licenses directory was specified through
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- December 2024: The .dist-info/licenses directory was specified through
- December 2024: The :file:`.dist-info/licenses/` directory was specified through

@@ -427,6 +438,8 @@ History
regular files (the expected behaviour of consuming tools when encountering
symlinks or subdirectories in this folder is not formally defined, and hence
may vary between tools).
- December 2024: The .dist-info/licenses directory was specified through
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- December 2024: The .dist-info/licenses directory was specified through
- December 2024: The :file:`.dist-info/licenses/` directory was specified through

either Apache 2.0 or the 2-clause BSD, your license expression would
be ``MIT AND (Apache-2.0 OR BSD-2-Clause)``. You might have a
:file:`LICENSE.txt` in your repo root, and a :file:`LICENSE-APACHE.txt` and
:file:`LICENSE-BSD.txt` in the ``_vendor`` subdirectory, so to include
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
:file:`LICENSE-BSD.txt` in the ``_vendor`` subdirectory, so to include
:file:`LICENSE-BSD.txt` in the :file:`_vendor/` subdirectory, so to include

In the built wheel, with :file:`/` being the root of the archive and
``{VERSION}`` as the previous, the license files would be stored at:

.. code-block:: shell
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.. code-block:: shell
.. code-block:: text

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A self-contained enhancement or new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

State clearly how a license should be declared
5 participants