-
-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use samefile() instead of regex matching when checking bootloader kernel paths #43
Conversation
ecleankernel/process.py
Outdated
def get_kernel_images(kernel_files: typing.List[GenericFile] | ||
) -> typing.Iterable[KernelImage]: | ||
return (kernel_file.path | ||
for kernel_file in kernel_files | ||
if type(kernel_file) is KernelImage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need to do this, you can just match against all files.
Now if you wanted to optimize this, you could collect (st_dev, st_ino)
tuples for files and match against that but I don't think it's technically necessary here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed. I just thought it would be silly (though not strictly wrong) to match against all files. In lilo.conf
(and I believe grub.cfg
) I do only expect to find a reference to a KernelImage
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know but I think it's better to keep the code simple.
…nel paths This allows eclean-kernel to correctly identify the kernels used by the bootloader config in the following scenario: 1. LILO says `image=/boot/vmlinuz` 2. `/boot/vmlinuz` is a symlink to `/boot/vmlinuz-X.Y.Z-suffix`, which is the actual kernel. Before this commit, eclean-kernel -ap woulc complain that Note: strangely named used kernel: /boot/vmlinuz and then list `vmlinuz-X.Y.Z-suffix` as one of the kernels to be removed.
Do you want to fix the QA issues reported by CI or should I do it while merging? |
Sorry about that! Should all be fixed now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
I'll release 2.99.6 shortly. |
Thanks! You do not need to make me the reason for a hasty release. I can use |
This allows eclean-kernel to correctly identify the kernels used by the bootloader config in the following scenario:
image=/boot/vmlinuz
/boot/vmlinuz
is a symlink to/boot/vmlinuz-X.Y.Z-suffix
, which is the actual kernel.Before this commit, eclean-kernel -ap woulc complain that
and then list
vmlinuz-X.Y.Z-suffix
as one of the kernels to be removed.Closes #42