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

[READY] Drop Python 2.6 and Python 3.3 support #904

Merged
merged 1 commit into from
Jan 31, 2018

Conversation

bstaletic
Copy link
Collaborator

@bstaletic bstaletic commented Jan 22, 2018

This drops support for python versions that have reached their end of life.
Python 2.6 has reached its end of life in 2013-10-29.
Python 3.3 has reached its end of life in 2017-09-29.
Reference: https://devguide.python.org/#status-of-python-branches

Besides dropping python version, this also does two more things:

  • Updates waitress to version v1.1.0 - this protects us from this vulnerability.
  • Removes the argparse submodule, because it has been a part of the system library since Python 2.7.

The pull request is tagged RFC because you might disagree with some things I've done, or we might decide to split this in more than one PR.

Once we're satisfied with the PR state, the commits will be squashed.


This change is Reviewable

@codecov-io
Copy link

codecov-io commented Jan 22, 2018

Codecov Report

Merging #904 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #904      +/-   ##
==========================================
- Coverage    96.2%   96.19%   -0.01%     
==========================================
  Files          83       83              
  Lines        6500     6498       -2     
==========================================
- Hits         6253     6251       -2     
  Misses        247      247

@bstaletic
Copy link
Collaborator Author

The coverage isn't wrong. At least not completely.
The import in the clang_completer.py is correct.

I believe the part in flags.py is wrong.

Also, I told @Manishearth, in servo/homu#144 to watch over this PR so he can try to debug homu. So we probably should notify him once again before merging, to make sure he can do what he planned.


Review status: 0 of 10 files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@Manishearth
Copy link
Contributor

To be clear, I'm not watching, I don't have the time and I'm currently in a bad timezone. I was suggesting y'all add more logging to your running homu instance and see what crops up.

@bstaletic
Copy link
Collaborator Author

Travis flaked because of ycmd.tests.java.server_management_test.ServerManagement_RestartServer_test.

As for the coverage, the clang_completer.py is fixed, but flags.py is still being weird.


Review status: 0 of 11 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@bstaletic bstaletic force-pushed the drop_python26_and_python33 branch from e15b281 to 325f22d Compare January 23, 2018 15:09
@micbou
Copy link
Collaborator

micbou commented Jan 23, 2018

Reviewed 4 of 5 files at r1, 6 of 6 files at r2, 1 of 1 files at r3, 2 of 2 files at r4, 1 of 1 files at r5, 1 of 1 files at r6, 6 of 6 files at r7.
Review status: all files reviewed at latest revision, 3 unresolved discussions.


build.py, line 30 at r7 (raw file):

         ( PY_MAJOR == 3 and PY_MINOR >= 3 ) or
         PY_MAJOR > 3 ):
  sys.exit( 'ycmd requires Python >= 2.6 or >= 3.3; '

We need to update that condition and the error message that comes with it. We could rewrite the message to "ycmd requires Python 2.7 or 3.4+".


.circleci/install_dependencies.sh, line 57 at r2 (raw file):

  PYENV_VERSION="2.7.8"
else
  PYENV_VERSION="3.4.5"

Any reason to pick 3.4.5? We could pick the last version of the 3.4 series instead: 3.4.7.


ci/travis/travis_install.sh, line 54 at r2 (raw file):

if [ "${YCMD_PYTHON_VERSION}" == "2.7" ]; then
  # We need a recent enough version of Python 2.7 on OS X or an error occurs
  # when installing the psutil dependency for our tests.

I think we can remove this comment since we only use Travis for Linux now.


Comments from Reviewable

@bstaletic
Copy link
Collaborator Author

Review status: 16 of 21 files reviewed at latest revision, 3 unresolved discussions.


build.py, line 30 at r7 (raw file):

Previously, micbou wrote…

We need to update that condition and the error message that comes with it. We could rewrite the message to "ycmd requires Python 2.7 or 3.4+".

Done.


.circleci/install_dependencies.sh, line 57 at r2 (raw file):

Previously, micbou wrote…

Any reason to pick 3.4.5? We could pick the last version of the 3.4 series instead: 3.4.7.

Done.

For some reason I thought the 3.4.5 was the latest. Should we also update 2.7.8 to 2.7.14?


ci/travis/travis_install.sh, line 54 at r2 (raw file):

Previously, micbou wrote…

I think we can remove this comment since we only use Travis for Linux now.

Done.


Comments from Reviewable

@bstaletic
Copy link
Collaborator Author

Review status: 16 of 21 files reviewed at latest revision, 3 unresolved discussions, some commit checks broke.


ci/travis/travis_install.sh, line 55 at r8 (raw file):

  PYENV_VERSION="2.7.8"
else
  PYENV_VERSION="3.4.7"

Did this line screw up travis?
Is 3.4.7 somehow unavailable?
Travis says: python-build: definition not found: 3.4.7


Comments from Reviewable

@bstaletic bstaletic force-pushed the drop_python26_and_python33 branch from 80a9629 to f48b94d Compare January 23, 2018 19:36
@micbou
Copy link
Collaborator

micbou commented Jan 23, 2018

Reviewed 2 of 5 files at r8, 1 of 4 files at r9.
Review status: all files reviewed at latest revision, 1 unresolved discussion, some commit checks broke.


ci/travis/travis_install.sh, line 55 at r8 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

Did this line screw up travis?
Is 3.4.7 somehow unavailable?
Travis says: python-build: definition not found: 3.4.7

We need to update the version of pyenv. Replace the line git checkout v1.0.8 with git checkout v1.2.1 just above.


Comments from Reviewable

@bstaletic
Copy link
Collaborator Author

Review status: 17 of 18 files reviewed at latest revision, 1 unresolved discussion.


ci/travis/travis_install.sh, line 55 at r8 (raw file):

Previously, micbou wrote…

We need to update the version of pyenv. Replace the line git checkout v1.0.8 with git checkout v1.2.1 just above.

Done.


Comments from Reviewable

@micbou
Copy link
Collaborator

micbou commented Jan 24, 2018

Reviewed 1 of 1 files at r10.
Review status: all files reviewed at latest revision, 2 unresolved discussions.


CONTRIBUTING.md, line 105 at r10 (raw file):

======================================

We support Python 2.7 and 3.3+. Since we use

Replace 3.3 with 3.4.


.circleci/install_dependencies.sh, line 57 at r2 (raw file):

Should we also update 2.7.8 to 2.7.14?

We could.


Comments from Reviewable

@bstaletic
Copy link
Collaborator Author

Review status: 15 of 18 files reviewed at latest revision, 2 unresolved discussions.


CONTRIBUTING.md, line 105 at r10 (raw file):

Previously, micbou wrote…

Replace 3.3 with 3.4.

As I said, I shouldn't be multitasking.

Done.


.circleci/install_dependencies.sh, line 57 at r2 (raw file):

Previously, micbou wrote…

Should we also update 2.7.8 to 2.7.14?

We could.

Done.


Comments from Reviewable

@micbou
Copy link
Collaborator

micbou commented Jan 24, 2018

:lgtm:

@bstaletic Will you prepare a similar PR for YCM repository?


Reviewed 3 of 3 files at r11.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@bstaletic
Copy link
Collaborator Author

I will, but my plan was not to merge this until @Valloric gets around to adding more logging statements for @zzbot, so that we can find out what's been going on with the double builds.


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@Valloric
Copy link
Member

@bstaletic @Manishearth Not sure it would be easy (or a good idea) to change our zzbot instance since it's running vanilla homu code. There's nothing custom to it. If more logging should be added to debug our issue, we should probably add this logging code to the real homu codebase (at DEBUG level, so off by default). Then others in the future could also benefit from this.

Hacking around our checkout of homu seems... iffy to me.

WRT deprecating older version of python, dropping 3.3 is easy. Dropping 2.6 is more complicated. We'll definitely break people by doing it. OTOH, it did reach end-of-life 5 years ago... I think it's OK.

@puremourning Thoughts? You also have experience with corps moving at the corp-standard glacial pace of updating.


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@bstaletic
Copy link
Collaborator Author

@Valloric I've discussed dropping python2.6 with @puremourning and @micbou before making this PR and both were fine with dropping it.

I'll mark this PR as [READY].


Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@bstaletic bstaletic changed the title [RFC] Drop Python 2.6 and Python 3.3 support [READY] Drop Python 2.6 and Python 3.3 support Jan 24, 2018
@bstaletic bstaletic force-pushed the drop_python26_and_python33 branch 2 times, most recently from 79403fd to 58e0aec Compare January 26, 2018 15:25
@micbou
Copy link
Collaborator

micbou commented Jan 26, 2018

Reviewed 3 of 3 files at r12.
Review status: all files reviewed at latest revision, 1 unresolved discussion.


CONTRIBUTING.md, line 105 at r10 (raw file):

Previously, bstaletic (Boris Staletic) wrote…

As I said, I shouldn't be multitasking.

Done.

It's 3.3 again.


Comments from Reviewable

@bstaletic bstaletic force-pushed the drop_python26_and_python33 branch from 58e0aec to 33b04c9 Compare January 26, 2018 22:03
@bstaletic
Copy link
Collaborator Author

Review status: 17 of 18 files reviewed at latest revision, 1 unresolved discussion.


CONTRIBUTING.md, line 105 at r10 (raw file):

Previously, micbou wrote…

It's 3.3 again.

I somehow messed up the branch, I don't know what happened.

Done.


Comments from Reviewable

@Valloric
Copy link
Member

If everyone is onboard with this, I am :lgtm: as well.


Review status: 17 of 18 files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@bstaletic
Copy link
Collaborator Author

That means I can submit the PR for YouCompleteMe as well! 👍


Review status: 17 of 18 files reviewed at latest revision, 1 unresolved discussion.


Comments from Reviewable

@micbou
Copy link
Collaborator

micbou commented Jan 31, 2018

Thanks for taking care of that.

:lgtm_strong:

@zzbot r+


Reviewed 1 of 1 files at r13.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@zzbot
Copy link
Contributor

zzbot commented Jan 31, 2018

📌 Commit 33b04c9 has been approved by micbou

zzbot added a commit that referenced this pull request Jan 31, 2018
[READY] Drop Python 2.6 and Python 3.3 support

This drops support for python versions that have reached their end of life.
Python 2.6 has reached its end of life in 2013-10-29.
Python 3.3 has reached its end of life in 2017-09-29.
Reference: https://devguide.python.org/#status-of-python-branches

Besides dropping python version, this also does two more things:
- Updates waitress to version v1.1.0 - this protects us from [this vulnerability](https://www.djangoproject.com/weblog/2015/jan/13/security/).
- Removes the argparse submodule, because it has been a part of the system library since Python 2.7.

The pull request is tagged RFC because you might disagree with some things I've done, or we might decide to split this in more than one PR.

Once we're satisfied with the PR state, the commits will be squashed.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/904)
<!-- Reviewable:end -->
@zzbot
Copy link
Contributor

zzbot commented Jan 31, 2018

⌛ Testing commit 33b04c9 with merge cbc489c...

@zzbot
Copy link
Contributor

zzbot commented Jan 31, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: micbou
Pushing cbc489c to master...

@zzbot zzbot merged commit 33b04c9 into ycm-core:master Jan 31, 2018
zzbot added a commit to ycm-core/YouCompleteMe that referenced this pull request Feb 1, 2018
[READY] Drop Python 2.6 and Python 3.3

# PR Prelude

Thank you for working on YCM! :)

**Please complete these steps and check these boxes (by putting an `x` inside
the brackets) _before_ filing your PR:**

- [x] I have read and understood YCM's [CONTRIBUTING][cont] document.
- [x] I have read and understood YCM's [CODE_OF_CONDUCT][code] document.
- [x] I have included tests for the changes in my PR. If not, I have included a
  rationale for why I haven't.
- [x] **I understand my PR may be closed if it becomes obvious I didn't
  actually perform all of these steps.**

# Why this change is necessary and useful

[Please explain **in detail** why the changes in this PR are needed.]

Since we are dropping support for Python 2.6 and 3.3 in ycm-core/ycmd#904 we should drop support for them in YCM as well.

[cont]: https://github.com/Valloric/YouCompleteMe/blob/master/CONTRIBUTING.md
[code]: https://github.com/Valloric/YouCompleteMe/blob/master/CODE_OF_CONDUCT.md

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2887)
<!-- Reviewable:end -->
zzbot added a commit to ycm-core/YouCompleteMe that referenced this pull request Feb 10, 2018
[READY] Update ycmd

Include the following changes:

 - PR ycm-core/ycmd#789: add support for Windows flags when --driver-mode=cl is given;
 - PR ycm-core/ycmd#848: hide C++ symbols by default;
 - PR ycm-core/ycmd#857: add Java support using jdt.ls;
 - PR ycm-core/ycmd#861: translate libclang error codes to exceptions;
 - PR ycm-core/ycmd#880: support downloading Clang binaries on ARM systems;
 - PR ycm-core/ycmd#883: handle zero column diagnostic from OmniSharp;
 - PR ycm-core/ycmd#884: specify Platform property when compiling OmniSharp;
 - PR ycm-core/ycmd#886: use current working directory in JavaScript completer;
 - PR ycm-core/ycmd#887: update Boost to 1.66.0;
 - PR ycm-core/ycmd#888: update JediHTTP;
 - PR ycm-core/ycmd#889: update Clang to 5.0.1;
 - PR ycm-core/ycmd#891: fix building with system libclang on Gentoo amd64;
 - PR ycm-core/ycmd#904: drop Python 2.6 and Python 3.3 support;
 - PR ycm-core/ycmd#905: calculate the start column when items are not resolved in the language server completer;
 - PR ycm-core/ycmd#912: download Clang binaries from HTTPS;
 - PR ycm-core/ycmd#914: do not try to symlink libclang on Windows.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/youcompleteme/2902)
<!-- Reviewable:end -->
@bstaletic bstaletic deleted the drop_python26_and_python33 branch May 22, 2018 04:41
zzbot added a commit that referenced this pull request Sep 9, 2018
[READY] Do not add third-party argparse path in scripts

Bundled argparse was removed in PR #904.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/valloric/ycmd/1089)
<!-- Reviewable:end -->
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.

6 participants