morrownr / rtl8852bu

Linux Driver for USB WiFi Adapters that are based on the RTL8832BU and RTL8852BU Chipsets - v1.19.3 - 20230505
Other
91 stars 19 forks source link

Fix issues encountered when packaging for Debian dkms #22

Closed RadxaYuntian closed 6 months ago

RadxaYuntian commented 6 months ago
RadxaYuntian commented 6 months ago

Build tested.

alkisg commented 6 months ago

I recommend this dkms.conf:

PACKAGE_NAME="rtl8852bu"
PACKAGE_VERSION="#MODULE_VERSION#"
BUILT_MODULE_NAME[0]="8852bu"
MAKE="arch='$arch' kernelver='$kernelver' ./dkms-make.sh"
CLEAN="make clean"
DEST_MODULE_LOCATION[0]="/updates/dkms"
AUTOINSTALL="YES"

I.e. don't hardcode PACKAGE_VERSION, also pass arch instead of only kernelver, and properly quote them.

RadxaYuntian commented 6 months ago

I.e. don't hardcode PACKAGE_VERSION

That's what we did in our package. But if you want the source code to work as a standalone dkms manual install, then you should provide a valid version number by default.

also pass arch instead of only kernelver

arch is not used in dkms-make.sh. Passing it makes no difference unless the script is updated to accommodate this as well. However, I did not encounter issues when cross installing it from a x86 machine to an aarch64 rootfs.

and properly quote them.

I'm trying to stay consistent with this commit. I also don't think we will encounter IFS in normal kernel version.

morrownr commented 6 months ago

@alkisg

PACKAGE_VERSION="#MODULE_VERSION#"

also pass arch instead of only kernelver

That's what we did in our package. But if you want the source code to work as a standalone dkms manual install, then you should provide a valid version number by default.

Remember that we have somewhat different goals so we need to be careful. I'm trying to make this driver available for as many different distros as possible so I have to consider a lot of things. I want users to be able to do standalone dkms manual installations and cross-compiles. Tell me why the following changes make things better and won't lead to problems in some situations:

PACKAGE_VERSION="#MODULE_VERSION#"

also pass arch instead of only kernelver

I'm perfectly willing to make the changes but I don't want to introduce bugs in certain cases.

@RadxaYuntian

Thanks for the PR. I had forgotten about the executable problem. This has come up before but it has been a long time ago. How did you turn off the executable bit for the code? ...

Since we need the bit set for the scripts, I assume you went to the directories core, include, os_dep, phl, and platform and ran:

$ chmod -R -x *

alkisg commented 6 months ago

RadxaYuntian's points are valid, please completely ignore my proposals, I should only open my mouth when I have enough time to properly review all the related upstream code! :disappointed: In short, in my customized version, I put #MODULE_VERSION# in debian/dkms, not in upstream/dkms.conf; I handle $arch in my debian/make.sh variant; while regarding quoting, it would indeed be rare to encounter IFS or wildcards like '*' so it's not important.

RadxaYuntian commented 6 months ago

Thanks for the PR. I had forgotten about the executable problem. This has come up before but it has been a long time ago. How did you turn off the executable bit for the code? ...

Since we need the bit set for the scripts, I assume you went to the directories core, include, os_dep, phl, and platform and ran:

$ chmod -R -x *

That will also remove the execution bit from directory, and then you cannot cd or view the content of them.

I ran find . -type f -exec chmod -x {} \;, then I took a look in VS Code's git panel, and revert the changes to shell scripts. Didn't do anything fancy for this one-off job.