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

Aditional files to the debian package #12

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

orensbruli
Copy link
Contributor

@orensbruli orensbruli commented Nov 2, 2024

https://app.shortcut.com/greenzie/story/41781/fix-emuccan-installation-to-include-services-rules-etc

The main reason for these modifications is to add to the generated debian dkms package the files that were previously distributed with the package and that were lost when moving to the dkms version.

@edavies64 noticed that with the installation of the new package the emuccan.service file was missing. Reviewing the process, not only that file was missing, but also the udev rules and the default for bash.

This PR is created to try to solve that problem.
After searching for hours, with no luck, how to add extra files to a Debian package generated with dkms, we have proceeded with a different approach. The deb file is created with dkms, decompressed, the extra files are added, and then compressed again. It is not the most orthodox, but it seems to work and it avoids the other possible solution which is to create a new Debian package only with the extra packages that are not the module code.

In this PR we have also taken the opportunity to change the old CI and create a shell script that can be used both locally and in the CI, making this process clearer for any developer.

Changes

  • Updated Makefile to specify kernel headers dynamically.
  • Added build_dmks.sh script for streamlined DKMS build and packaging.
  • Modified .circleci/config.yml to align with the new artifact storage structure.
  • Removed redundant kernel dependency checks from build_dmks.sh.

Testing

It needs to be tested on a real mower, and everything that is required must be installed now.

@orensbruli orensbruli self-assigned this Nov 2, 2024
@mfsurvilo
Copy link

Can you add some descriptions about the practical impact of this? What will the impact be on the CANBus drivers we occasionally have hiccups with?

GITHUB_WORKSPACE="$PWD" greenzie-dkms mkdeb -d
cd /tmp/debians/ && tar cvf /tmp/debians/alldebs.tar *.deb
ls -lah
bash -x "$PWD/build_dmks.sh"
Copy link
Contributor Author

@orensbruli orensbruli Nov 3, 2024

Choose a reason for hiding this comment

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

New build script that can be readed to understand how the process works.

Comment on lines +10 to +34
echo "==> Checking for the module name and version in $DRIVER_CODE_DIR/dkms.conf"
# Define module name and version based on dkms.conf
MODULE_NAME=$(grep '^PACKAGE_NAME' "$DRIVER_CODE_DIR/dkms.conf" | awk -F'=' '{print $2}' | tr -d ' ')
VERSION=$(grep '^PACKAGE_VERSION' "$DRIVER_CODE_DIR/dkms.conf" | awk -F'=' '{print $2}' | tr -d ' ')

if [ -z "$MODULE_NAME" ] || [ -z "$VERSION" ]; then
echo "Error: Module name or version not found in dkms.conf"
exit 1
fi

echo "==> Module name: $MODULE_NAME"
echo "==> Version: $VERSION"

echo "==> Checking for required tools and dependencies..."
# Check if dkms is installed, install if missing
if ! command -v dkms &> /dev/null; then
echo "dkms command not found. Installing dkms..."
sudo apt update && sudo apt install -y dkms
fi

# Check if debhelper is installed, install if missing
if ! dpkg-query -W -f='${Status}' debhelper 2>/dev/null | grep -q "install ok installed"; then
echo "debhelper not found. Installing debhelper..."
sudo apt install -y debhelper
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Check that everything that is needed is installed or it installs it.

Comment on lines +44 to +52
# Add the module
if dkms status -m "$MODULE_NAME" -v "$VERSION" | grep -q "$VERSION"; then
echo "Module '$MODULE_NAME' already exists"
else
sudo dkms add -m "$MODULE_NAME" -v "$VERSION"
fi

# Create Debian package
sudo dkms mkdeb -m "$MODULE_NAME" -v "$VERSION" --kernelsourcedir="$DKMS_KERNEL_SOURCE_DIR"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use dkms to create a debian package with the source code of the module/driver.

Comment on lines +79 to +88
echo "==> We have found the .deb package at $DEB_FILE"
# Unpack the .deb package
if [ -f "$DEB_FILE" ]; then
echo "==> Unpacking $DEB_FILE for modification..."
mkdir -p "$UNPACK_DIR"
dpkg-deb -R "$DEB_FILE" "$UNPACK_DIR"
else
echo "ERROR: .deb file not found in $DST_DIR."
exit 1
fi
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unpack the created Debian file

Comment on lines +90 to +111
echo "==> We have unpacked the .deb package to $UNPACK_DIR"
echo "==> Before adding files, the unpacked package structure is as follows:"
ls "$UNPACK_DIR"
echo "==> Now we will add the necessary files to the unpacked package."

# Create necessary directories in unpacked package
mkdir -p "$UNPACK_DIR/lib/systemd/system"
mkdir -p "$UNPACK_DIR/lib/udev/rules.d"
mkdir -p "$UNPACK_DIR/etc/modules-load.d"
mkdir -p "$UNPACK_DIR/etc/default"

# Copy files into the unpacked structure
ls "$PROJECT_DIR"
cp "$PROJECT_DIR/emuccan.service" "$UNPACK_DIR/lib/systemd/system/emuccan.service"
cp "$PROJECT_DIR/emuccan.udev" "$UNPACK_DIR/lib/udev/rules.d/60-emuccan.rules"
cp "$PROJECT_DIR/emuccan.conf" "$UNPACK_DIR/etc/modules-load.d/emuccan.conf"
cp "$PROJECT_DIR/emuccan.default" "$UNPACK_DIR/etc/default/emuccan"

echo "==> We added the files to the unpacked package."
rm -rf "$UNPACK_DIR/usr/src/${MODULE_NAME}-${VERSION}/debian"
echo "==> After adding files, the unpacked package structure is as follows:"
ls "$UNPACK_DIR"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add the missing files

Comment on lines +114 to +125
# Rebuild the .deb package with the added files
echo "==> Now we will rebuild the .deb package with the added files."
echo "==> Repacking the modified .deb package..."
dpkg-deb -b "$UNPACK_DIR" "$DST_DIR/modified_${MODULE_NAME}_${VERSION}.deb"

# Clean up
rm -rf "$UNPACK_DIR"
echo "==> Repackaging complete. Modified package located at $DST_DIR/modified_${MODULE_NAME}_${VERSION}.deb"
echo "==> Replacing the original package with the modified package."
mv "$DST_DIR/modified_${MODULE_NAME}_${VERSION}.deb" "$DEB_FILE"
echo "==> Done. Final modified package is located at $DEB_FILE"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pack again and replace the generated one.

driver/Makefile Outdated
Comment on lines 4 to 5
KVERSION ?= $(ls /lib/modules | tail -n 1)
KERNEL_SRC ?= $(DKMS_KERNEL_SOURCE_DIR)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Build for the installed kernel sources instead of the reported by uname -r.
This resolves problems on containers and in the mowers we always have a fixed version of the kernel.

Comment on lines +1 to +2
PACKAGE_NAME=emuccan
PACKAGE_VERSION=2.6.1
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Remove extra quotes that generate a malfunction when parsed by shell scripts.

@orensbruli
Copy link
Contributor Author

Can you add some descriptions about the practical impact of this?

Yeah, sorry as it's a fix for something reported by Evan I missed adding more context (I thought you were also in the loop).
I added some descriptions and comments in the code itself to facilitate the review.

What will the impact be on the CANBus drivers we occasionally have hiccups with?
I don't think it will impact any hiccups you have seen recently as it's only related to the installation of some extra files that I think will not have any other impact on the functionality of the driver itself.

I'm also wondering if we have been using the dkms version of the module during the last few months. I think we have not as this is the first time that we realized that the service is not installed with the new deb package.

@edavies64
Copy link

Can you add some descriptions about the practical impact of this?

Yeah, sorry as it's a fix for something reported by Evan I missed adding more context (I thought you were also in the loop). I added some descriptions and comments in the code itself to facilitate the review.

What will the impact be on the CANBus drivers we occasionally have hiccups with?
I don't think it will impact any hiccups you have seen recently as it's only related to the installation of some extra files that I think will not have any other impact on the functionality of the driver itself.

I'm also wondering if we have been using the dkms version of the module during the last few months. I think we have not as this is the first time that we realized that the service is not installed with the new deb package.

  1. Correct. This is resolving a bug where we were correctly installing the kernel modules, but not installing the systemd service to configure the devices.
  2. Functionally, there is no impact to CANBus performance. The main impact is in ensuing proper install of the driver on a brand new hard drive.
  3. @orensbruli I can confirm that the dkms version of the module was getting installed. The version of this package (prior to switching to dkms) resulted in the service getting installed, but NOT the kernel module. Whereas, the dkms version resulted in the the kernel module installed but not the service. When we were debugging the most recent computer, I was able to confirm i) emuccan-dkms was installed, ii) the kernel module was installed and active, iii) the service was not installed.

build_dmks.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@AGummyBear AGummyBear left a comment

Choose a reason for hiding this comment

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

This all looks reasonable to me.

@orensbruli
Copy link
Contributor Author

orensbruli commented Nov 11, 2024

Ok. We have a problem:

$ sudo apt install ./emuccan-dkms_2.6.1_amd64.deb 
Reading package lists... Done
Building dependency tree       
Reading state information... Done
Note, selecting 'emuccan-dkms' instead of './emuccan-dkms_2.6.1_amd64.deb'
The following packages will be DOWNGRADED:
  emuccan-dkms
0 upgraded, 0 newly installed, 1 downgraded, 0 to remove and 30 not upgraded.
Need to get 0 B/17.2 kB of archives.
After this operation, 1024 B of additional disk space will be used.
Do you want to continue? [Y/n] Y
Get:1 /usr/home/chomper/emuccan-dkms_2.6.1_amd64.deb emuccan-dkms amd64 2.6.1 [17.2 kB]
(Reading database ... 185173 files and directories currently installed.)
Preparing to unpack .../emuccan-dkms_2.6.1_amd64.deb ...

-------- Uninstall Beginning --------
Module:  emuccan
Version: 2.6.1
Kernel:  5.4.0-96-generic (x86_64)
-------------------------------------

Status: Before uninstall, this module version was ACTIVE on this kernel.

emuc2socketcan.ko:
 - Uninstallation
   - Deleting from: /lib/modules/5.4.0-96-generic/updates/dkms/
 - Original module
   - No original module was found for this module on this kernel.
   - Use the dkms install command to reinstall any previous module version.


Running the post_remove script:
depmod....

DKMS: uninstall completed.

------------------------------
Deleting module version: 2.6.1
completely from the DKMS tree.
------------------------------
Done.
Unpacking emuccan-dkms (2.6.1) over (2.6.1) ...
dpkg: error processing archive /usr/home/chomper/emuccan-dkms_2.6.1_amd64.deb (--unpack):
 trying to overwrite '/etc/default/emuccan', which is also in package emuccan-b202-5.4.0-96-generic 2.7.2-greenzie-3
Errors were encountered while processing:
 /usr/home/chomper/emuccan-dkms_2.6.1_amd64.deb
E: Sub-process /usr/bin/dpkg returned an error code (1)

@orensbruli
Copy link
Contributor Author

Tested in GZDEV9. @edavies64 can we test this on a real mower and get it reviewed and merged?

@edavies64
Copy link

The 2 key things to verify after the package is installed.

  1. The kernel module is properly installed.
  2. The service is installed and enabled.
    I would recommend removing and confirming that the kernel module and service are deleted prior to installing the package to ensure that they haven't been previously installed (either manually or from a previous version of the package).

I'm not sure if we have any mowers with an emuccan module at the office (they're fairly rare these days)

@orensbruli
Copy link
Contributor Author

Testing in GZDEV25.

$ sudo apt install --reinstall ./emuccan-dkms_2.6.1_amd64.deb 
Reading package lists... Done
Building dependency tree       
Reading state information... Done
Note, selecting 'emuccan-dkms' instead of './emuccan-dkms_2.6.1_amd64.deb'
0 upgraded, 0 newly installed, 1 reinstalled, 0 to remove and 0 not upgraded.
After this operation, 0 B of additional disk space will be used.
Get:1 /usr/home/chomper/emuccan-dkms_2.6.1_amd64.deb emuccan-dkms amd64 2.6.1 [17.2 kB]
(Reading database ... 179533 files and directories currently installed.)
Preparing to unpack .../emuccan-dkms_2.6.1_amd64.deb ...

-------- Uninstall Beginning --------
Module:  emuccan
Version: 2.6.1
Kernel:  5.4.0-96-generic (x86_64)
-------------------------------------

Status: Before uninstall, this module version was ACTIVE on this kernel.

emuc2socketcan.ko:
 - Uninstallation
   - Deleting from: /lib/modules/5.4.0-96-generic/updates/dkms/
 - Original module
   - No original module was found for this module on this kernel.
   - Use the dkms install command to reinstall any previous module version.


Running the post_remove script:
depmod...

DKMS: uninstall completed.

------------------------------
Deleting module version: 2.6.1
completely from the DKMS tree.
------------------------------
Done.
Unpacking emuccan-dkms (2.6.1) over (2.6.1) ...
Setting up emuccan-dkms (2.6.1) ...
Loading new emuccan-2.6.1 DKMS files...
Building for 5.4.0-96-generic
Building for architecture x86_64
Building initial module for 5.4.0-96-generic
Secure Boot not enabled on this system.
Done.

emuc2socketcan.ko:
Running module version sanity check.
 - Original module
   - No original module exists within this kernel
 - Installation
   - Installing to /lib/modules/5.4.0-96-generic/updates/dkms/

Running the post_install script:
# Ensure the new module is loaded
override emuc2socketcan * updates/dkms/
emuc2socketcan

depmod...

DKMS: install completed.

Checks:

chomper@GZDEV25:~$ ls /lib/systemd/system/emuccan.service
/lib/systemd/system/emuccan.service
chomper@GZDEV25:~$ ls /lib/udev/rules.d/60-emuccan.rules
/lib/udev/rules.d/60-emuccan.rules
chomper@GZDEV25:~$ ls /etc/modules-load.d/emuccan.conf
/etc/modules-load.d/emuccan.conf
chomper@GZDEV25:~$ ls /etc/default/emuccan
/etc/default/emuccan
chomper@GZDEV25:~$ sudo dkms status
emuccan, 2.6.1, 5.4.0-96-generic, x86_64: installed
fintek-f81601-driver, 1.19, 5.4.0-96-generic, x86_64: installed
librealsense2-dkms, 1.3.18, 5.4.0-96-generic, x86_64: installed
wdt_dio, 1.1, 5.4.0-96-generic, x86_64: installed
chomper@GZDEV25:~$ lsmod | grep emuc2socketcan
emuc2socketcan         20480  0
can_dev                32768  2 emuc2socketcan,f81601
chomper@GZDEV25:~$ lsmod | grep emuc2socketcan^C
chomper@GZDEV25:~$ modinfo emuc2socketcan
filename:       /lib/modules/5.4.0-96-generic/updates/dkms/emuc2socketcan.ko
author:         Innodisk
alias:          Innodisk EMUC-B202
description:    Innodisk EMUC-B202 CAN interface driver
license:        GPL
version:        v2.6.1
srcversion:     1DB9DB9292E4BBD9CBD7DAD
depends:        can-dev
retpoline:      Y
name:           emuc2socketcan
vermagic:       5.4.0-96-generic SMP mod_unload modversions 
sig_id:         PKCS#7
signer:         ubuntu-server Secure Boot Module Signature key
sig_key:        3F:D5:C7:B7:FB:C7:CD:DD:47:E4:61:37:8D:F4:E6:F1:A4:E7:EA:5B
sig_hashalgo:   sha512
signature:      B2:31:A4:C5:EB:92:B6:31:C3:E5:89:B2:F1:9E:CF:74:FE:E9:81:9E:
		32:6E:6E:B6:34:14:8F:6B:88:39:B3:76:44:11:1A:40:50:52:A7:6F:
		6C:CB:56:53:B9:9B:B2:4E:26:94:79:A5:29:6D:AD:FA:A0:A8:BE:9F:
		31:79:92:58:D1:E1:2B:D6:8B:A5:47:33:43:BD:25:34:96:DB:61:CC:
		B1:68:FB:62:0C:3B:35:39:17:DA:E2:C4:77:CD:71:AC:98:B7:D7:08:
		FE:24:A8:89:B2:27:D1:9F:4C:13:51:82:F8:F0:BF:3D:0F:01:7B:D0:
		38:39:16:76:CB:36:7E:0F:42:71:42:37:7C:4F:93:35:4E:10:84:63:
		23:43:21:75:76:CB:11:57:ED:42:A3:51:F5:54:C3:3B:CE:F0:23:55:
		A1:EB:0F:5A:9A:2C:4E:50:C8:C2:5C:CC:BD:0B:5C:3F:5D:1D:AE:38:
		12:AF:6E:77:51:3F:73:25:75:C2:0A:F5:A9:CE:70:47:24:F5:50:E9:
		04:1C:93:D5:03:10:CF:A2:C7:0C:B1:83:F7:30:57:0C:D0:FB:FC:F9:
		8F:35:FA:53:B6:55:20:10:28:AD:CE:88:CF:16:92:23:E7:FC:D4:A7:
		A9:BE:DD:3F:73:0D:E4:AE:A0:E4:2B:F9:B3:7D:3E:4B
chomper@GZDEV25:~$ ls -l /lib/modules/$(uname -r)/updates/dkms/
total 676
-rw-r--r-- 1 root root  24332 Dec  2 16:00 emuc2socketcan.ko
-rw-r--r-- 1 root root  57652 Jul 11 19:31 f81601.ko
-rw-r--r-- 1 root root  17724 May 25  2023 hid-sensor-accel-3d.ko
-rw-r--r-- 1 root root  17020 May 25  2023 hid-sensor-gyro-3d.ko
-rw-r--r-- 1 root root 170612 May 25  2023 uvcvideo.ko
-rw-r--r-- 1 root root 366900 May 25  2023 videodev.ko
-rw-r--r-- 1 root root  20636 Jul 16 20:35 wdt_dio.ko
chomper@GZDEV25:~$ dmesg | grep emuc2socketcan

@orensbruli
Copy link
Contributor Author

Same results for GZ25!

@orensbruli
Copy link
Contributor Author

asciinema recording of both tests.
asciinema-gz25.txt
asciinema-gzdev25.txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants