Add flake8 and isort to .pre-commit-config.yaml
Checks
Context |
Check |
Description |
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
This adds flake8 and isort to .pre-commit-config.yaml. This way, they
will automatically be run on commit.
I chose the most recent available versions after verifying that they
don't cause any reports or changes in the current tree.
Internally at AdaCore, we also use a few flake8 plugins as well, so
perhaps that's another avenue for investigation.
---
.pre-commit-config.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)
Comments
On 2024-04-02 14:11, Tom Tromey wrote:
> This adds flake8 and isort to .pre-commit-config.yaml. This way, they
> will automatically be run on commit.
>
> I chose the most recent available versions after verifying that they
> don't cause any reports or changes in the current tree.
>
> Internally at AdaCore, we also use a few flake8 plugins as well, so
> perhaps that's another avenue for investigation.
> ---
> .pre-commit-config.yaml | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
> index 7afe60c20be..bca8acc5d02 100644
> --- a/.pre-commit-config.yaml
> +++ b/.pre-commit-config.yaml
> @@ -6,3 +6,14 @@ repos:
> hooks:
> - id: black
> files: 'gdb/.*'
> + - repo: https://github.com/pycqa/flake8
> + rev: 7.0.0
> + hooks:
> + - id: flake8
> + files: gdb/python/.*\.py$
> + args: [--config, gdb/setup.cfg]
> + - repo: https://github.com/pycqa/isort
> + rev: 5.13.2
> + hooks:
> + - id: isort
> + files: gdb/.*\.py$
I only used `gdb/.*.py` for the black hook, because pre-commit does its
own filtering already. The hooks specify the `python` type, so
pre-commit only runs the hook on what it believes is a Python file. The
only purpose of the regex was to limit the hooks to the gdb
sub-directory.
However, I realized we have at least one file that is python but isn't
named *.py, gdb-gdb.py.in. I just tried, and the black hook doesn't run
on gdb-gdb.py-in. If you run black manually, it considers gdb-gdb.py.in
as a Python file (I don't know by which magic) and re-formats it:
$ pwd
/home/simark/src/binutils-gdb
$ black gdb
reformatted /home/simark/src/binutils-gdb/gdb/gdb-gdb.py.in
But the filtering done by pre-commit doesn't consider gdb-gdb.py.in a
Python file. The hooks specify `types: 'python'`, which doesn't match
'*.py.in'. The doc for `types` is here:
https://pre-commit.com/#filtering-files-with-types
The mapping between extensions and types is here:
https://github.com/pre-commit/identify/blob/main/identify/extensions.py
If we want to make pre-commit consider that file for our Python hooks,
the only solution I see currently would be to use something like this,
as described in the doc above:
hooks:
- id: black
types_or: [file]
files: '^gdb/.*\.(py|py.in)$'
It didn't seem to work for me using `types` (an error in gdb-gdb.py.in
wouldn't be caught), but it works using `types_or`. So, I would
consider something similar for other hooks as well.
Simon
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> If we want to make pre-commit consider that file for our Python hooks,
Simon> the only solution I see currently would be to use something like this,
Simon> as described in the doc above:
I did this in v2. Let me know what you think.
Tom
commit c3c3740f73b5f20697074c5a20a26d0805afbb98
Author: Tom Tromey <tromey@adacore.com>
Date: Tue Apr 2 12:04:21 2024 -0600
Add flake8 and isort to .pre-commit-config.yaml
This adds flake8 and isort to .pre-commit-config.yaml. This way, they
will automatically be run on commit.
I chose the most recent available versions after verifying that they
don't cause any reports or changes in the current tree.
Internally at AdaCore, we also use a few flake8 plugins as well, so
perhaps that's another avenue for investigation.
v2: Also update the various file-selection clauses to pick up
gdb-gdb.py.in; include the isort change made to this file; and finally
add a comment about the exclusions from flake8.
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 7afe60c20be..8721dac678b 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -5,4 +5,20 @@ repos:
rev: 24.3.0
hooks:
- id: black
- files: 'gdb/.*'
+ types_or: [file]
+ files: 'gdb/.*\.py(\.in)?$'
+ - repo: https://github.com/pycqa/flake8
+ rev: 7.0.0
+ hooks:
+ - id: flake8
+ types_or: [file]
+ # Note this one is only run on gdb/python, not (for now) the
+ # test suite.
+ files: 'gdb/python/.*\.py(\.in)?$'
+ args: [--config, gdb/setup.cfg]
+ - repo: https://github.com/pycqa/isort
+ rev: 5.13.2
+ hooks:
+ - id: isort
+ types_or: [file]
+ files: 'gdb/.*\.py(\.in)?$'
diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in
index 54db9b00cf3..b5a7fa4f390 100644
--- a/gdb/gdb-gdb.py.in
+++ b/gdb/gdb-gdb.py.in
@@ -15,9 +15,10 @@
# You should have received a copy of the GNU General Public License
# along with this program. If not, see <http://www.gnu.org/licenses/>.
-import gdb
import os.path
+import gdb
+
class TypeFlag:
"""A class that allows us to store a flag name, its short name,
On 4/3/24 3:52 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>
> Simon> If we want to make pre-commit consider that file for our Python hooks,
> Simon> the only solution I see currently would be to use something like this,
> Simon> as described in the doc above:
>
> I did this in v2. Let me know what you think.
>
> Tom
>
> commit c3c3740f73b5f20697074c5a20a26d0805afbb98
> Author: Tom Tromey <tromey@adacore.com>
> Date: Tue Apr 2 12:04:21 2024 -0600
>
> Add flake8 and isort to .pre-commit-config.yaml
>
> This adds flake8 and isort to .pre-commit-config.yaml. This way, they
> will automatically be run on commit.
>
> I chose the most recent available versions after verifying that they
> don't cause any reports or changes in the current tree.
>
> Internally at AdaCore, we also use a few flake8 plugins as well, so
> perhaps that's another avenue for investigation.
>
> v2: Also update the various file-selection clauses to pick up
> gdb-gdb.py.in; include the isort change made to this file; and finally
> add a comment about the exclusions from flake8.
>
> diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
> index 7afe60c20be..8721dac678b 100644
> --- a/.pre-commit-config.yaml
> +++ b/.pre-commit-config.yaml
> @@ -5,4 +5,20 @@ repos:
> rev: 24.3.0
> hooks:
> - id: black
> - files: 'gdb/.*'
> + types_or: [file]
> + files: 'gdb/.*\.py(\.in)?$'
> + - repo: https://github.com/pycqa/flake8
> + rev: 7.0.0
> + hooks:
> + - id: flake8
> + types_or: [file]
> + # Note this one is only run on gdb/python, not (for now) the
> + # test suite.
> + files: 'gdb/python/.*\.py(\.in)?$'
> + args: [--config, gdb/setup.cfg]
> + - repo: https://github.com/pycqa/isort
> + rev: 5.13.2
> + hooks:
> + - id: isort
> + types_or: [file]
> + files: 'gdb/.*\.py(\.in)?$'
> diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in
> index 54db9b00cf3..b5a7fa4f390 100644
> --- a/gdb/gdb-gdb.py.in
> +++ b/gdb/gdb-gdb.py.in
> @@ -15,9 +15,10 @@
> # You should have received a copy of the GNU General Public License
> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>
> -import gdb
> import os.path
>
> +import gdb
> +
>
> class TypeFlag:
> """A class that allows us to store a flag name, its short name,
Thanks, LGTM.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Simon
On 4/3/24 21:57, Simon Marchi wrote:
> On 4/3/24 3:52 PM, Tom Tromey wrote:
>>>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
>>
>> Simon> If we want to make pre-commit consider that file for our Python hooks,
>> Simon> the only solution I see currently would be to use something like this,
>> Simon> as described in the doc above:
>>
>> I did this in v2. Let me know what you think.
>>
>> Tom
>>
>> commit c3c3740f73b5f20697074c5a20a26d0805afbb98
>> Author: Tom Tromey <tromey@adacore.com>
>> Date: Tue Apr 2 12:04:21 2024 -0600
>>
>> Add flake8 and isort to .pre-commit-config.yaml
>>
>> This adds flake8 and isort to .pre-commit-config.yaml. This way, they
>> will automatically be run on commit.
>>
>> I chose the most recent available versions after verifying that they
>> don't cause any reports or changes in the current tree.
>>
>> Internally at AdaCore, we also use a few flake8 plugins as well, so
>> perhaps that's another avenue for investigation.
>>
>> v2: Also update the various file-selection clauses to pick up
>> gdb-gdb.py.in; include the isort change made to this file; and finally
>> add a comment about the exclusions from flake8.
>>
>> diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
>> index 7afe60c20be..8721dac678b 100644
>> --- a/.pre-commit-config.yaml
>> +++ b/.pre-commit-config.yaml
>> @@ -5,4 +5,20 @@ repos:
>> rev: 24.3.0
>> hooks:
>> - id: black
>> - files: 'gdb/.*'
>> + types_or: [file]
>> + files: 'gdb/.*\.py(\.in)?$'
>> + - repo: https://github.com/pycqa/flake8
>> + rev: 7.0.0
>> + hooks:
>> + - id: flake8
>> + types_or: [file]
>> + # Note this one is only run on gdb/python, not (for now) the
>> + # test suite.
>> + files: 'gdb/python/.*\.py(\.in)?$'
>> + args: [--config, gdb/setup.cfg]
>> + - repo: https://github.com/pycqa/isort
>> + rev: 5.13.2
>> + hooks:
>> + - id: isort
>> + types_or: [file]
>> + files: 'gdb/.*\.py(\.in)?$'
>> diff --git a/gdb/gdb-gdb.py.in b/gdb/gdb-gdb.py.in
>> index 54db9b00cf3..b5a7fa4f390 100644
>> --- a/gdb/gdb-gdb.py.in
>> +++ b/gdb/gdb-gdb.py.in
>> @@ -15,9 +15,10 @@
>> # You should have received a copy of the GNU General Public License
>> # along with this program. If not, see <http://www.gnu.org/licenses/>.
>>
>> -import gdb
>> import os.path
>>
>> +import gdb
>> +
>>
>> class TypeFlag:
>> """A class that allows us to store a flag name, its short name,
>
> Thanks, LGTM.
>
> Approved-By: Simon Marchi <simon.marchi@efficios.com>
Hi,
just to report back on this with my current state.
I use openleap 15.4 with python 3.6, and the used version of flake8
requires python 3.7, so I ran into trouble after this commit.
Then I learned about "git commit -n", and started using that as
workaround, but eventually ran into trouble because that doesn't seem to
work while rebasing.
I then decided to install python 3.11 and use a virtual environment, and
use git commit from within that environment. AFAICT, the virtual
environment works as expected:
...
$ python --version
Python 3.11.5
$ python3 --version
Python 3.11.5
...
but for some reason I keep getting:
...
SyntaxError: future feature annotations is not defined
...
which is the same error I got with python 3.6.
I've tried removing ~/.cache/pre-commit a couple of times, but that
didn't help either.
So, atm I can no longer rebase. I'll try to workaround this by adding a
local commit that reverts this change, or something similar, but if
anybody has another idea I'd be happy to hear it.
Thanks,
- Tom
Tom de Vries <tdevries@suse.de> writes:
> just to report back on this with my current state.
>
> I use openleap 15.4 with python 3.6, and the used version of flake8 requires python 3.7, so I ran into trouble after this commit.
>
> Then I learned about "git commit -n", and started using that as workaround, but eventually ran into trouble because that doesn't seem to work while rebasing.
>
> I then decided to install python 3.11 and use a virtual environment, and use git commit from within that environment. AFAICT, the virtual environment works as expected:
> ...
> $ python --version
> Python 3.11.5
> $ python3 --version
> Python 3.11.5
> ...
> but for some reason I keep getting:
> ...
> SyntaxError: future feature annotations is not defined
> ...
> which is the same error I got with python 3.6.
>
> I've tried removing ~/.cache/pre-commit a couple of times, but that didn't help either.
>
> So, atm I can no longer rebase. I'll try to workaround this by adding a local commit that reverts this change, or something similar, but if anybody has another idea I'd be happy to hear it.
Hunch: have you installed pre-commit in this virtual environment, or are
you using a pre-commit that comes from "outside", e.g. your distro, or
'pip install --user' before activating the venv? FWIW this might yield
clues:
$ head -1 $(which pre-commit)
Over here this shows a shebang that points to […VENV…]/bin/python3
(which in turn is a symlink to my system Python).
I would hope that 'pip install pre-commit' within your virtual
environment would install a pre-commit that will use "the right Python",
but I could be missing something.
On 4/9/24 11:41, Kévin Le Gouguec wrote:
> Tom de Vries <tdevries@suse.de> writes:
>
>> just to report back on this with my current state.
>>
>> I use openleap 15.4 with python 3.6, and the used version of flake8 requires python 3.7, so I ran into trouble after this commit.
>>
>> Then I learned about "git commit -n", and started using that as workaround, but eventually ran into trouble because that doesn't seem to work while rebasing.
>>
>> I then decided to install python 3.11 and use a virtual environment, and use git commit from within that environment. AFAICT, the virtual environment works as expected:
>> ...
>> $ python --version
>> Python 3.11.5
>> $ python3 --version
>> Python 3.11.5
>> ...
>> but for some reason I keep getting:
>> ...
>> SyntaxError: future feature annotations is not defined
>> ...
>> which is the same error I got with python 3.6.
>>
>> I've tried removing ~/.cache/pre-commit a couple of times, but that didn't help either.
>>
>> So, atm I can no longer rebase. I'll try to workaround this by adding a local commit that reverts this change, or something similar, but if anybody has another idea I'd be happy to hear it.
>
> Hunch: have you installed pre-commit in this virtual environment, or are
> you using a pre-commit that comes from "outside", e.g. your distro, or
> 'pip install --user' before activating the venv? FWIW this might yield
> clues:
>
> $ head -1 $(which pre-commit)
>
Hi Kevin,
thanks for the reaction.
The command you mention showed the correct pre-commit version.
But when running git commit, the wrong one was used. I just needed to
rerun "pre-commit install" to re-install the hooks, which fixes this.
I'll update the wiki entry to clarify this.
Thanks,
- Tom
> Over here this shows a shebang that points to […VENV…]/bin/python3
> (which in turn is a symlink to my system Python).
>
> I would hope that 'pip install pre-commit' within your virtual
> environment would install a pre-commit that will use "the right Python",
> but I could be missing something.
Tom de Vries <tdevries@suse.de> writes:
> But when running git commit, the wrong one was used. I just needed to rerun "pre-commit install" to re-install the hooks, which fixes this.
Oooh, hadn't though of that - pre-commit does capture an
"INSTALL_PYTHON" into .git/hooks/pre-commit. Good catch!
>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
Tom> So, atm I can no longer rebase. I'll try to workaround this by adding
Tom> a local commit that reverts this change, or something similar, but if
Tom> anybody has another idea I'd be happy to hear it.
It sounds like you found a fix, but in extremis you can also
"pre-commit uninstall" and just not use the hook.
Tom
On 4/9/24 17:22, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
>
> Tom> So, atm I can no longer rebase. I'll try to workaround this by adding
> Tom> a local commit that reverts this change, or something similar, but if
> Tom> anybody has another idea I'd be happy to hear it.
>
> It sounds like you found a fix, but in extremis you can also
> "pre-commit uninstall" and just not use the hook.
Yes, I didn't realize that at the point I ran into the trouble. Perhaps
pre-commit could be improved to advertise considering using SKIP at that
point, that could have helped.
I dropped the approach of installing a second python version alongside,
I probably did something wrong but versions got mixed and things went
downhill from there pretty quickly. Deinstalling the second python
version was easy, but manually purging various files in ~/.local and
~/.cache to get back to a working state took some work.
Anyway, my current approach is to install the hooks and add a line:
...
export SKIP=flake8,isort
...
That way, I'm still running black.
Thanks,
- Tom
@@ -6,3 +6,14 @@ repos:
hooks:
- id: black
files: 'gdb/.*'
+ - repo: https://github.com/pycqa/flake8
+ rev: 7.0.0
+ hooks:
+ - id: flake8
+ files: gdb/python/.*\.py$
+ args: [--config, gdb/setup.cfg]
+ - repo: https://github.com/pycqa/isort
+ rev: 5.13.2
+ hooks:
+ - id: isort
+ files: gdb/.*\.py$