The following gdb/testsuite subdirs are codespell-clean:
...
$ for d in gdb/testsuite/gdb.*; do \
echo -n "$d:"; \
codespell --config ./gdb/contrib/setup.cfg $d \
| wc -l; \
done 2>&1 \
| grep :0
gdb/testsuite/gdb.ctf:0
gdb/testsuite/gdb.dap:0
gdb/testsuite/gdb.gdb:0
gdb/testsuite/gdb.go:0
gdb/testsuite/gdb.modula2:0
gdb/testsuite/gdb.objc:0
gdb/testsuite/gdb.opencl:0
gdb/testsuite/gdb.perf:0
gdb/testsuite/gdb.replay:0
gdb/testsuite/gdb.server:0
gdb/testsuite/gdb.testsuite:0
...
Add them to the pre-commit configuration.
Approved-By: Tom Tromey <tom@tromey.com>
Now that we're using codespell to check spelling in gdb files, can we use
codespell to bring this spelling warning:
...
$ echo usuable | codespell -
1: usuable
usuable ==> usable
...
to:
...
$ git commit -a -m "Usuable stuff"
...
?
First, let's look at a straightforward commit-msg hook implementation:
...
- id: codespell
name: codespell-commit-msg
verbose: true
always_run: true
stages: [commit-msg]
...
installed using:
...
$ pre-commit install -t commit-msg
...
When trying the commit, we get:
...
$ echo "/* bla */" >> gdb/gdb.c
$ git commit -a -m "Usuable stuff"
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
isort................................................(no files to check)Skipped
codespell............................................(no files to check)Skipped
check-include-guards.................................(no files to check)Skipped
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
codespell............................................(no files to check)Skipped
codespell-commit-msg.....................................................Failed
- hook id: codespell
- duration: 0.06s
- exit code: 65
.git/COMMIT_EDITMSG:1: Usuable ==> Usable
check-include-guards.................................(no files to check)Skipped
$
...
The commit was aborted, but the commit message is still there:
...
$ cat .git/COMMIT_EDITMSG
Usuable stuff
...
We can retry and edit the commit message to clean up the typo:
...
$ git commit -e -F .git/COMMIT_EDITMSG -a
...
but it's a bit cumbersome.
Furthermore, say we fix a typo and want to document this in the commit log, and
do:
...
$ git commit -m "Fixed typo: useable -> usable" -a
...
This commit cannot succeed, unless we add a codespell ignore tag, which feels
like taking it too far.
Both these problems can be addressed by setting things up in such a way that
the commit always succeeds, and codespell output is shown as a hint.
Ideally, we'd tell to pre-commit to implement this using some setting, but
there doesn't seem to be one.
So we use some indirection. Instead of using native codespell, use a local
hook that calls a script gdb/contrib/codespell-log.sh, which calls pre-commit,
which calls codespell.
Using this approach, we get:
...
$ echo "/* bla */" >> gdb/gdb.c
$ git commit -a -m "Usuable stuff"
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
isort................................................(no files to check)Skipped
codespell............................................(no files to check)Skipped
check-include-guards.................................(no files to check)Skipped
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
codespell............................................(no files to check)Skipped
check-include-guards.................................(no files to check)Skipped
codespell-log............................................................Passed
- hook id: codespell-log
- duration: 0.18s
codespell-log-internal...................................................Failed
- hook id: codespell
- exit code: 65
.git/COMMIT_EDITMSG:1: Usuable ==> Usable
[codespell/codespell-log-2 d081bd25a40] Usuable stuff
1 file changed, 1 insertion(+)
$
...
This is obviously convoluted, but it works. Perhaps we can propose a
pre-commit improvement (always_pass) and simplify this eventually.
Checked new script codespell-log.sh with shell-check.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Run `pre-commit autoupdate`. This brings in new versions of isort and
flake8.
Change-Id: I55f8b51b100e090e9a210338f46e10cf131a4aa7
Approved-By: Tom Tromey <tom@tromey.com>
Ignore the following spellcheck warning:
...
$ codespell --config gdb/contrib/setup.cfg gdb/unittests
gdb/unittests/rsp-low-selftests.c:54: fo ==> of, for, to, do, go
...
and add gdb/unittests to the pre-commit codespell configuration.
Approved-By: Tom Tromey <tom@tromey.com>
This cleans up the last codespell reports in the Guile directory and
adds gdb/guile to pre-commit.
It also tells codespell to ignore URLs. I think this is warranted
because many URLs don't really contain words per se; and furthermore
if any URL-checking is needed at all, it would be for liveness and not
spelling.
Also I was wondering why the codespell config is in contrib and not
gdb/setup.cfg.
Approved-By: Tom de Vries <tdevries@suse.de>
As an alternative to adding the gdb dir to codespell while it's still not
codespell-clean [1], add gdb subdirs which are codespell-clean.
Found using:
...
$ for d in $(find gdb -maxdepth 1 -type d | egrep -v "testsuite|gdb$"); do \
echo -n "$d: "; \
codespell --config gdb/contrib/setup.cfg $d 2>/dev/null \
| wc -l; \
done 2>&1 \
| grep ": 0"
gdb/tui: 0
gdb/target: 0
gdb/data-directory: 0
gdb/po: 0
gdb/system-gdbinit: 0
gdb/mi: 0
gdb/syscalls: 0
gdb/arch: 0
gdb/regformats: 0
gdb/compile: 0
...
Verified using:
...
$ pre-commit run codespell --all-files
codespell................................................................Passed
...
Approved-By: Tom Tromey <tom@tromey.com>
[1] https://sourceware.org/pipermail/gdb-patches/2025-March/216781.html
Add a pre-commit codespell hook for directories gdbsupport and gdbserver,
which are codespell-clean:
...
$ pre-commit run codespell --all-files
codespell................................................................Passed
...
A non-trivial question is where the codespell configuration goes.
Currently we have codespell sections in gdbsupport/setup.cfg and
gdbserver/setup.cfg, but codespell doesn't automatically use those because the
pre-commit hook runs codespell at the root of the repository.
A solution would be to replace those 2 setup.cfg files with a setup.cfg in the
root of the repository. Not ideal because generally we try to avoid adding
files related to subdirectories at the root.
Another solution would be to add two codespell hooks, one using
--config gdbsupport/setup.cfg and one using --config gdbserver/setup.cfg, and
add a third one once we start supporting gdb. Not ideal because it creates
duplication, but certainly possible.
I went with the following solution: a setup.cfg file in gdb/contrib (alongside
codespell-ignore-words.txt) which is used for both gdbserver and gdbsupport.
So, what can this new setup do for us? Let's demonstrate by simulating a typo:
...
$ echo "/* aways */" >> gdbsupport/agent.cc
...
We can check unstaged changes before committing:
...
$ pre-commit run codespell --all-files
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Likewise, staged changes (no need for the --all-files):
...
$ git add gdbsupport/agent.cc
$ pre-commit run codespell
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
...
Or we can try to commit, and run into the codespell failure:
...
$ git commit -a
black................................................(no files to check)Skipped
flake8...............................................(no files to check)Skipped
isort................................................(no files to check)Skipped
codespell................................................................Failed
- hook id: codespell
- exit code: 65
gdbsupport/agent.cc:282: aways ==> always, away
check-include-guards.................................(no files to check)Skipped
...
which makes the commit fail.
Approved-By: Tom Tromey <tom@tromey.com>
Recent commit 0bd340d670 ("pre-commit autoupdate") bumped the isort version
to 6.0.0.
Subsequently, I started running into:
...
$ SKIP=flake8,isort pre-commit run
An error has occurred: InvalidManifestError:
==> File /home/vries/.cache/pre-commit/repommstqefj/.pre-commit-hooks.yaml
==> At Hook(id='isort')
==> At key: stages
==> At index 0
=====> Expected one of commit, commit-msg, manual, merge-commit, \
post-checkout, post-commit, post-merge, post-rewrite, prepare-commit-msg, \
push but got: 'pre-commit'
Check the log at /home/vries/.cache/pre-commit/pre-commit.log
...
I found a similar PR [1], that explains that using pre-commit as a stage (as
isort 6.0.0 does) is supported starting pre-commit 3.2.0.
Add minimum_pre_commit_version 3.2.0 in .pre-commit-config.yaml, as suggested
in the PR.
After adding this, I get a more helpful message:
...
$ SKIP=flake8,isort pre-commit run
An error has occurred: InvalidConfigError:
==> File .pre-commit-config.yaml
==> At Config()
==> At key: minimum_pre_commit_version
=====> pre-commit version 3.2.0 is required but version 2.17.0 is installed. \
Perhaps run `pip install --upgrade pre-commit`.
Check the log at /home/vries/.cache/pre-commit/pre-commit.log
...
and after doing so which upgrades pre-commit to version 4.1.0, as well as
re-installing pre-commit using:
...
$ pre-commit uninstall
$ pre-commit install
...
I have a functional setup again.
Interestingly, since pre-commit 4.1.0 runs in a python 3.11 environment, I no
longer need to skip flake8 and isort, as I needed to previously when the
system python 3.6 was used.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
[1] https://github.com/psf/black/issues/4065
pre-commit currently runs flake8 only on `gdb/python/**/*.py`. There
are more files we can run it on, without running it on all the testsuite
files. Add:
- gdb/gdb-gdb.py.in
- gdb/*.py
- gdb/testsuite/*.py
Fix the new errors that popped up:
gdb/copyright.py:29:21: W605 invalid escape sequence '\*'
gdb/copyright.py:29:29: W605 invalid escape sequence '\*'
gdb/copyright.py:29:38: W605 invalid escape sequence '\*'
gdb/copyright.py:29:46: W605 invalid escape sequence '\*'
gdb/copyright.py:34:1: F401 'datetime' imported but unused
gdb/testsuite/analyze-racy-logs.py:150:9: E722 do not use bare 'except'
Change-Id: Ia864c22d4f170d4e18ce3beb06a86c49966654b2
Approved-By: Tom Tromey <tom@tromey.com>
Run `pre-commit autoupdate`.
This picks up a fresh Black version from 2025, and with it comes a small
but welcome formatting change.
There is a new version of isort as well, but no formatting change there.
Change-Id: Ie654a9c14c3a4096893011082668efb57c166fa4
Add some documention to .pre-commit-config.yaml that explains:
- what the file is,
- how it can be used, and
- how to skip specific hooks in case of trouble.
Approved-By: Tom Tromey <tom@tromey.com>
Run `pre-commit autoupdate`, this is the outcome. There is no change in
formatting of Python files.
Change-Id: I79c221af1b2192f866a344ab17d6199b137371cb
Run `pre-commit autoupdate`, this is the outcome. There is no change in
formatting of Python files.
Change-Id: I977781fa6cc924c398cc3b9d9954dc0fbb95d082
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.
Approved-By: Simon Marchi <simon.marchi@efficios.com>
Running `pre-commit autoupdate` showed that there is a new version of
the black hook for v24.3.0. Update it.
ChangeLog:
* .pre-commit-config.yaml: Bump black hook to 24.3.0
Change-Id: I5ec7d2edf99cd15f6525281a43aed9ff481ee9ee
Add a pre-commit [1] config file, with a single hook to run black on the
gdb directory whenever a Python file is modified. We can always add
more hooks if we find some that are useful.
Using pre-commit to run hooks is opt-in, as in it's not mandatory at all
for development, but it can be useful to run some checks that are easy
to forget (like running black). The hooks run locally on the
developer's machine when doing `git commit` (although they can also be
configured to run at other stages of the git workflow).
Follow these instructions to install the hooks in your local development
git repository:
- Install pre-commit the way you prefer. It can be using your OS
package manager if it has a recent enough version, or using `pip
install pre-commit`.
- Go to the binutils-gdb repository and run `pre-commit install`.
This installs a git hook at `.git/hooks/pre-commit`.
Now, whenever you modify and try to commit a Python file, pre-commit
will run black on it. For instance, if I try to insert something
misformatted, I get this when doing `git commit`:
$ git commit
black....................................................................Failed
- hook id: black
- files were modified by this hook
reformatted gdb/python/lib/gdb/dap/breakpoint.py
All done! ✨🍰✨
1 file reformatted.
At this point, black has already reformatted the files in place, so the
changes that fix the formatting are ready to add and commit. black is
only ran on files modified in the commit.
The hook defines a black version, which is downloaded at `pre-commit
install` time. pre-commit manages its own env at
`$HOME/.cache/pre-commit/<some-hash>`, so it won't use the version of
black you have installed already. This may help ensure that
contributors use the right black version.
The procedure when there is a new version of black (or a new version of
any hook we might be using in the future) is:
- Modify .pre-commit-config.yaml to change the version number, push to
the upstream repo.
- Have contributors run `pre-commit autoupdate` to make their local
pre-commit installation update the hooks.
It is possible to have pre-commit skip some hooks if needed [2].
I will add these instructions to the wiki if this patch gets merged, so
they are easy to find. We could perhaps think of having a
gdb/CONTRIBUTING document of some sort checked in the repo with that
kind of information.
I have not used pre-commit in a real project before, but have heard good
things from it. If we want to give it a try before pushing it to the
repo, some volunteers can copy the .pre-commit-config.yaml file locally
and try it for some time. However, pushing the file upstream is not
going to impact anybody who doesn't care about it, so I'd say it's
relatively low-risk to push it right now.
[1] https://pre-commit.com
[2] https://pre-commit.com/#temporarily-disabling-hooks
Change-Id: Id00cda882f5140914a670c87e574fa7f2f972099
Acked-By: Tom Tromey <tromey@adacore.com>
Acked-By: Guinevere Larsen <blarsen@redhat.com>
Acked-By: Andrew Burgess <aburgess@redhat.com>
