Skip to content

debian: Fix udev rule triggering - #92

Open
Michaël Blanc (mika-blanc) wants to merge 1 commit into
qualcomm-linux:qcom/ubuntu/resolutefrom
mika-blanc:udev-fix
Open

Michaël Blanc (mika-blanc) wants to merge 1 commit into
qualcomm-linux:qcom/ubuntu/resolutefrom
mika-blanc:udev-fix

Conversation

@mika-blanc

@mika-blanc Michaël Blanc (mika-blanc) commented Aug 28, 2026

Copy link
Copy Markdown

Most users expect their fastrpc use cases to work after installation of fastrpc-support without retriggered udev rules, so we trigger fastrpc udev rules in the postinst script (dh_installudev doesn't do it).

@shoudil

Copy link
Copy Markdown

This PR is to fix same issue: #86

@mika-blanc

Copy link
Copy Markdown
Author

Ok, I removed the fastrpc group creation part of my commit and only kept the udev rule triggering part, which users installing fastrpc-support might expect so they don't have to reboot to use the dev nodes.

@mika-blanc Michaël Blanc (mika-blanc) changed the title debian: Fix fastrpc group creation and udev rule triggering debian: Fix udev rule triggering Sep 8, 2026
@mika-blanc

Copy link
Copy Markdown
Author

Waiting for #86 to be merged first.

@ekanshibu
Ekansh Gupta (ekanshibu) dismissed their stale review September 8, 2026 09:17

Please add Signed-off-by tag to the commit

@lool Loïc Minier (lool) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks Michaël Blanc (@mika-blanc), that's an interesting idea!

I've left a few review comments; you can ignore the one on "which"

I'm not 100% this change is actually needed though: doesn't udevd watch changes to /usr/lib/udev/rules.d?

Comment thread debian/fastrpc-support.postinst Outdated
Comment thread debian/fastrpc-support.postinst Outdated
#DEBHELPER#

if [ "$1" = "configure" ]; then
if which udevadm >/dev/null 2>&1 ; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"which" is fine, consider "command" as a shell builtin instead of an external command

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'd rather use "which", "command" triggers research in apt if the command is not found on Ubuntu. Instead I simplified the command to "which -s" for silent operation.

Comment thread debian/fastrpc-support.postinst Outdated
Comment thread debian/fastrpc-support.postinst Outdated
@lool

Copy link
Copy Markdown

I've tweaked description to drop the sysuser parts.

Most users expect their fastrpc use cases to work after installation of
fastrpc-support without retriggered udev rules, so we trigger the
fastrpc udev rules in the postinst script (dh_installudev doesn't do
it).
@mika-blanc

Copy link
Copy Markdown
Author

Hi Loïc Minier (@lool)

I'm not 100% this change is actually needed though: doesn't udevd watch changes to /usr/lib/udev/rules.d?

Yes, udevd reloads rules on change, so I removed the "--reload-rules" command altogether. However, triggering these rules for /dev/fastrpc-* is not automatic on running boards since these device nodes were already brought up at the time user installs fastrpc packages.

@lool Loïc Minier (lool) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I've left one more minor nit, but I mostly have one important request: would you mind adding a Signed-off-by? (git commit --amend -s)

Shoudi Li (@shoudil) otherwise ready for merging, I'd like the same change in the debian/latest though, and this branch depends on the other changes, so perhaps worth rebasing on top of debian/latest?

#!/bin/sh
set -e

#DEBHELPER#

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

would perhaps be nice to underline that this needs to be before the snippet below

(FYI, this branch needs to land after a rebase/merge on top of debian/latest, otherwise dh-sequence-installsysusers is missing. – Cc: Shoudi Li (@shoudil))

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