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

Revert "improve encoding handling for setup.cfg" #1656

Conversation

benoit-pierre
Copy link
Member

Reverts #1180, fix #1653. #1180 was initially deferred until #417 is handled, with good reason: I see no clean way to work around the lack of Unicode awareness in distutils.ccompiler. We either need to resolve #417, or wait for Python 2 support to be dropped.

@jaraco
Copy link
Member

jaraco commented Jan 28, 2019

This might be our only option here... although if there's a shorter path to stability, I'd like to selectively adopt the portions of distutils that are broken by #1180. In other words, could setuptools adopt distutils.ccompiler and enable that for use, addressing the str issue the same way I did in b1615d1 and 249f24a?

@jaraco
Copy link
Member

jaraco commented Jan 28, 2019

Another approach might be to, when parsing options, instead of allowing unicode values to pass, on Python 2 convert those to str where possible.

@benoit-pierre
Copy link
Member Author

It would be simpler to either, disable encoding support, and keep using binary for Python 2, or defer support until Python 2 support is dropped.

@benoit-pierre
Copy link
Member Author

Do we plan to drop Python 2 support ahead of the 2020 EOL deadline?

@jaraco
Copy link
Member

jaraco commented Jan 28, 2019

Do we plan to drop Python 2 support ahead of the 2020 EOL deadline?

I'd like to drop support for Python 2.7 as soon as possible, but because setuptools is at the bottom of the toolchain (literally almost everything depends on it), it probably needs to be one of the last things that drops support. I'm okay with dropping support and leaving Python 2-compatible projects to pin to older setuptools, but I'd like to do that with a fairly stable feature set.

As I understand it, the encoding support is required to fully support even basic features like src layouts for declarative config... which are documented but broken (#1136). I'd really like fix for that before dropping support for 2.7.


I'd like to selectively adopt the portions of distutils

Reading the OP in the ticket, I see that it may not be possible to override the behavior because the project that's failing (Pillow) is importing distutils.command.build_ext directly--there's no hook for setuptools to even get involved, so I don't think this approach is viable.

It would be simpler to either, disable encoding support, and keep using binary for Python 2, or defer support until Python 2 support is dropped.

I'd be okay with this, but wouldn't it be even better to support unicode (where it doesn't cause failures) and downcast to str only when unicode values aren't present?

@pganssle
Copy link
Member

Regarding dropping Python 2.7, there's this thread on the discourse about it.

I agree with Jason - if Python packaging were relatively stable we could rely on python_requires and just go for it, but I think Python 2.7 will EOL before we get setuptools to a point where we can confidently say that we can drop support.

That said, it wouldn't be unreasonable to support Python 2.7 on a "best effort" basis. As long as we're not actively breaking Python 2.7, if it's significantly harder to get a bugfix to work for both 2 and 3, I'm fine with just fixing it for 3 and leaving it open for 2.

@ofek
Copy link
Sponsor Contributor

ofek commented Jan 28, 2019

This is breaking our master builds too on py2 https://travis-ci.com/DataDog/integrations-core/jobs/173502395#L814

@benoit-pierre
Copy link
Member Author

Added a simple regression test.

@benoit-pierre
Copy link
Member Author

Which of course is failing on Windows... sigh

@jaraco
Copy link
Member

jaraco commented Jan 28, 2019

Added a simple regression test.

@benoit-pierre If you have a regression test, would you consider pushing it as a branch against 40.7.0 so it might be merged with whatever fix we settle on?

@benoit-pierre benoit-pierre force-pushed the revert-1180-fix_889_and_non-ascii_in_setup.cfg_take_2 branch from dd69c1c to d703350 Compare January 28, 2019 23:01
@benoit-pierre
Copy link
Member Author

@jaraco: the regression test should be fixed now, I suggest just cherry-picking d703350 into #1660.

@jaraco
Copy link
Member

jaraco commented Jan 28, 2019

I suggest just cherry-picking

Will do. Thanks!

@jaraco
Copy link
Member

jaraco commented Jan 29, 2019

Thanks for working on this, but I think #1660 gets us closer to goal, so I'm going to go with that approach.

@jaraco jaraco closed this Jan 29, 2019
@jaraco jaraco deleted the revert-1180-fix_889_and_non-ascii_in_setup.cfg_take_2 branch July 1, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: 'output_dir' must be a string or None since setuptools 40.7.0 Adopt distutils
4 participants