From bb41fd7dabe3a7dbec5b6c49da8fc38747cc4bad Mon Sep 17 00:00:00 2001 From: "Mr. Senko" Date: Fri, 9 Mar 2018 12:34:56 +0200 Subject: [PATCH 1/7] Revert "List Django as a dependency. Fix #96" This reverts commit 9b520e9b7a08162e9b92c198ee4e2afaaea9d310. Apparently this is causing more problems than it solves, see: https://github.com/PyCQA/pylint-django/pull/132 --- README.md | 32 +++++++++----------------------- setup.py | 1 - 2 files changed, 9 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index af8029b6..ff1185f5 100644 --- a/README.md +++ b/README.md @@ -10,33 +10,19 @@ pylint-django `pylint-django` is a [Pylint](http://pylint.org) plugin for improving code analysis for when analysing code using Django. It is also used by the [Prospector](https://github.com/landscapeio/prospector) tool. -# Installation +## Usage -``` -pip install pylint-django -``` - -**WARNING:** `pylint-django` requires `Django` to be installed. Our `setup.py` file doesn't -specify which particular version of Django is going to be installed because we have no idea -what version is used inside your project. The latest version of Django will be installed if -it has not been installed beforehand! DO NOT report issues about mismatching Django versions -if that happens. Instead get your testing environment sorted out and make sure that you have -the appropriate version of Django installed! - -# Usage - -## Pylint +#### Pylint -Ensure `pylint-django` is installed and on your path and then execute: +Ensure `pylint-django` is installed and on your path (`pip install pylint-django`), and then run pylint: ``` pylint --load-plugins pylint_django [..other options..] ``` -## Prospector +#### Prospector -If you have `prospector` installed, then `pylint-django` will already be installed as a dependency, -and will be activated automatically if Django is detected. +If you have `prospector` installed, then `pylint-django` will already be installed as a dependency, and will be activated automatically if Django is detected. ``` prospector [..other options..] @@ -56,14 +42,14 @@ Please feel free to add your name to the `CONTRIBUTORS.md` file if you want to b credited when pull requests get merged. You can also add to the `CHANGELOG.md` file if you wish, although I'll also do that when merging if not. -# Tests +## Tests The structure of the test package follows that from pylint itself. It is fairly simple: create a module starting with `func_` followed by a test name, and insert into it some code. The tests will run pylint -against these modules. If the idea is that no messages now occur, then -that is fine, just check to see if it works by running `scripts/test.sh`. + against these modules. If the idea is that no messages now occur, then + that is fine, just check to see if it works by running `scripts/test.sh`. Ideally, add some pylint error suppression messages to the file to prevent spurious warnings, since these are all tiny little modules not designed to @@ -78,4 +64,4 @@ These are useful to quickly add "expected messages". # License -`pylint-django` is available under the GPLv2 license. +`pylint-django` is available under the GPLv2 license. \ No newline at end of file diff --git a/setup.py b/setup.py index b6f77185..d19ad9e7 100644 --- a/setup.py +++ b/setup.py @@ -18,7 +18,6 @@ install_requires=[ 'pylint-plugin-utils>=0.2.1', 'pylint>=1.8.2', - 'Django', ], license='GPLv2', classifiers=[ From 9aae2fe323a44761532a58e2aff5795ed4dfa57b Mon Sep 17 00:00:00 2001 From: "Mr. Senko" Date: Fri, 9 Mar 2018 12:46:23 +0200 Subject: [PATCH 2/7] Add optional dependency for Django and update README This commit adds an optional dependency for Django for users who would like to have Django installed automatically when installing pylint-django. It also updates the text in README and adds an ISSUE_TEMPLATE which warns the users not to report bugs if Django is not installed automatically! --- .github/ISSUE_TEMPLATE.md | 2 ++ README.md | 18 ++++++++++++++++++ setup.py | 3 +++ 3 files changed, 23 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE.md diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md new file mode 100644 index 00000000..9fe964ad --- /dev/null +++ b/.github/ISSUE_TEMPLATE.md @@ -0,0 +1,2 @@ +**WARNING:** Please do not report issues about missing Django, see +[README](https://github.com/PyCQA/pylint-django#installation)! diff --git a/README.md b/README.md index ff1185f5..7613ec2b 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,24 @@ pylint-django `pylint-django` is a [Pylint](http://pylint.org) plugin for improving code analysis for when analysing code using Django. It is also used by the [Prospector](https://github.com/landscapeio/prospector) tool. +# Installation + +``` +pip install pylint-django +``` + +**WARNING:** `pylint-django` will not install `Django` by default because this +causes more trouble than good, +[see discussion](https://github.com/PyCQA/pylint-django/pull/132). If you wish +to automatically install the latest version of `Django` then + +``` +pip install pylint-django[with_django] +``` + +otherwise sort out your testing environment and please **DO NOT** report issues +about missing Django! + ## Usage #### Pylint diff --git a/setup.py b/setup.py index d19ad9e7..f0cbdf06 100644 --- a/setup.py +++ b/setup.py @@ -19,6 +19,9 @@ 'pylint-plugin-utils>=0.2.1', 'pylint>=1.8.2', ], + extras_require={ + 'with_django': ['Django'], + }, license='GPLv2', classifiers=[ 'Development Status :: 4 - Beta', From 9cb74343b29747176745adee4359b24ad14bf69e Mon Sep 17 00:00:00 2001 From: "Mr. Senko" Date: Fri, 9 Mar 2018 12:50:33 +0200 Subject: [PATCH 3/7] README formatting --- README.md | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 7613ec2b..eaa3e3e3 100644 --- a/README.md +++ b/README.md @@ -28,19 +28,18 @@ pip install pylint-django[with_django] otherwise sort out your testing environment and please **DO NOT** report issues about missing Django! -## Usage +# Usage -#### Pylint - -Ensure `pylint-django` is installed and on your path (`pip install pylint-django`), and then run pylint: +Ensure `pylint-django` is installed and on your path and then execute: ``` -pylint --load-plugins pylint_django [..other options..] +pylint --load-plugins pylint_django [..other options..] ``` -#### Prospector +## Prospector -If you have `prospector` installed, then `pylint-django` will already be installed as a dependency, and will be activated automatically if Django is detected. +If you have `prospector` installed, then `pylint-django` will already be installed as a dependency, +and will be activated automatically if Django is detected. ``` prospector [..other options..] @@ -60,14 +59,14 @@ Please feel free to add your name to the `CONTRIBUTORS.md` file if you want to b credited when pull requests get merged. You can also add to the `CHANGELOG.md` file if you wish, although I'll also do that when merging if not. -## Tests +# Tests The structure of the test package follows that from pylint itself. It is fairly simple: create a module starting with `func_` followed by a test name, and insert into it some code. The tests will run pylint - against these modules. If the idea is that no messages now occur, then - that is fine, just check to see if it works by running `scripts/test.sh`. +against these modules. If the idea is that no messages now occur, then +that is fine, just check to see if it works by running `scripts/test.sh`. Ideally, add some pylint error suppression messages to the file to prevent spurious warnings, since these are all tiny little modules not designed to @@ -82,4 +81,4 @@ These are useful to quickly add "expected messages". # License -`pylint-django` is available under the GPLv2 license. \ No newline at end of file +`pylint-django` is available under the GPLv2 license. From 6e34b7ad982e260c8e54bce4a28dc74494b89e04 Mon Sep 17 00:00:00 2001 From: "Mr. Senko" Date: Fri, 9 Mar 2018 13:27:41 +0200 Subject: [PATCH 4/7] Remove unused import --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index f0cbdf06..309cc849 100644 --- a/setup.py +++ b/setup.py @@ -2,7 +2,6 @@ """ Setup module for Pylint plugin for Django. """ -import os from setuptools import setup, find_packages From 850bf833333e8b489b8e92b67586d1eec198db63 Mon Sep 17 00:00:00 2001 From: "Mr. Senko" Date: Fri, 9 Mar 2018 13:02:42 +0200 Subject: [PATCH 5/7] Let DjangoInstalledChecker warn when Django isn't available by ignoring the import failure Python will not produce a traceback but instead let DjangoInstalledChecker do its job and warn the user. I also add a CI build stage to test this scenario since we don't want to continue if this isn't working. NOTE: tox doesn't allow us to easily execute shell scripts and commands outside its virtualenv so I have to resort to ugly bash escaping to make this work! Finally properly fixes #96. --- .travis.yml | 10 +++++++--- pylint_django/plugin.py | 9 +++++++-- tox.ini | 3 +++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6258fd4c..9ed2fe9e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,14 +7,18 @@ python: env: - DJANGO=1.11 - DJANGO=2.0 +stages: + - django_not_installed + - test matrix: exclude: # Python/Django combinations that aren't officially supported - { python: 2.7, env: DJANGO=2.0 } include: - - { python: 3.6, env: TOXENV=flake8 } - - { python: 3.6, env: TOXENV=pylint } - - { python: 3.6, env: TOXENV=readme } + - { stage: django_not_installed, python: 3.6, env: TOXENV=django_not_installed } + - { stage: test, python: 3.6, env: TOXENV=flake8 } + - { stage: test, python: 3.6, env: TOXENV=pylint } + - { stage: test, python: 3.6, env: TOXENV=readme } allow_failures: - env: TOXENV=flake8 - env: TOXENV=pylint diff --git a/pylint_django/plugin.py b/pylint_django/plugin.py index 06fb6f4d..9420e351 100644 --- a/pylint_django/plugin.py +++ b/pylint_django/plugin.py @@ -2,7 +2,6 @@ from pylint.checkers.base import NameChecker from pylint_plugin_utils import get_checker -from pylint_django.augmentations import apply_augmentations from pylint_django.checkers import register_checkers # we want to import the transforms to make sure they get added to the astroid manager, @@ -26,4 +25,10 @@ def register(linter): register_checkers(linter) # register any checking fiddlers - apply_augmentations(linter) + try: + from pylint_django.augmentations import apply_augmentations + apply_augmentations(linter) + except ModuleNotFoundError: + # probably trying to execute pylint_django when Django isn't installed + # in this case the django-not-installed checker will kick-in + pass diff --git a/tox.ini b/tox.ini index 3f374773..350c073a 100644 --- a/tox.ini +++ b/tox.ini @@ -3,6 +3,7 @@ [tox] envlist = + django_not_installed flake8 pylint readme @@ -11,6 +12,7 @@ envlist = [testenv] commands = + django_not_installed: bash -c \'pylint --load-plugins=pylint_django setup.py | grep django-not-available\' flake8: flake8 pylint: pylint --rcfile=tox.ini pylint_django setup readme: python setup.py check --restructuredtext --strict @@ -35,6 +37,7 @@ setenv = PIP_DISABLE_PIP_VERSION_CHECK = 1 PYTHONPATH = . whitelist_externals = + django_not_installed: bash clean: find clean: rm From 2dbcb18001b9ab834078dc380d3637b6754c0b32 Mon Sep 17 00:00:00 2001 From: "Mr. Senko" Date: Fri, 9 Mar 2018 14:35:21 +0200 Subject: [PATCH 6/7] Add test invoking pylint_django when Django has been installed in this case we expect no errors for setup.py --- .travis.yml | 2 ++ tox.ini | 3 +++ 2 files changed, 5 insertions(+) diff --git a/.travis.yml b/.travis.yml index 9ed2fe9e..499113d1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,6 +9,7 @@ env: - DJANGO=2.0 stages: - django_not_installed + - django_is_installed - test matrix: exclude: @@ -16,6 +17,7 @@ matrix: - { python: 2.7, env: DJANGO=2.0 } include: - { stage: django_not_installed, python: 3.6, env: TOXENV=django_not_installed } + - { stage: django_is_installed, python: 3.6, env: TOXENV=django_is_installed } - { stage: test, python: 3.6, env: TOXENV=flake8 } - { stage: test, python: 3.6, env: TOXENV=pylint } - { stage: test, python: 3.6, env: TOXENV=readme } diff --git a/tox.ini b/tox.ini index 350c073a..ca4f63da 100644 --- a/tox.ini +++ b/tox.ini @@ -4,6 +4,7 @@ [tox] envlist = django_not_installed + django_is_installed flake8 pylint readme @@ -13,6 +14,7 @@ envlist = [testenv] commands = django_not_installed: bash -c \'pylint --load-plugins=pylint_django setup.py | grep django-not-available\' + django_is_installed: pylint --load-plugins=pylint_django setup.py flake8: flake8 pylint: pylint --rcfile=tox.ini pylint_django setup readme: python setup.py check --restructuredtext --strict @@ -21,6 +23,7 @@ commands = clean: find . -type d -name __pycache__ -delete clean: rm -rf build/ .cache/ dist/ .eggs/ pylint_django.egg-info/ .tox/ deps = + django_is_installed: Django flake8: flake8 pylint: pylint pylint: Django>=2.0,<2.1 From 58265d74c6d0c33a0b2199890260d1b2fbeb9e9f Mon Sep 17 00:00:00 2001 From: "Mr. Senko" Date: Fri, 9 Mar 2018 17:37:17 +0200 Subject: [PATCH 7/7] Build packages and perform package sanity tests. Fixes #136 --- .travis.yml | 22 ++++++++++--- scripts/build.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 4 deletions(-) create mode 100755 scripts/build.sh diff --git a/.travis.yml b/.travis.yml index 499113d1..a8fd73ee 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,12 +5,15 @@ python: - 3.5 - 3.6 env: - - DJANGO=1.11 + # note: latest versions first b/c the top-most is included in new + # build stages if not specified - DJANGO=2.0 + - DJANGO=1.11 stages: - django_not_installed - django_is_installed - test + - build_and_package_sanity matrix: exclude: # Python/Django combinations that aren't officially supported @@ -21,6 +24,7 @@ matrix: - { stage: test, python: 3.6, env: TOXENV=flake8 } - { stage: test, python: 3.6, env: TOXENV=pylint } - { stage: test, python: 3.6, env: TOXENV=readme } + - { stage: build_and_package_sanity, python: 3.6, env: SANITY_CHECK=1 } allow_failures: - env: TOXENV=flake8 - env: TOXENV=pylint @@ -29,11 +33,21 @@ matrix: install: - pip install tox-travis script: - - tox + - | + + if [ -z "$SANITY_CHECK" ]; then + tox + else + ./scripts/build.sh + fi after_success: - - pip install coveralls - - coveralls + - | + if [ -z "$SANITY_CHECK" ]; then + pip install coveralls + coveralls + fi + notifications: email: on_failure: change diff --git a/scripts/build.sh b/scripts/build.sh new file mode 100755 index 00000000..350ec40c --- /dev/null +++ b/scripts/build.sh @@ -0,0 +1,81 @@ +#!/bin/bash + +# Build packages for distribution on PyPI +# and execute some sanity scripts on them +# +# note: must be executed from the root directory of the project + +# first clean up the local environment +echo "..... Clean up first" +find . -type f -name '*.pyc' -delete +find . -type d -name __pycache__ -delete +find . -type d -name '*.egg-info' | xargs rm -rf +rm -rf build/ .cache/ dist/ .eggs/ .tox/ + +# then build the packages +echo "..... Building PyPI packages" +set -e +python setup.py sdist >/dev/null +python setup.py bdist_wheel >/dev/null +set +e + +# then run some sanity tests +echo "..... Searching for .pyc files inside the built packages" +matched_files=`tar -tvf dist/*.tar.gz | grep -c "\.pyc"` +if [ "$matched_files" -gt "0" ]; then + echo "ERROR: .pyc files found in .tar.gz package" + exit 1 +fi +matched_files=`unzip -t dist/*.whl | grep -c "\.pyc"` +if [ "$matched_files" -gt "0" ]; then + echo "ERROR: .pyc files found in wheel package" + exit 1 +fi + +echo "..... Trying to verify that all source files are present" +# remove pylint_django/*.egg-info/ generated during build +find . -type d -name '*.egg-info' | xargs rm -rf + +source_files=`find ./pylint_django/ -type f | sed 's|./||'` + +# verify for .tar.gz package +package_files=`tar -tvf dist/*.tar.gz` +for src_file in $source_files; do + echo "$package_files" | grep $src_file >/dev/null + if [ "$?" -ne 0 ]; then + echo "ERROR: $src_file not found inside tar.gz package" + exit 1 + fi +done + +# verify for wheel package +package_files=`unzip -t dist/*.whl` +for src_file in $source_files; do + echo "$package_files" | grep $src_file >/dev/null + if [ "$?" -ne 0 ]; then + echo "ERROR: $src_file not found inside wheel package" + exit 1 + fi +done + +# exit on error from now on +set -e + +echo "..... Trying to install the new tarball inside a virtualenv" +# note: installs with the optional dependency to verify this is still working +virtualenv .venv/test-tarball +source .venv/test-tarball/bin/activate +pip install --no-binary :all: -f dist/ pylint_django[with_django] +pip freeze | grep Django +deactivate +rm -rf .venv/ + +echo "..... Trying to install the new wheel inside a virtualenv" +virtualenv .venv/test-wheel +source .venv/test-wheel/bin/activate +pip install pylint-plugin-utils # because it does not provide a wheel package +pip install --only-binary :all: -f dist/ pylint_django +deactivate +rm -rf .venv/ + +echo "..... PASS"