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

drop support for python 2.7 #245

Closed
samuelcolvin opened this issue Jan 10, 2020 · 34 comments
Closed

drop support for python 2.7 #245

samuelcolvin opened this issue Jan 10, 2020 · 34 comments

Comments

@samuelcolvin
Copy link

It looks like cibuildwheel has a significant amount of logic dedicated to python 2.7.

Isn't it time to drop support for 2.7 and simplify the library?

@Czaki
Copy link
Contributor

Czaki commented Jan 16, 2020

Being the last of the 2.x series, 2.7 will receive bugfix support until 2020. Support officially stops January 1 2020, but the final release will occur after that date.

Planned future release dates:

2.7.18 code freeze January, 2020
2.7.18 release candidate early April, 2020
2.7.18 mid-April, 2020

from https://www.python.org/dev/peps/pep-0373/

@samuelcolvin
Copy link
Author

humm, so when would you suggest cibuildwheel can drop 2.7 support?

Many other libraries have already dropped python 2 and rely on prior releases for those still (inexplicably) using python 2.7.

@YannickJadoul
Copy link
Member

Personally, I think it would be nice to have one final Python 2.7 compatible release with the current merged PRs and some of the pending ones.

Note also that PyPy (which we'll support soon: #185) won't drop 2.7 support (since large parts of their toolchain are built upon the 2.7 version of the language), so I'm not sure we can remove all the code that relates to 2.7.

@Czaki
Copy link
Contributor

Czaki commented Jan 16, 2020

I agree. Also there is Tauthon which may be supported in future.

@YannickJadoul
Copy link
Member

YannickJadoul commented Jan 16, 2020

Urgh, "Tauthon"; what a silly pun :-D

EDIT: It might depend a bit on how widely it will be used. I'm not a big fan of adding a compilation step for Tauthon in cibuildwheel.

@Czaki
Copy link
Contributor

Czaki commented Jan 16, 2020

Adding Thauton is an option only if they provide bundled version for all systems. Not when it need to be compiled from source.

@joerick
Copy link
Contributor

joerick commented Jan 17, 2020

Thanks for raising this @samuelcolvin!

I'd be open to supporting Python 2 wheels for a little while longer, but curious to opinions.

I had thought that we'd get a good, stable release of cibuildwheel out, and then would refer anyone who needs Python 2 support to that version. But... now that I think about it, it would be good to get deterministic builds #239 in before this! At some point, that pinned version of cibuildwheel will be pulling in dependencies that don't support or are buggy on Python 2, for example, the manylinux images, setuptools, or virtualenv.

However we could definitely remove Python 2 support for running cibuildwheel itself now, I think it's totally fine to require it on the build machine.

edit: @YannickJadoul's point on PyPy 2.7 is a good one - we won't be dropping support for that Python 2.7 - it's still actively maintained.

@jtrmal
Copy link

jtrmal commented Jan 25, 2020

For what it is worth coming from someone just passing by, Py 2.7 won't disappear in two weeks. There will still be (IMO) maintainers, especially in the LTS distributions. So my opinion is to support Py2.7 as long as (reasonably) feasible.

@YannickJadoul
Copy link
Member

YannickJadoul commented Jan 30, 2020

I'd very happily drop the support for running cibuildwheel with Python 2, though; any thoughts on that?

It would reduce the amount of tests to run, and avoid annoyances like this: https://circleci.com/gh/joerick/cibuildwheel/3212

@jtrmal
Copy link

jtrmal commented Jan 30, 2020

yeah, that makes sense to me. In our projects, we stopped accepting py2 code and enforce py3 for new addition.

@joerick
Copy link
Contributor

joerick commented Jan 30, 2020

I'd very happily drop the support for running cibuildwheel with Python 2, though; any thoughts on that?

Agreed. Will reduce code clutter, and reduce CI load. Let's do it.

@YannickJadoul
Copy link
Member

Agreed. Will reduce code clutter, and reduce CI load. Let's do it.

Yesssss; working on it!

@joerick
Copy link
Contributor

joerick commented Feb 2, 2020

Wishlist for the Python 3 modernisation:

  • remove run_docker in linux.py (replace with subprocess.run)
  • removing these:
    try:
        from urllib.request import urlopen
    except ImportError:
        from urllib2 import urlopen
  • consider using f-strings for the bash script in linux.py (?)

@Czaki
Copy link
Contributor

Czaki commented Feb 2, 2020

add type annotation?

consider using f-strings for the bash script in linux.py (?)

It means also drop for running on python 3.5?

@samuelcolvin
Copy link
Author

surely everyone running cibuildwheel has access to python 3.6 or above?

@YannickJadoul
Copy link
Member

Wishlist for the Python 3 modernisation:

Great, thanks! I'll try to get that first PR ready soon, so we can all have a further look at what can be better.

surely everyone running cibuildwheel has access to python 3.6 or above?

Python 3.5 is still officially supported, so I don't want to blame people for still using it.

@samuelcolvin
Copy link
Author

perhaps.

Another big gain would be using pathlib instead of all the os.path.*. I think this was why I originally suggested dropping python 2.

@Czaki
Copy link
Contributor

Czaki commented Feb 2, 2020

@YannickJadoul Maybe wait with this PR since #185 is merged. These changes will be big.

@joerick
Copy link
Contributor

joerick commented Feb 2, 2020

Another big gain would be using pathlib instead of all the os.path.*. I think this was why I originally suggested dropping python 2.

Interesting suggestion - I've not had much experience with these path objects. How do they work when using them with subprocess.run() for example? My experience has been the need to throw str(path) all over the place, but I might have been using it wrong.

@YannickJadoul
Copy link
Member

Another big gain would be using pathlib instead of all the os.path.*. I think this was why I originally suggested dropping python 2.

Ah, yes, thanks for the suggestion!

@YannickJadoul Maybe wait with this PR since #185 is merged. These changes will be big.

Ugh, it's true, yes :-/ Would you have some time to look at #185, maybe, @Czaki, just to check if the rebase on top of #156 and #220 (which caused lots of merge conflicts) are not too bad?

@samuelcolvin
Copy link
Author

How do they work when using them with subprocess.run() for example?

Unfortunately not perfectly, i think here and elsewhere you do need str()

However that's more than made up for with .isfile() etc. and join using the / operator.

@Czaki
Copy link
Contributor

Czaki commented Feb 2, 2020

@YannickJadoul I do not understand your request. Can you wrote it another way?

@YannickJadoul
Copy link
Member

@YannickJadoul I do not understand your request. Can you wrote it another way?

I meant to ask if you have time to review #185, maybe. I needed to rebase it two times, after #156 and #220. And it was a big rebase, these two times, with lots of merge conflicts. So maybe I you would still find some mistakes?

@Czaki
Copy link
Contributor

Czaki commented Feb 2, 2020

I left some comments. I do not see any important problem.

@YannickJadoul
Copy link
Member

if anyone else here wants to have a look and find things @joerick, @Czaki, and I haven't thought of yet in #265, feel free to let us know! Except for f-strings (I'd love them, but I'd argue that's only in after 3.5 gets dropped) and type annotations (that's another discussion, #258), that is.

@YannickJadoul
Copy link
Member

Oh, pathlib. I should still have a look at that, yes!

@Czaki
Copy link
Contributor

Czaki commented Feb 3, 2020

Due to pathlib i would rather modify shell/call function to check arguments and convert to string than use str everywhere.

@YannickJadoul
Copy link
Member

See #265 (comment)

I argue against pathlib until we can drop 3.5.

@Czaki
Copy link
Contributor

Czaki commented Feb 4, 2020

This what I think is to change ex.:

    def call(args, env=None, cwd=None, shell=False):
        # print the command executing for the logs
        if shell:
            print('+ %s' % args)
        else:
            print('+ ' + ' '.join(shlex.quote(a) for a in args))

        return subprocess.check_call(args, env=env, cwd=cwd, shell=shell)

to

    def call(args, env=None, cwd=None, shell=False):
        # print the command executing for the logs
        args = [str(x) if isinstance(x, pathlib.Path) else x for x in args]
        if shell:
            print('+ %s' % args)
        else:
            print('+ ' + ' '.join(shlex.quote(a) for a in args))

        return subprocess.check_call(args, env=env, cwd=cwd, shell=shell)

@YannickJadoul
Copy link
Member

This what I think is to change ex.:

I know. But it is not enough. shutil.rmtree, os.chdir, ... also all cannot take pathlib.Path arguments in Python 3.5, because PEP 519 is only implemented from Python 3.6 onwards. Believe me, I tried for half an hour, yesterday; it's really more than just that function.

@joerick
Copy link
Contributor

joerick commented Feb 10, 2020

We have dropped support for Python 2.7 on the host side. For the moment, we'll continue to support building CPython 2.7 wheels, though I expect we'll stop supporting that in a future release at some point this year.

@joerick joerick closed this as completed Feb 10, 2020
@Czaki
Copy link
Contributor

Czaki commented Feb 10, 2020

There is any reason to drop support to build cPython 2.7 build if there is plan to support pypy 2.7?

@EricCousineau-TRI
Copy link

EricCousineau-TRI commented Jul 10, 2020

Note also that PyPy [...] won't drop 2.7 support (since large parts of their toolchain are built upon the 2.7 version of the language) [...]

Reference contributed by the peanut gallery (me): https://doc.pypy.org/en/latest/faq.html#how-long-will-pypy-support-python2

@YannickJadoul
Copy link
Member

Note also that PyPy [...] won't drop 2.7 support (since large parts of their toolchain are built upon the 2.7 version of the language) [...]

Reference contributed by the peanut gallery (me): https://doc.pypy.org/en/latest/faq.html#how-long-will-pypy-support-python2

Yeah. There might be a point at which PyPy 2.7 itself is not really used anymore by users (if most code has become Python 3-targeted, because CPython 2 is dead), and when building library wheels for PyPy 2.7 becomes useless?

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

No branches or pull requests

6 participants