10249 Use enviroment markers for extra all

[McSinyx]

Scope and purpose

This will make it valid to install every extra on any platform. This sounds odd but it is a criteria we are experimentally mandate in IPWHL, a WIP downstream repository for Python packages.

Contributor Checklist:

• The associated ticket in Trac is here: https://twistedmatrix.com/trac/ticket/10249
• I ran tox -e lint to format my patch to meet the Twisted Coding Standard
• I have created a newsfragment in src/twisted/newsfragments/ (see: News files)
• The title of the PR starts with the associated Trac ticket number (without the # character).
• I have updated the automated tests and checked that all checks for the PR are green.
• I have submitted the associated Trac ticket for review by adding the word review to the keywords field in Trac, and putting a link to this PR in the comment; it shows up in https://twisted.reviews now.
• The merge commit will use the below format
  The first line is automatically generated by GitHub based on PR ID and branch name.
  The other lines generated by GitHub should be replaced.

Merge pull request #123 from twisted/4356-branch-name-with-trac-id

Author: <github_username>, <github_usernames_if_more_authors>
Reviewer: <github_username>, <github_username_if_more_reviewers>
Fixes: ticket:<trac_ticket_number>, ticket:<another_if_more_in_one_PR>

Long description providing a summary of these changes.
(as long as you wish)

[McSinyx]

Gentle ping!

[adiroiban]

Thanks for the update. I think that this is an important one.

I think that all_non_platform should be kept as it is (with test removed) and not install pywin32.

and maybe create something like _platform_specific and add it to the new all toge

all_non_platform =
  AS_BEFORE

_platform_specific =
    pyobjc-core; platform_system == "Darwin"
    pyobjc-framework-CFNetwork; platform_system == "Darwin"
    pyobjc-framework-Cocoa; platform_system == "Darwin"
    pywin32 != 226; platform_system == "Windows"

all =
    %(all_non_platform)s
    %(_platform_specifics)s

Does it make sense?

The concern raised by Thomas is that maybe you are already calling pip twisted[all_non_platform] on pypy on windows or macos and you might end up with a failure.

---

We don't have pypy test runs on macOS or Windows (I think that we should have at least one) so I don't know if that is the case.

But I think that is best to play it safe and keep all_non_platform is it was before.

Then we can add all or standard or any new extra :)

[McSinyx]

Sorry for the late reply, do you think it would be preferable to use the macos_platform and macos_platform extra name or _platform_specific should be the way to go?

[installing] on pypy on windows or macos and you might end up with a failure.

Yup pyobjc and pywin32 looks like CPython extension modules, I think I'll go head and add platform_python_implementation == "CPython" to the marker.

[glyph]

Merging changes back to the branch appears to be disabled so I can't update it for current trunk.

[McSinyx]

I've rebased it on top of trunk.

[twm]

I had a minor note, but even so this looks like a real improvement and I'd like to merge it. Unfortunately it has drifted from trunk once more. Could you merge/rebase again?

[adiroiban]

The changes looks good.

We have the deprecation info in the release notes. That is great.

We also raises deprecation warnings for things that are deprecated... so that people will that don't read the release notes will know what was changed.

Is there an easy way in which a deprecation error can be raised for using a specific extra?

I guess that we can try to keep them deprecated and without removal for as long as we can.

I guess that you can include a package that once installed it will trigger a deprecation warning...but looks complicated.

---

so @graingert any comments on the current PR?

Can it be merged?

Thanks!

[twm]

@adiroiban I think the hold-up here is entirely that Twisted maintainers can't merge/rebase it easily, so it drifts out of sync with trunk. (Well, and now there are conflicts.) I'd have merged it a month ago otherwise.

[adiroiban]

Many thanks for the update.

As commented by Thomas over gitter, all_non_platform should also install test as it was doing in the past to keep backward compatibility.

I left more comments inline.

[adiroiban]

@graingert ... so. Thomas. Anything bad with this PR? Are you happy to see it merged?

Right now the PR is blocked as you have previously rejected the changes.

Thanks!

[glyph]

Sorry to see that this has drifted out of sync with trunk yet again, probably due to the big packaging overhaul. Hopefully that means that this will be easier to do, if somewhat annoying that it all needs to be done again.

[glyph]

Not the responsibility of this change, but, I think that we should not actually re-enumerate all dependencies in this document any more. Just list the extras, and let the package metadata speak for itself if people want to know specifics. We can't necessarily keep this up to the minute up-to-date if someone adds a transitive dependency, for example.

[glyph]

Note that you can ask the robot for a re-review at any time by saying "p-l-e-a-s-e review" without the dashes.