这是indexloc提供的服务，不要输入任何密码
Skip to content

fix(main/zig): check the Kernel version and stop if unsupported at install #25126

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix(main/zig): check the Kernel version and stop if unsupported at install #25126

wants to merge 1 commit into from

Conversation

TomJo2000
Copy link
Member

@TomJo2000 TomJo2000 requested a review from Grimler91 as a code owner June 20, 2025 23:12
@TomJo2000
Copy link
Member Author

CC: @Biswa96 @0komo @truboxl

@TomJo2000 TomJo2000 requested review from truboxl and Biswa96 June 20, 2025 23:17
@TomJo2000 TomJo2000 force-pushed the zig-kernel-warning branch from 35aa8a6 to 737dbfb Compare June 20, 2025 23:19
@TomJo2000 TomJo2000 force-pushed the zig-kernel-warning branch from 737dbfb to 1e215d0 Compare June 21, 2025 01:23
@truboxl
Copy link
Contributor

truboxl commented Jun 21, 2025

  1. You forgot to sed @TERMUX_PREFIX@ causing
Get:1 /data/data/com.termux/files/home/downloads/debs/zig_0.14.1-1_aarch64.deb zig aarch64 0.14.1-1 [46.0 MB]
(Reading database ... 43907 files and directories currently installed.)
Preparing to unpack .../debs/zig_0.14.1-1_aarch64.deb ...
dpkg (subprocess): unable to execute new zig package pre-installation script (/data/data/com.termux/files/usr/var/lib/dpkg//tmp.ci/preinst): No such file or directory
dpkg: error processing archive /data/data/com.termux/files/home/downloads/debs/zig_0.14.1-1_aarch64.deb (--unpack):
 new zig package pre-installation script subprocess returned error exit status 2
Errors were encountered while processing:
 /data/data/com.termux/files/home/downloads/debs/zig_0.14.1-1_aarch64.deb
Error: Sub-process /data/data/com.termux/files/usr/bin/dpkg returned an error code (1)
  1. I don't think displaying conditional warning should exit 1 mid installation. A warning is NOT an error and should not break installation / upgrade.

@TomJo2000
Copy link
Member Author

TomJo2000 commented Jun 21, 2025 •
edited
Loading

  1. You forgot to sed @TERMUX_PREFIX@ causing

There goes another 4 hours...

  1. I don't think displaying conditional warning should exit 1 mid installation. A warning is NOT an error and should not break installation / upgrade.

If the package cannot function on the device it is being installed on, should that package not stop its own installation?

I suspect a simple warning and continue will not be sufficiently clear but I'm happy to have a discussion about it.
(Preferably before I push the sed fix so we don't waste another 4 hours of CI time.)

Grimler91
Grimler91 reviewed Jun 23, 2025
View reviewed changes
packages/zig/preinst.sh.in
fi
;;
pacman)
if [ "$(vercmp "$_KERNEL_VERSION" "$_MINIMUM_VERSION")" != 1 ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will return 0 if kernel version is 3.16.0, should probably check if it is -1 instead

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh you're right I flipped the condition 3.16.0 would return 0 which is fine.

packages/zig/preinst.sh.in
echo "Your device does not support any version of Zig"
echo "There's nothing we can do to make it supported."
fi
exit 1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing in the preinst script is not a good idea, users then manually have to clean up by removing zig. It would be better to have the kernel version check in the actual zig binary.

Next best option would be to exit without an error here even if kernel version is not supported by zig, the error/warning is then printed and users might understand why the program does not work

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the package cannot function on the device it is being installed on, should that package not stop its own installation?

If stopping meant that it uninstalled itself again and cleaned everything up, then that would work. When preinst script fails it leaves apt unusable though until user manually fixes it, and I don't think that is a situation we should ever create on purpose

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep that's my mistake.
We can probably throw up a read -t 60 "press any key to continue" style warning.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nvm -t is a bash addition to read

@TomJo2000 TomJo2000 marked this pull request as draft June 24, 2025 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Grimler91 Grimler91 Grimler91 left review comments

@truboxl truboxl Awaiting requested review from truboxl

@Biswa96 Biswa96 Awaiting requested review from Biswa96

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Zig SIGILL (illegal opecodes) [Bug]: Running some zig commands gives "illegal instruction"
3 participants
@TomJo2000 @truboxl @Grimler91