* [PATCH] Add flake8 and isort to .pre-commit-config.yaml @ 2024-04-02 18:11 Tom Tromey 2024-04-02 20:26 ` Simon Marchi 0 siblings, 1 reply; 10+ messages in thread From: Tom Tromey @ 2024-04-02 18:11 UTC (permalink / raw) To: gdb-patches; +Cc: Tom Tromey 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$ -- 2.43.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-02 18:11 [PATCH] Add flake8 and isort to .pre-commit-config.yaml Tom Tromey @ 2024-04-02 20:26 ` Simon Marchi 2024-04-03 19:52 ` Tom Tromey 0 siblings, 1 reply; 10+ messages in thread From: Simon Marchi @ 2024-04-02 20:26 UTC (permalink / raw) To: Tom Tromey, gdb-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-02 20:26 ` Simon Marchi @ 2024-04-03 19:52 ` Tom Tromey 2024-04-03 19:57 ` Simon Marchi 0 siblings, 1 reply; 10+ messages in thread From: Tom Tromey @ 2024-04-03 19:52 UTC (permalink / raw) To: Simon Marchi; +Cc: Tom Tromey, gdb-patches >>>>> "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, ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-03 19:52 ` Tom Tromey @ 2024-04-03 19:57 ` Simon Marchi 2024-04-09 7:58 ` Tom de Vries 0 siblings, 1 reply; 10+ messages in thread From: Simon Marchi @ 2024-04-03 19:57 UTC (permalink / raw) To: Tom Tromey; +Cc: Tom Tromey, gdb-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-03 19:57 ` Simon Marchi @ 2024-04-09 7:58 ` Tom de Vries 2024-04-09 9:41 ` Kévin Le Gouguec 2024-04-09 15:22 ` Tom Tromey 0 siblings, 2 replies; 10+ messages in thread From: Tom de Vries @ 2024-04-09 7:58 UTC (permalink / raw) To: Simon Marchi, Tom Tromey; +Cc: Tom Tromey, gdb-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-09 7:58 ` Tom de Vries @ 2024-04-09 9:41 ` Kévin Le Gouguec 2024-04-09 12:56 ` Tom de Vries 2024-04-09 15:22 ` Tom Tromey 1 sibling, 1 reply; 10+ messages in thread From: Kévin Le Gouguec @ 2024-04-09 9:41 UTC (permalink / raw) To: Tom de Vries; +Cc: Simon Marchi, Tom Tromey, Tom Tromey, gdb-patches 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-09 9:41 ` Kévin Le Gouguec @ 2024-04-09 12:56 ` Tom de Vries 2024-04-09 13:04 ` Kévin Le Gouguec 0 siblings, 1 reply; 10+ messages in thread From: Tom de Vries @ 2024-04-09 12:56 UTC (permalink / raw) To: Kévin Le Gouguec; +Cc: Simon Marchi, Tom Tromey, Tom Tromey, gdb-patches 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. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-09 12:56 ` Tom de Vries @ 2024-04-09 13:04 ` Kévin Le Gouguec 0 siblings, 0 replies; 10+ messages in thread From: Kévin Le Gouguec @ 2024-04-09 13:04 UTC (permalink / raw) To: Tom de Vries; +Cc: Simon Marchi, Tom Tromey, Tom Tromey, gdb-patches 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! ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-09 7:58 ` Tom de Vries 2024-04-09 9:41 ` Kévin Le Gouguec @ 2024-04-09 15:22 ` Tom Tromey 2024-04-10 5:38 ` Tom de Vries 1 sibling, 1 reply; 10+ messages in thread From: Tom Tromey @ 2024-04-09 15:22 UTC (permalink / raw) To: Tom de Vries; +Cc: Simon Marchi, Tom Tromey, Tom Tromey, gdb-patches >>>>> "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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] Add flake8 and isort to .pre-commit-config.yaml 2024-04-09 15:22 ` Tom Tromey @ 2024-04-10 5:38 ` Tom de Vries 0 siblings, 0 replies; 10+ messages in thread From: Tom de Vries @ 2024-04-10 5:38 UTC (permalink / raw) To: Tom Tromey; +Cc: Simon Marchi, Tom Tromey, gdb-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2024-04-10 5:38 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-04-02 18:11 [PATCH] Add flake8 and isort to .pre-commit-config.yaml Tom Tromey 2024-04-02 20:26 ` Simon Marchi 2024-04-03 19:52 ` Tom Tromey 2024-04-03 19:57 ` Simon Marchi 2024-04-09 7:58 ` Tom de Vries 2024-04-09 9:41 ` Kévin Le Gouguec 2024-04-09 12:56 ` Tom de Vries 2024-04-09 13:04 ` Kévin Le Gouguec 2024-04-09 15:22 ` Tom Tromey 2024-04-10 5:38 ` Tom de Vries
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).