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

PEP 517: Make final #1712

Merged
merged 1 commit into from
Jun 30, 2021
Merged

PEP 517: Make final #1712

merged 1 commit into from
Jun 30, 2021

Conversation

pfmoore
Copy link
Member

@pfmoore pfmoore commented Nov 13, 2020

Make PEP 517 final. It's implemented in most tools now, and has plenty of real-world usage, so I think it's time for it to be made final.

/cc @ncoghlan as BDFL-Delegate for the original PEP, do you agree?

@pradyunsg
Copy link
Member

My only concern with marking this PEP Final: The entire config_settings section of this PEP is basically not implemented/tried/adopted by anyone (at least not enough folks -- pip doesn't do anything about it, and AFAIK, no backend uses it either).

@pfmoore
Copy link
Member Author

pfmoore commented Nov 14, 2020

Conversely, if we’re going to wait for that, what can we do to move it along? I think it’s fairly obvious by now that for 99% of use cases, config options are not needed. Do we wait for someone with both a use case and the time/expertise to implement something?

I’d prefer to leave it as it stands, make it final, and then if someone later comes along with an update to the design, we can just update the spec as needed, with a new PEP if the change is sufficiently substantial. (Personally, I’m open to approving small or low impact changes being made direct to the PyPA specs document, I feel the PEP process should be a tool, not a burden).

Do we need a wider discussion on what provisional means for packaging interop standards? (I hope not, it seems a bit too abstract for me...) I think the standards should be adaptable as the ecosystem changes. Final status should not block that - if it does, we need to keep PEPs provisional essentially forever.

@pfmoore
Copy link
Member Author

pfmoore commented Nov 15, 2020

One further point - the config_settings mechanism is by now included in every implementation of PEP 517, even if it's not actually used for anything. So changing it in a backward incompatible way would be pretty disruptive. Making the PEP final doesn't prohibit changes - it just requires us to do so while taking backward compatibility into account. Which we'd have to do anyway at this point, so IMO marking the PEP as final is really just a formality now.

Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

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

Let's do it.

/cc @ncoghlan @brettcannon

@brettcannon
Copy link
Member

/cc @njsmith @takluyver as co-authors

@takluyver
Copy link
Contributor

Fine by me 🙂

@abitrolly
Copy link
Contributor

There are more concerns matching @pradyunsg that config_settings section is not well defined pypa/setuptools#2491 (comment)

@abitrolly
Copy link
Contributor

Found one problematic part.

For example, ``pip``
might choose to map a mix of modern and legacy command line arguments
like::

  pip install                                           \
    --package-config CC=gcc                             \
    --global-option="--some-global-option"              \
    --build-option="--build-option1"                    \
    --build-option="--build-option2"

into a ``config_settings`` dictionary like::

  {
   "CC": "gcc",
   "--global-option": ["--some-global-option"],
   "--build-option": ["--build-option1", "--build-option2"],
  }
  • it assumes the frontend knows about the backend options, and it assumes backends should account for an unexpected option from the frontend
  • there is no specification on config serialization, which again assumes that frontend should know what backend accepts and vice versa, for example where is the sign that "CC" values should be serialized as strings and --global-option values as a list of strings? The paragraph above says about string-key/string-value pairs and the example illustrates string-key/list-value.

Another paragraph.

The hooks may be called with positional or keyword arguments, so backends
implementing them should be careful to make sure that their signatures match
both the order and the names of the arguments above.

In the end, is it a single parameter dict, or a positional and keyword arguments?
Why it is in the config settings section?

  • should be at least the link to specification of those arguments

MAY print arbitrary informational text on stdout and stderr

  • better define specific logging interface to get debug messages for backend in uniform way

Why is this in the config settings section?

If a hook raises an exception, or causes the process to terminate,
then this indicates an error.

  • how hook can cause the process to terminate, which process? Who indicates the error and how should frontend handle that?

I edited the text a bit to be more to my liking #1766

@brettcannon
Copy link
Member

If we don't hear anything from anyone July 12 then I consider marking this final as decided.

@ncoghlan
Copy link
Contributor

If there are any fixes needed to the config settings part, I think those would be better handled as a PEP revising the spec on packaging.python.org (with a new PEP delegate), so +1 from me for marking this iteration final.

@brettcannon brettcannon merged commit a8e7358 into python:master Jun 30, 2021
@brettcannon
Copy link
Member

Thanks, Nick! PR is merged!

@abitrolly
Copy link
Contributor

I feel ignored.

@pfmoore pfmoore deleted the pep517_final branch July 1, 2021 07:32
@encukou
Copy link
Member

encukou commented Jul 1, 2021

@abitrolly, this repo not the place to discuss changes to PEP text (as opposed to changing the status), so your comment here is not read by the relevant people. Please raise your concerns at https://discuss.python.org/c/packaging/14 . (You could also use [email protected], as mentioned in the PEP header, but that list isn't all that active now; AFAIK the PEP predates the discuss.python.org site.)

@ncoghlan
Copy link
Contributor

ncoghlan commented Jul 1, 2021

My comment about any changes at this stage requiring a new PEP was specifically related to the concerns already raised. The opportunity to amend 517 itself without needing a migration plan passed years ago, when it was declared unprovisional.

@abitrolly
Copy link
Contributor

@encukou that GitHub process can be improved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants