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

Add KMP package and some cleanup #5

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

darix
Copy link

@darix darix commented Aug 15, 2021

A while ago i promised a KMP port for openSUSE so we do not need build
tools on the user machines.

While I did this i did some cleanup to the spec file

A more readable diff might be:

diff -urwN {hardware:razer,home:darix:playground}/openrazer/openrazer.spec

  1. renaming main package to openrazer because the kmp packages are named -kmp
    and the main package needs to be an arch specific package so the kmp
    macros work. So I move the meta package to a subpackage and marked the
    subpackage noarch. The 2 python packages are marked noarch for the same
    reason now.

  2. which handling of the kernel module is being used is decided with the
    conditional on top. to keep the configuration in one place, I also
    defined a variable in the same place for the driver package

    so if you want to build a dkms version for opensuse:

    osc build --without=openrazer_kmp openSUSE_Tumbleweed

    In theory you could do a kmp build on non opensuse distros but this should just fail.

  3. moved the udev rules and related handling to the daemon subpackage
    I think it makes more sense to have that here or can there be a valid
    use case where you want the driver but not at least the openrazer-daemon?

    If there is such an use case then we should probably move the udev
    rules + user handling into a subpackage like openrazer-udev so that
    the dkms and kmp can Requires it.

  4. generic cleanup

    1. BuildRequires can work in subpackages but it is usually preferred
      to have them all on the main package
    2. only run the the dkms test build if we are not in the OBS build
      env. Or do you prefer having it run all the time to make sure it
      works? I also fixed the requires for the dkms/make package and
      added a longer comment on why they are needed
    3. more consistent indenting in the preambles

This fixes issue #2.

@darix darix force-pushed the kmp-and-cleanup branch 2 times, most recently from ae3293e to 4b4389a Compare August 15, 2021 20:57
@darix
Copy link
Author

darix commented Aug 15, 2021

File list of the kmp rpm:

/lib/modules/5.13.8-1-default
/lib/modules/5.13.8-1-default/updates
/lib/modules/5.13.8-1-default/updates/razeraccessory.ko
/lib/modules/5.13.8-1-default/updates/razerkbd.ko
/lib/modules/5.13.8-1-default/updates/razerkraken.ko
/lib/modules/5.13.8-1-default/updates/razermouse.ko

@z3ntu
Copy link
Member

z3ntu commented Aug 16, 2021

Does OBS rebuild the package correctly when a new kernel version is available?

I also don't particularly like the way the packages are split with this change. In theory currently a user can just install the kernel driver package and use that standalone without the daemon (or pylib) package. So the udev rule and razer_mount script definitely need to stay in the kernel module package. Or if it's a separate package (just containing udev&mount script as a dependency of dkms/kmp package) then I also don't mind.

Also I've pushed the 3.1.0 update to this repo, I forgot to do that before, that's why there's a merge conflict now.

@darix
Copy link
Author

darix commented Aug 16, 2021

  1. yes the OBS rebuilds your package if underlying package change
  2. so i will add an openrazer-udev package and require that in the dkms and kmp package.

A while ago i promised a KMP port for openSUSE so we do not need build
tools on the user machines.

While I did this i did some cleanup to the spec file

A more readable diff might be:

diff -urwN {hardware:razer,home:darix:playground}/openrazer/openrazer.spec

1. renaming main package to openrazer because the kmp packages are named
   name_of_main_package-kmp and the main package needs to be an arch
   specific package so the kmp macros work.  So I move the meta package to
   a subpackage and marked the subpackage noarch. The 2 python packages are
   marked noarch for the same reason now.

2. which handling of the kernel module is being used is decided with the
   conditional on top. to keep the configuration in one place, I also
   defined a variable in the same place for the driver package

   so if you want to build a dkms version for opensuse:

   `osc build --without=openrazer_kmp openSUSE_Tumbleweed`

   In theory you could do a kmp build on non opensuse distros but this
   should just fail.

3. moved the udev rules and related handling to the daemon subpackage
   I think it makes more sense to have that here or can there be a valid
   use case where you want the driver but not at least the openrazer-daemon?

   If there is such an use case then we should probably move the udev
   rules + user handling into a subpackage like openrazer-udev so that
   the dkms and kmp can Requires it.

4. generic cleanup
   1. BuildRequires can work in subpackages but it is usually preferred
      to have them all on the main package
   2. only run the the dkms test build if we are not in the OBS build
      env. Or do you prefer having it run all the time to make sure it
      works?  I also fixed the requires for the dkms/make package and
      added a longer comment on why they are needed
   3. more consistent indenting in the preambles

This fixes issues openrazer#2
@darix
Copy link
Author

darix commented Aug 16, 2021

udev rules package done.

@darix
Copy link
Author

darix commented Aug 16, 2021

Thinking about it ... the current version does not have a provides/obsoletes for the dkms package in the kmp package.

Two options

  1. build the kmp and dkms package for opensuse but make sure only one is installed
  2. add provides/obsolete for the dkms into the kmp so we would automatically upgrade users to the precompiled modules.

which solution would you prefer?

we provide the symbol openrazer-driver for this and prefer the kmp
if it is build but the user has the choice to replace it with the dkms
package. both packages can not be installed at the same time.
@darix
Copy link
Author

darix commented Aug 18, 2021

I implemented the version where the kmp is only built additionally but the dkms is still available, the Recommends on openrazer-kmp makes zypp prefer the kmp if it is available.

@darix
Copy link
Author

darix commented Aug 21, 2021

Anything else you want to have changed?

@z3ntu
Copy link
Member

z3ntu commented Aug 22, 2021

Thinking about it ... the current version does not have a provides/obsoletes for the dkms package in the kmp package.

Two options

  1. build the kmp and dkms package for opensuse but make sure only one is installed
  2. add provides/obsolete for the dkms into the kmp so we would automatically upgrade users to the precompiled modules.

which solution would you prefer?

I don't know what's done normally in opensuse for out-of-tree kernel modules, so follow whatever is standard there.

@darix
Copy link
Author

darix commented Aug 23, 2021

I don't know what's done normally in opensuse for out-of-tree kernel modules, so follow whatever is standard there.

kmps are normal, but i left dkms in for people who prefer those. dkms have the advantage that they are not tied to the kernel that they are built against.

and it is all implemented now and i used it while testing the razer viper ultimate (still need to learn how to remap buttons on that device)

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

Successfully merging this pull request may close these issues.

2 participants