kmax
kmax copied to clipboard
koverage unable to handle new file modes/permissions
Description
- When checking diffs from patches to create a coverage report,
koverage
may run into an error as its existing logic is not built to handle file changes that only modify the permissions for a file.
Steps to reproduce
Steps followed
To check the coverage of a randconfig file on a set of patches, I followed these steps:
- Clone the Linux kernel with
git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
and enter the directory. - Create a diff from a range of patches using
git diff {commit1}..{commit2} > patchset.diff
. I initially encountered this issue with the range0e8b53979ac86eddb3fd76264025a70071a25574
to4d7c3c1aba3ca12fad2e90163b8d5153363f93e5
. - Create a
randconfig
withmake randconfig
. Add any modifications to the configuration file if desired. - Check coverage of the configuration file with
koverage --config .config --arch x86_64 --check-patch patchset.diff -o coverage_results.json
.
What I expected to happen
- I expected
koverage
to successfully output coverage information intocoverage_results.json
.
What actually happened
-
koverage
ran into an error:
alexei@turing:~/LinuxKernels/hybridtest/linux$ koverage --config .config --arch x86_64 --check-patch patchset.diff -o coverage_results.json
koverage, kmax v4.7.3
INFO: Intermediate files will be saved in "koverage_files/"
Traceback (most recent call last):
File "/home/alexei/.local/bin/koverage", line 1114, in <module>
main()
File "/home/alexei/.local/bin/koverage", line 1019, in main
covreq_check_patch = parse_check_patch_args(check_patch_arg)
File "/home/alexei/.local/bin/koverage", line 811, in parse_check_patch_args
new_covreqs = get_patch_target_lines(a)
File "/home/alexei/.local/bin/koverage", line 775, in get_patch_target_lines
covreq = patch.get_target_lines(patch_txt)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/patch.py", line 200, in get_target_lines
patch_summary = summarize_patch(patch_txt)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/patch.py", line 60, in summarize_patch
change_type = FileChangeType.getType(d)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/common.py", line 137, in getType
else: assert False
AssertionError
Additional information
- I added some debugging print statements to the affected code at
common.py:137
:
class FileChangeType(enum.Enum):
CREATED = 1
REMOVED = 2
MOVED_ONLY = 3 # file was moved but the content didn't change
MOVED_MODIFIED = 4 # file was moved and the content was modified
MODIFIED_ONLY = 5 # content was modified but the file wasn't moved
@classmethod
def getType(cls, diff):
none_file="/dev/null"
assert not (diff.header.old_path == none_file and diff.header.new_path == none_file)
if diff.header.old_path == none_file:
print(f"DEBUG: Created file, old_path = {diff.header.old_path}, none_file = {none_file}")
return FileChangeType.CREATED
elif diff.header.new_path == none_file:
print(f"DEBUG: Removed file, new_path = {diff.header.new_path}, none_file = {none_file}")
return FileChangeType.REMOVED
elif diff.header.old_path != diff.header.new_path:
# filename changed
if diff.changes:
print(f"DEBUG: File moved and content modified, old_path = {diff.header.old_path}, new_path = {diff.header.new_path}")
return FileChangeType.MOVED_MODIFIED
else:
print(f"DEBUG: File moved and content UNmodified, old_path = {diff.header.old_path}, new_path = {diff.header.new_path}")
return FileChangeType.MOVED_ONLY
elif diff.header.old_path == diff.header.new_path and diff.changes:
print(f"DEBUG: Content modified, old_path = {diff.header.old_path}, new_path = {diff.header.new_path}")
return FileChangeType.MODIFIED_ONLY
elif "deleted file" in diff.text:
# When deleted a file, some patches might have the same file path for
# both before and after instead of none_file name. See
# e8bf1f522aee3b3e1e7658e8f224dca1d88c3338 for the Linux kernel.
print(f"DEBUG: Removed file, new_path = {diff.header.new_path}, none_file = {none_file}")
return FileChangeType.REMOVED
else:
print(f"DEBUG: Assertion failed, unhandled case. old_path = {diff.header.old_path}, new_path = {diff.header.new_path}, none_file = {none_file}, diff.text = {diff.text}")
assert False # all cases are covered above
- I found this:
alexei@turing:~/LinuxKernels/hybridtest/linux$ koverage --config .config --arch x86_64 --check-patch patchset.diff -o coverage_results.json
koverage, kmax v4.7.3
INFO: Intermediate files will be saved in "koverage_files/"
DEBUG: Content modified, old_path = Documentation/admin-guide/kernel-parameters.txt, new_path = Documentation/admin-guide/kernel-parameters.txt
DEBUG: Content modified, old_path = Documentation/arch/arm64/silicon-errata.rst, new_path = Documentation/arch/arm64/silicon-errata.rst
DEBUG: Content modified, old_path = Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml, new_path = Documentation/devicetree/bindings/ata/rockchip,dwc-ahci.yaml
DEBUG: Content modified, old_path = Documentation/driver-api/thermal/sysfs-api.rst, new_path = Documentation/driver-api/thermal/sysfs-api.rst
DEBUG: Content modified, old_path = Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst, new_path = Documentation/userspace-api/media/v4l/pixfmt-yuv-luma.rst
DEBUG: Content modified, old_path = Documentation/virt/kvm/api.rst, new_path = Documentation/virt/kvm/api.rst
DEBUG: Content modified, old_path = MAINTAINERS, new_path = MAINTAINERS
DEBUG: Content modified, old_path = Makefile, new_path = Makefile
DEBUG: Content modified, old_path = arch/alpha/include/asm/io.h, new_path = arch/alpha/include/asm/io.h
DEBUG: Content modified, old_path = arch/arm/boot/dts/arm/versatile-ab.dts, new_path = arch/arm/boot/dts/arm/versatile-ab.dts
DEBUG: Content modified, old_path = arch/arm64/Kconfig, new_path = arch/arm64/Kconfig
DEBUG: Content modified, old_path = arch/arm64/include/asm/cputype.h, new_path = arch/arm64/include/asm/cputype.h
DEBUG: Content modified, old_path = arch/arm64/include/asm/jump_label.h, new_path = arch/arm64/include/asm/jump_label.h
DEBUG: Content modified, old_path = arch/arm64/kernel/Makefile.syscalls, new_path = arch/arm64/kernel/Makefile.syscalls
DEBUG: Content modified, old_path = arch/arm64/kernel/cpu_errata.c, new_path = arch/arm64/kernel/cpu_errata.c
DEBUG: Content modified, old_path = arch/arm64/kernel/jump_label.c, new_path = arch/arm64/kernel/jump_label.c
DEBUG: Content modified, old_path = arch/loongarch/kernel/Makefile.syscalls, new_path = arch/loongarch/kernel/Makefile.syscalls
DEBUG: Content modified, old_path = arch/parisc/Kconfig, new_path = arch/parisc/Kconfig
DEBUG: Content modified, old_path = arch/parisc/include/asm/cache.h, new_path = arch/parisc/include/asm/cache.h
DEBUG: Content modified, old_path = arch/parisc/net/bpf_jit_core.c, new_path = arch/parisc/net/bpf_jit_core.c
DEBUG: Content modified, old_path = arch/riscv/kernel/Makefile.syscalls, new_path = arch/riscv/kernel/Makefile.syscalls
DEBUG: Content modified, old_path = arch/riscv/kernel/cpufeature.c, new_path = arch/riscv/kernel/cpufeature.c
DEBUG: Content modified, old_path = arch/riscv/kernel/sbi-ipi.c, new_path = arch/riscv/kernel/sbi-ipi.c
DEBUG: Content modified, old_path = arch/riscv/mm/fault.c, new_path = arch/riscv/mm/fault.c
DEBUG: Content modified, old_path = arch/riscv/mm/init.c, new_path = arch/riscv/mm/init.c
DEBUG: Content modified, old_path = arch/riscv/purgatory/entry.S, new_path = arch/riscv/purgatory/entry.S
DEBUG: Assertion failed, unhandled case. old_path = arch/s390/kernel/alternative.h, new_path = arch/s390/kernel/alternative.h, none_file = /dev/null, diff.text = diff --git a/arch/s390/kernel/h
new file mode 100644
index 000000000000..e69de29bb2d1
Traceback (most recent call last):
File "/home/alexei/.local/bin/koverage", line 1114, in <module>
main()
File "/home/alexei/.local/bin/koverage", line 1019, in main
covreq_check_patch = parse_check_patch_args(check_patch_arg)
File "/home/alexei/.local/bin/koverage", line 811, in parse_check_patch_args
new_covreqs = get_patch_target_lines(a)
File "/home/alexei/.local/bin/koverage", line 775, in get_patch_target_lines
covreq = patch.get_target_lines(patch_txt)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/patch.py", line 200, in get_target_lines
patch_summary = summarize_patch(patch_txt)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/patch.py", line 60, in summarize_patch
change_type = FileChangeType.getType(d)
File "/home/alexei/.local/share/pipx/venvs/kmax/lib/python3.8/site-packages/kmax/common.py", line 145, in getType
assert False # all cases are covered above
AssertionError
alexei@turing:~/LinuxKernels/hybridtest/linux$
- This indicates that the file mode for
arch/s390/kernel/alternative.h
simply got changed to 100644, as indicated in this Stack Overflow discussion. - To fix this bug, one may need to add another type to FileChangeType, such as
PERMISSION_CHANGE
, as well as any associated logic with that code tokoverage
andkmax
.