[RFC PATCH] Add new build target 'check-spelling'

[RFC PATCH] Add new build target 'check-spelling'

[Stefan Weil via]

`make check-spelling` can now be used to get a list of spelling errors.
It uses the latest version of codespell, a spell checker implemented in Python.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---

This RFC can already be used for manual tests, but still reports false
positives, mostly because some variable names are interpreted as words.
These words can either be ignored in the check, or in some cases the code
might be changed to use different variable names.

The check currently only skips a few directories and files, so for example
checked out submodules are also checked.

The rule can be extended to allow user provided ignore and skip lists,
for example by introducing Makefile variables CODESPELL_SKIP=userfile
or CODESPELL_IGNORE=userfile. A limited check could be implemented by
providing a base directory CODESPELL_START=basedirectory, for example
CODESPELL_START=docs.

Regards,
Stefan

 tests/Makefile.include       | 10 ++++++++++
 tests/codespell/README.rst   | 18 ++++++++++++++++++
 tests/codespell/exclude-file |  3 +++
 tests/codespell/ignore-words | 19 +++++++++++++++++++
 tests/requirements.txt       |  1 +
 5 files changed, 51 insertions(+)
 create mode 100644 tests/codespell/README.rst
 create mode 100644 tests/codespell/exclude-file
 create mode 100644 tests/codespell/ignore-words

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 9422ddaece..b9daeda932 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -155,6 +155,16 @@ check-acceptance-deprecated-warning:
 
 check-acceptance: check-acceptance-deprecated-warning | check-avocado
 
+.PHONY: check-spelling
+CODESPELL_DIR=tests/codespell
+check-spelling: check-venv
+	source $(TESTS_VENV_DIR)/bin/activate && \
+	cd "$(SRC_PATH)" && \
+	codespell -s . \
+	  --exclude-file=$(CODESPELL_DIR)/exclude-file \
+	  --ignore-words=$(CODESPELL_DIR)/ignore-words \
+	  --skip="./.git,./bin,./build,./linux-headers,*.patch,nohup.out"
+
 # Consolidated targets
 
 .PHONY: check check-clean get-vm-images
diff --git a/tests/codespell/README.rst b/tests/codespell/README.rst
new file mode 100644
index 0000000000..67e070d631
--- /dev/null
+++ b/tests/codespell/README.rst
@@ -0,0 +1,18 @@
+=============================
+Check spelling with codespell
+=============================
+
+`make check-spelling` can be used to get a list of spelling errors.
+It reports files with spelling errors and a summary of all misspelled words.
+The report is generated by the latest version of codespell, a spell checker
+implemented in Python.
+
+See https://github.com/codespell-project/codespell for more information.
+
+Some file patterns are excluded from the check.
+
+In addition tests/codespell includes several files which are used to
+suppress certain false positives in the codespell report.
+
+exclude-file - complete lines which should be ignored
+ignore-words - list of words which should be ignored
diff --git a/tests/codespell/exclude-file b/tests/codespell/exclude-file
new file mode 100644
index 0000000000..57de81a4eb
--- /dev/null
+++ b/tests/codespell/exclude-file
@@ -0,0 +1,3 @@
+ * VAS controller.
+number generator daemon such as the one found in the vhost-device crate of
+introspection.  The latter can conceivably confuse clients, so tread
diff --git a/tests/codespell/ignore-words b/tests/codespell/ignore-words
new file mode 100644
index 0000000000..4d336a2f44
--- /dev/null
+++ b/tests/codespell/ignore-words
@@ -0,0 +1,19 @@
+buid
+busses
+dout
+falt
+fpr
+hace
+hax
+hda
+nd
+ot
+pard
+parm
+ptd
+ser
+som
+synopsys
+te
+toke
+ue
diff --git a/tests/requirements.txt b/tests/requirements.txt
index 0ba561b6bd..dd44e6768f 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -4,3 +4,4 @@
 # Note that qemu.git/python/ is always implicitly installed.
 avocado-framework==88.1
 pycdlib==1.11.0
+codespell
-- 
2.30.2

Re: [RFC PATCH] Add new build target 'check-spelling'

[Thomas Huth]

On 31/10/2022 08.43, Stefan Weil wrote:
> `make check-spelling` can now be used to get a list of spelling errors.
> It uses the latest version of codespell, a spell checker implemented in Python.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> This RFC can already be used for manual tests, but still reports false
> positives, mostly because some variable names are interpreted as words.
> These words can either be ignored in the check, or in some cases the code
> might be changed to use different variable names.
> 
> The check currently only skips a few directories and files, so for example
> checked out submodules are also checked.
> 
> The rule can be extended to allow user provided ignore and skip lists,
> for example by introducing Makefile variables CODESPELL_SKIP=userfile
> or CODESPELL_IGNORE=userfile. A limited check could be implemented by
> providing a base directory CODESPELL_START=basedirectory, for example
> CODESPELL_START=docs.
> 
> Regards,
> Stefan
> 
>   tests/Makefile.include       | 10 ++++++++++
>   tests/codespell/README.rst   | 18 ++++++++++++++++++
>   tests/codespell/exclude-file |  3 +++
>   tests/codespell/ignore-words | 19 +++++++++++++++++++
>   tests/requirements.txt       |  1 +
>   5 files changed, 51 insertions(+)
>   create mode 100644 tests/codespell/README.rst
>   create mode 100644 tests/codespell/exclude-file
>   create mode 100644 tests/codespell/ignore-words
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 9422ddaece..b9daeda932 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -155,6 +155,16 @@ check-acceptance-deprecated-warning:
>   
>   check-acceptance: check-acceptance-deprecated-warning | check-avocado
>   
> +.PHONY: check-spelling
> +CODESPELL_DIR=tests/codespell
> +check-spelling: check-venv
> +	source $(TESTS_VENV_DIR)/bin/activate && \
> +	cd "$(SRC_PATH)" && \
> +	codespell -s . \
> +	  --exclude-file=$(CODESPELL_DIR)/exclude-file \
> +	  --ignore-words=$(CODESPELL_DIR)/ignore-words \
> +	  --skip="./.git,./bin,./build,./linux-headers,*.patch,nohup.out"

I like the idea, but I think it's unlikely that we can make this work for 
the whole source tree any time soon. So maybe it makes more sense to start 
with some few directories first (e.g. docs/ ) and then the maintainers can 
opt-in by cleaning up their directories first and then by adding their 
directories to this target here?

  Thomas

Re: [RFC PATCH] Add new build target 'check-spelling'

[Stefan Weil via]

[-- Attachment #1.1.1: Type: text/plain, Size: 3422 bytes --]

Am 31.10.22 um 08:52 schrieb Thomas Huth:

> On 31/10/2022 08.43, Stefan Weil wrote:
>> `make check-spelling` can now be used to get a list of spelling errors.
>> It uses the latest version of codespell, a spell checker implemented 
>> in Python.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> This RFC can already be used for manual tests, but still reports false
>> positives, mostly because some variable names are interpreted as words.
>> These words can either be ignored in the check, or in some cases the 
>> code
>> might be changed to use different variable names.
>>
>> The check currently only skips a few directories and files, so for 
>> example
>> checked out submodules are also checked.
>>
>> The rule can be extended to allow user provided ignore and skip lists,
>> for example by introducing Makefile variables CODESPELL_SKIP=userfile
>> or CODESPELL_IGNORE=userfile. A limited check could be implemented by
>> providing a base directory CODESPELL_START=basedirectory, for example
>> CODESPELL_START=docs.
>>
>> Regards,
>> Stefan
[...]
>> I like the idea, but I think it's unlikely that we can make this work 
>> for the whole source tree any time soon. So maybe it makes more sense 
>> to start with some few directories first (e.g. docs/ ) and then the 
>> maintainers can opt-in by cleaning up their directories first and 
>> then by adding their directories to this target here?
>
>  Thomas


Even without implementing CODESPELL_START as described above, the script 
can already be used and integrated into CI scripts.

It takes about 60 seconds to check the whole source tree including 
submodules on my (slow) virtual machine.

The resulting output has about 20000 lines or 1272 KiB. It can be 
filtered for relevant parts of the source tree or used for a summary.

Sample script: grep "^[.]" spellcheck.log | sed s/^..// | sed 's/\/.*//' 
| sed s/:.*// | sort | uniq -c

This produces a summary for the top level hierarchy of files and 
directories:

       3 accel
       1 audio
       1 backends
      77 block
       7 block.c
      20 bsd-user
     386 capstone
      12 chardev
       1 configure
       8 contrib
       6 crypto
      64 disas
      32 docs
      31 dtc
       8 fpu
       1 gdbstub
       1 gdb-xml
       1 .github
     537 hw
       7 inc
     114 include
       1 libdecnumber
      33 linux-user
       1 MAINTAINERS
     150 meson
       6 meson.build
      16 migration
       1 nbd
       5 net
      12 pc-bios
       7 python
       3 qapi
       2 qemu
       5 qemu-options.hx
      22 qga
   14175 roms
      43 scripts
       3 semihosting
      18 slirp
       2 softmmu
      59 subprojects
     504 target
       6 tcg
       3 test.rb
     175 tests
       6 tools
      20 ui
       8 util

It shows that "roms" contributes by far the most typos. Omitting it 
would reduce the required time to 22 seconds and the number of typos 
found (2947 lines in output) very much.

"capstone" (which has no entry in MAINTAINERS), "target" and "hw" also 
contribute more than 300 hits each, therefore cc'ing Richard.

Stefan


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 6511 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [RFC PATCH] Add new build target 'check-spelling'

[Thomas Huth]

On 31/10/2022 11.44, Stefan Weil wrote:
> Am 31.10.22 um 08:52 schrieb Thomas Huth:
> 
>> On 31/10/2022 08.43, Stefan Weil wrote:
>>> `make check-spelling` can now be used to get a list of spelling errors.
>>> It uses the latest version of codespell, a spell checker implemented in 
>>> Python.
>>>
>>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>>> ---
>>>
>>> This RFC can already be used for manual tests, but still reports false
>>> positives, mostly because some variable names are interpreted as words.
>>> These words can either be ignored in the check, or in some cases the code
>>> might be changed to use different variable names.
>>>
>>> The check currently only skips a few directories and files, so for example
>>> checked out submodules are also checked.
>>>
>>> The rule can be extended to allow user provided ignore and skip lists,
>>> for example by introducing Makefile variables CODESPELL_SKIP=userfile
>>> or CODESPELL_IGNORE=userfile. A limited check could be implemented by
>>> providing a base directory CODESPELL_START=basedirectory, for example
>>> CODESPELL_START=docs.
>>>
>>> Regards,
>>> Stefan
> [...]
>>> I like the idea, but I think it's unlikely that we can make this work for 
>>> the whole source tree any time soon. So maybe it makes more sense to 
>>> start with some few directories first (e.g. docs/ ) and then the 
>>> maintainers can opt-in by cleaning up their directories first and then by 
>>> adding their directories to this target here?
>>
>>  Thomas
> 
> 
> Even without implementing CODESPELL_START as described above, the script can 
> already be used and integrated into CI scripts.
> 
> It takes about 60 seconds to check the whole source tree including 
> submodules on my (slow) virtual machine.
> 
> The resulting output has about 20000 lines or 1272 KiB. It can be filtered 
> for relevant parts of the source tree or used for a summary.
> 
> Sample script: grep "^[.]" spellcheck.log | sed s/^..// | sed 's/\/.*//' | 
> sed s/:.*// | sort | uniq -c
> 
> This produces a summary for the top level hierarchy of files and directories:
> 
>        3 accel
>        1 audio
>        1 backends
>       77 block
>        7 block.c
>       20 bsd-user
>      386 capstone
>       12 chardev
>        1 configure
>        8 contrib
>        6 crypto
>       64 disas
>       32 docs
>       31 dtc
>        8 fpu
>        1 gdbstub
>        1 gdb-xml
>        1 .github
>      537 hw
>        7 inc
>      114 include
>        1 libdecnumber
>       33 linux-user
>        1 MAINTAINERS
>      150 meson
>        6 meson.build
>       16 migration
>        1 nbd
>        5 net
>       12 pc-bios
>        7 python
>        3 qapi
>        2 qemu
>        5 qemu-options.hx
>       22 qga
>    14175 roms
>       43 scripts
>        3 semihosting
>       18 slirp
>        2 softmmu
>       59 subprojects
>      504 target
>        6 tcg
>        3 test.rb
>      175 tests
>        6 tools
>       20 ui
>        8 util
> 
> It shows that "roms" contributes by far the most typos. Omitting it would 
> reduce the required time to 22 seconds and the number of typos found (2947 
> lines in output) very much.

"roms" mostly consists of third-party submodules that we do not have direct 
control of. I think this should definitely be omitted.

> "capstone" (which has no entry in MAINTAINERS)

That's likely because it has been a submodule that has been removed a while 
ago. "rm -rf capstone" should solve that issue on your local buildtree ;-)

(yes, that's another nuisance of submodules - the checked out files don't go 
away when the submodule gets removed)

  Thomas

Re: [RFC PATCH] Add new build target 'check-spelling'

[Daniel P. Berrangé]

On Mon, Oct 31, 2022 at 11:44:48AM +0100, Stefan Weil via wrote:
> Am 31.10.22 um 08:52 schrieb Thomas Huth:
> 
> > On 31/10/2022 08.43, Stefan Weil wrote:
> > > `make check-spelling` can now be used to get a list of spelling errors.
> > > It uses the latest version of codespell, a spell checker implemented
> > > in Python.
> > > 
> > > Signed-off-by: Stefan Weil <sw@weilnetz.de>
> > > ---
> > > 
> > > This RFC can already be used for manual tests, but still reports false
> > > positives, mostly because some variable names are interpreted as words.
> > > These words can either be ignored in the check, or in some cases the
> > > code
> > > might be changed to use different variable names.
> > > 
> > > The check currently only skips a few directories and files, so for
> > > example
> > > checked out submodules are also checked.
> > > 
> > > The rule can be extended to allow user provided ignore and skip lists,
> > > for example by introducing Makefile variables CODESPELL_SKIP=userfile
> > > or CODESPELL_IGNORE=userfile. A limited check could be implemented by
> > > providing a base directory CODESPELL_START=basedirectory, for example
> > > CODESPELL_START=docs.
> > > 
> > > Regards,
> > > Stefan
> [...]
> > > I like the idea, but I think it's unlikely that we can make this
> > > work for the whole source tree any time soon. So maybe it makes more
> > > sense to start with some few directories first (e.g. docs/ ) and
> > > then the maintainers can opt-in by cleaning up their directories
> > > first and then by adding their directories to this target here?
> > 
> >  Thomas
> 
> 
> Even without implementing CODESPELL_START as described above, the script can
> already be used and integrated into CI scripts.

To get most value from CI, we strongly prefer the test to be a clear
pass/fail.

We do have some jobs that are marked non-gating, since they have
false failures and need manual inspection of results. The effect
is those jobs are largely ignored by everyone, so not really of
significant benefit.

So I'd agree with Thomas about starting with a config that can
get a clear pass/fail, and expanding from there if we can't get
the full-tree clean from the start.

> It shows that "roms" contributes by far the most typos. Omitting it would
> reduce the required time to 22 seconds and the number of typos found (2947
> lines in output) very much.

We should not look at 'roms' at all since it is just a place for
git submodulees, and build system integration. No interesting code
lives there.

> "capstone" (which has no entry in MAINTAINERS), "target" and "hw" also
> contribute more than 300 hits each, therefore cc'ing Richard.

We should completely ignoring 'capstone' and any other git submodule
as those are 3rd party codebases we don't maintain ourselves.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [RFC PATCH] Add new build target 'check-spelling'

[Philippe Mathieu-Daudé]

On 31/10/22 08:43, Stefan Weil via wrote:
> `make check-spelling` can now be used to get a list of spelling errors.
> It uses the latest version of codespell, a spell checker implemented in Python.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
> 
> This RFC can already be used for manual tests, but still reports false
> positives, mostly because some variable names are interpreted as words.
> These words can either be ignored in the check, or in some cases the code
> might be changed to use different variable names.
> 
> The check currently only skips a few directories and files, so for example
> checked out submodules are also checked.
> 
> The rule can be extended to allow user provided ignore and skip lists,
> for example by introducing Makefile variables CODESPELL_SKIP=userfile
> or CODESPELL_IGNORE=userfile. A limited check could be implemented by
> providing a base directory CODESPELL_START=basedirectory, for example
> CODESPELL_START=docs.
> 
> Regards,
> Stefan
> 
>   tests/Makefile.include       | 10 ++++++++++
>   tests/codespell/README.rst   | 18 ++++++++++++++++++
>   tests/codespell/exclude-file |  3 +++
>   tests/codespell/ignore-words | 19 +++++++++++++++++++
>   tests/requirements.txt       |  1 +
>   5 files changed, 51 insertions(+)
>   create mode 100644 tests/codespell/README.rst
>   create mode 100644 tests/codespell/exclude-file
>   create mode 100644 tests/codespell/ignore-words

Just wondering about this list...

> +++ b/tests/codespell/ignore-words
> @@ -0,0 +1,19 @@
> +buid

What is 'buid'? PPC-specific apparently.

> +busses
> +dout
> +falt
> +fpr
> +hace
> +hax
> +hda
> +nd

Apparently 'NIC info'...

> +ot

Is 'ot' MemOp?

> +pard
> +parm
> +ptd
> +ser
> +som
> +synopsys
> +te

Is that 'target endianness'?

> +toke

Where is 'toke'?

> +ue
Where is 'ue'?

Re: [RFC PATCH] Add new build target 'check-spelling'

[Stefan Weil via]

Below I added some examples for the words which are currently ignored by 
codespell.

Am 31.10.22 um 16:40 schrieb Philippe Mathieu-Daudé:

> On 31/10/22 08:43, Stefan Weil via wrote:
>> `make check-spelling` can now be used to get a list of spelling errors.
>> It uses the latest version of codespell, a spell checker implemented 
>> in Python.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> This RFC can already be used for manual tests, but still reports false
>> positives, mostly because some variable names are interpreted as words.
>> These words can either be ignored in the check, or in some cases the 
>> code
>> might be changed to use different variable names.
>>
>> The check currently only skips a few directories and files, so for 
>> example
>> checked out submodules are also checked.
>>
>> The rule can be extended to allow user provided ignore and skip lists,
>> for example by introducing Makefile variables CODESPELL_SKIP=userfile
>> or CODESPELL_IGNORE=userfile. A limited check could be implemented by
>> providing a base directory CODESPELL_START=basedirectory, for example
>> CODESPELL_START=docs.
>>
>> Regards,
>> Stefan
>>
>>   tests/Makefile.include       | 10 ++++++++++
>>   tests/codespell/README.rst   | 18 ++++++++++++++++++
>>   tests/codespell/exclude-file |  3 +++
>>   tests/codespell/ignore-words | 19 +++++++++++++++++++
>>   tests/requirements.txt       |  1 +
>>   5 files changed, 51 insertions(+)
>>   create mode 100644 tests/codespell/README.rst
>>   create mode 100644 tests/codespell/exclude-file
>>   create mode 100644 tests/codespell/ignore-words
>
> Just wondering about this list...
>
>> +++ b/tests/codespell/ignore-words
>> @@ -0,0 +1,19 @@
>> +buid
>
> What is 'buid'? PPC-specific apparently.

hw/ppc/spapr_pci.c:SpaprPhbState *spapr_pci_find_phb(SpaprMachineState 
*spapr, uint64_t buid)
include/hw/ppc/xics.h: * We currently only support one BUID which is our 
interrupt base
[...]


>> +busses
>> +dout
>> +falt
>> +fpr
>> +hace
>> +hax
>> +hda
>> +nd
>
> Apparently 'NIC info'...
hw/arm/aspeed.c:    NICInfo *nd = &nd_table[0];
hw/display/macfb.c:    NubusDevice *nd = NUBUS_DEVICE(s);
[...]


>> +ot
>
> Is 'ot' MemOp?

target/i386/tcg/decode-new.c.inc:static bool decode_op_size(DisasContext 
*s, X86OpEntry *e, X86OpSize size, MemOp *ot)
[...]


>> +pard
>> +parm
>> +ptd
>> +ser
>> +som
>> +synopsys
>> +te
>
> Is that 'target endianness'?

accel/tcg/cputlb.c: * @te: pointer to CPUTLBEntry
hw/audio/cs4231a.c:#define TE (1 << 6)
[...]


>> +toke
>
> Where is 'toke'?

This one is no longer needed. It was used in the old capstone code which 
I still had in my local sources.


>> +ue
> Where is 'ue'?
tests/tcg/i386/test-i386-fp-exceptions.c:#define UE (1 << 4)
tests/unit/test-keyval.c:    qdict = keyval_parse("val,,ue", "implied", 
NULL, &err);
[...]

I simply had added some examples of "words" which occurred often and 
which were reported by codespell as typos. These "typos" occur at least 
10 times (list produced with `grep "^[a-z]" codespell.log | sort -n +1`):

statics      10
regiser      11
usig         11
inh          12
tne          12
overriden    13
inactivate   15
upto         15
hsa          16
useable      17
daa          18
crate        21
endianess    22
olt          22
sring        23
vill         25
keypairs     35
gir          46
sav          47
asign       120
inflight    191

Some of them are real typos, others like aSign or statics are variable 
names and should be ignored, too.

Stefan