kmax icon indicating copy to clipboard operation
kmax copied to clipboard

koverage unable to handle new file modes/permissions

Open lolrepeatlol opened this issue 6 months ago • 0 comments

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:

  1. Clone the Linux kernel with git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git and enter the directory.
  2. Create a diff from a range of patches using git diff {commit1}..{commit2} > patchset.diff. I initially encountered this issue with the range 0e8b53979ac86eddb3fd76264025a70071a25574 to 4d7c3c1aba3ca12fad2e90163b8d5153363f93e5.
  3. Create a randconfig with make randconfig. Add any modifications to the configuration file if desired.
  4. 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

  1. I expected koverage to successfully output coverage information into coverage_results.json.

What actually happened

  1. 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 to koverage and kmax.

lolrepeatlol avatar Aug 19 '24 18:08 lolrepeatlol