Skip to content
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

Remove version argument check from initramfs cleanup script #3859

Merged
merged 1 commit into from Jun 11, 2022
Merged

Remove version argument check from initramfs cleanup script #3859

merged 1 commit into from Jun 11, 2022

Conversation

MichaIng
Copy link
Contributor

@MichaIng MichaIng commented Jun 5, 2022

Description

The version argument is not used by the script anymore, hence the check is obsolete.

Additionally make use of the currently unused defined MOD_DIR variable.

Jira reference number [AR-9999]

How Has This Been Tested?

Running the script locally, the change otherwise is trivial.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

The version argument is not used by the script anymore, hence the check is obsolete.

Additionally make use of the currently unused defined MOD_DIR variable.

Signed-off-by: MichaIng <micha@dietpi.com>
@igorpecovnik igorpecovnik added the Ready to merge Reviewed, tested and ready for merge label Jun 11, 2022
@igorpecovnik igorpecovnik merged commit 1b6e851 into armbian:master Jun 11, 2022
@MichaIng MichaIng deleted the patch-1 branch June 12, 2022 12:29
@The-going
Copy link
Contributor

@MichaIng What code did you use to debug this fix?

@MichaIng
Copy link
Contributor Author

It's not a fix, just a cleanup after the last rework of this script. I ran it once on console with a fake initramfs image, nothing more. The change otherwise is trivial: Before the last rework, $version was used to derive which initramfs images to remove, now it's done based on the fact whether a matching modules dir exists or not, that way supporting multiple installed kernel images, so the $version variable is obsolete.

Why are you asking, is there an issue with this change?

@The-going
Copy link
Contributor

Thanks. I understood. You did the right thing, but you didn't finish it. We have another such script. It also needs to be fixed. But this correction and the subsequent one require additional checks. You don't have to worry. I'll check it out and let you know when I make the changes.

@MichaIng
Copy link
Contributor Author

Ah you're right, I missed the postrm one: https://github.com/armbian/build/blob/master/packages/bsp/common/etc/kernel/postrm.d/xx-initramfs-cleanup

It can be changed the same way, let me know if I should open a PR accordingly.

@The-going
Copy link
Contributor

It can be changed the same way, let me know if I should open a PR accordingly.

No you don't need to.

version="$1"
[ -x /usr/sbin/update-initramfs ] || exit 0
# passing the kernel version is required
if [ -z "$version" ]; then
echo >&2 "W: initramfs-tools: ${DPKG_MAINTSCRIPT_PACKAGE:-kernel package} did not pass a version number"
exit 0
fi

If the script is run without arguments, it should not be executed!
You removed this check.
This alerted me.

@MichaIng
Copy link
Contributor Author

The way the initramfs images are cleaned now, is actually always safe to do, regardless how/when the script is executed, isn't it?

@The-going
Copy link
Contributor

Give me time to check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge Reviewed, tested and ready for merge
Development

Successfully merging this pull request may close these issues.

None yet

3 participants