Skip to content

PKGBUILD and DKMS to make it easier for arch users..#36

Merged
Gnarus-G merged 22 commits intoGnarus-G:mainfrom
pxlsec:main
Jan 26, 2025
Merged

PKGBUILD and DKMS to make it easier for arch users..#36
Gnarus-G merged 22 commits intoGnarus-G:mainfrom
pxlsec:main

Conversation

@pxlsec
Copy link
Copy Markdown
Contributor

@pxlsec pxlsec commented Dec 4, 2024

I took the time to rewrite some of the build system to make it easier to install, and maybe even possible to publish to the AUR. Anyways, I did some changes to the Makefile inside of the driver folder, so I have no clue if the original install script works properly at the moment. But ill take a look when I get some time left over.

Cheers <3

@vercel
Copy link
Copy Markdown

vercel Bot commented Dec 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
maccel ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 26, 2025 8:39pm

@Gnarus-G
Copy link
Copy Markdown
Owner

Gnarus-G commented Dec 4, 2024

@pxlsec Very much appreciate you working on this.

Now, I hate to be that guy but I want some better commit messages. Something like:
Create a PKGBUILD to build packages
The criteria roughly being:

  • First letter is capitalized
  • Imperative tense (e.g. "Add ...", not "Added ..." or "Adds ...")
  • Include as much detail as you can.

And, lol, I love that you're using the one true editor but that doesn't need to be part of the commit message.

So please rebase those commits when you can.

P.S. This how PR #35 is and I like it. My previous commits were in a different style which I am moving away from.

@pxlsec
Copy link
Copy Markdown
Contributor Author

pxlsec commented Dec 4, 2024

Now, I hate to be that guy but I want some better commit messages.

Haha, don't worry. As long as you had a good laugh I'm happy. Besides I need to learn how to use git & github properly anyways.
I also continued working on it a little bit and it feels decent now..

The good:

  • I can install the driver on my custom kernel build using clang.
  • All files are managed by the package, aka uninstalls are completely clean.
  • DKMS so that you don't have to manually install after every update.
  • Also installs to all kernels when installing the package.

The bad

  • The PKGBUILD currently pulls the whole repo again, even tho it's in the repo. This can be fixed quite easily but I don't know whether you want the package to be on the AUR or not.
  • DKMS only works when using the PKGBUILD (for now).
  • The module does not install itself, so you have to run modprobe maccel after install. It is highly discouraged to automate according to the arch wiki.
  • Debug builds are currently broken?

@Gnarus-G
Copy link
Copy Markdown
Owner

Gnarus-G commented Dec 4, 2024

Seems promising. Btw is it not possible to reuse the current install/uninstall scripts in the dkms configs or PKGBUILD?
And remember to rebase onto main because I merged #35 recently.

@Gnarus-G Gnarus-G added enhancement New feature or request good first issue Good for newcomers labels Dec 5, 2024
@MCPO-Spartan-117
Copy link
Copy Markdown
Contributor

MCPO-Spartan-117 commented Dec 5, 2024

Since this will be managed by the package manager it would be fine(and should) to use /usr/bin instead of /usr/local/bin for the binaries.

Also DEST_MODULE_LOCATION[0] should be /kernel/drivers/hid/usbhid if you want to follow the path of the original driver.

@MCPO-Spartan-117
Copy link
Copy Markdown
Contributor

MCPO-Spartan-117 commented Dec 5, 2024

The PKGBUILD currently pulls the whole repo again, even tho it's in the repo. This can be fixed quite easily but I don't know whether you want the package to be on the AUR or not.

You can just download the PKGBUILD directly.

The module does not install itself, so you have to run modprobe maccel after install. It is highly discouraged to automate according to the arch wiki.

You should inform in the .install then tell them to do these.

    udevadm control --reload-rules
    udevadm trigger --subsystem-match=usb --subsystem-match=input --subsystem-match=hid --attr-match=bInterfaceClass=03 --attr-match=bInterfaceSubClass=01 --attr-match=bInterfaceProtocol=02

Also move these lines from install to upgrade and looking at said file you misspelled upgrade.

post_uprgade() {
    udevadm control --reload-rules
}

Debug builds are currently broken?

Broken in what way?
Oh, if you mean the install path, you could just do a test check or look for a variable.

if [ -d "$srcdir"/maccel/cli/target/release ]; then
    BUILDTAR="release"
elif [ -d "$srcdir"/maccel/cli/target/debug ]; then
    BUILDTAR="debug"
else
    echo "There's either no build or it's a different build than we are expecting."
    return 1
fi

    install -Dm 755 "$srcdir"/maccel/cli/target/"$BUILDTAR"/maccel "${pkgdir}"/usr/local/bin/maccel
    install -Dm 755 "$srcdir"/maccel/cli/usbmouse/target/"$BUILDTAR"/maccel-driver-binder "${pkgdir}"/usr/local/bin/maccel-driver-binder

Should also almost always quote variables or the entire path.

@MCPO-Spartan-117
Copy link
Copy Markdown
Contributor

MCPO-Spartan-117 commented Dec 5, 2024

Btw is it not possible to reuse the current install/uninstall scripts in the dkms configs or PKGBUILD?

If you mean execute them then,
You are supposed to put stuff in ${pkgdir} and you aren't root in any stage in PKGBUILDs, so lines like these would fail.

	sudo groupadd -f maccel;
	sudo depmod; 
	sudo chown -v :maccel /sys/module/maccel/parameters/* /dev/maccel;
	sudo chmod g+r /dev/maccel;

@MCPO-Spartan-117
Copy link
Copy Markdown
Contributor

The criteria roughly being:

* First letter is capitalized

* Imperative tense (e.g. "Add ...", not "Added ..." or "Adds ...")

* Include as much detail as you can.

Perhaps make a Contribution section in the readme or separate file for this.

@Gnarus-G
Copy link
Copy Markdown
Owner

Gnarus-G commented Dec 6, 2024

BTW @MCPO-Spartan-117 and @pxlsec, you're welcome to join maccel's discord server. I'd be happy to chat with you there should you ever want to.

Comment thread PKGBUILD Outdated
Comment thread maccel.install Outdated
Comment thread driver/dkms.conf
Comment thread install.sh Outdated
Add ensurance message to install script
@MCPO-Spartan-117
Copy link
Copy Markdown
Contributor

Only things i can see this needs now is quoting variable paths and debug build handling for the CLI in the PKG, something like this.

if [ -d "$srcdir"/maccel/cli/target/release ]; then
    BUILDTAR="release"
elif [ -d "$srcdir"/maccel/cli/target/debug ]; then
    BUILDTAR="debug"
else
    echo "There's either no build or it's a different build than we are expecting."
    return 1
fi

    install -Dm 755 "$srcdir"/maccel/cli/target/"$BUILDTAR"/maccel "${pkgdir}"/usr/bin/maccel
    install -Dm 755 "$srcdir"/maccel/cli/usbmouse/target/"$BUILDTAR"/maccel-driver-binder "${pkgdir}"/usr/bin/maccel-driver-binder

Other than that it seems ready to merge.

@pxlsec
Copy link
Copy Markdown
Contributor Author

pxlsec commented Dec 11, 2024

Correct me if im wrong, but this way of handling debug builds seems like a very hacky approach and I'm not sure I like it..

  • I don't think you should be able to build a package in multiple ways, it would destroy reproducability.
  • The build mode specified in the Makefile is release, so you will not be able to install a debug build without editing the PKGBUILD anyways.
  • The debug info will be stripped anyways depending on the users makepkg.conf (which I think is default behavior?)

However if I'm not mistaken, you should be able to build release mode with debug info. This way the PKGBUILD can strip to get a regular release and you could have debug info if enabled in makepkg.conf.

@MCPO-Spartan-117
Copy link
Copy Markdown
Contributor

  1. Then don't change the options or PKG at all?
  2. Alright.
  3. Less effort to deal with than 4
  4. Is that something you can do without appending/removing 20 CFLAGs?

@pxlsec
Copy link
Copy Markdown
Contributor Author

pxlsec commented Dec 11, 2024

Yeah, It's done. I also added some stuff recommended by the arch wiki for rust packages.

I tested building the package with debug and strip settings enabled, meaning that debug symbols will be stripped and put into a separate package ending with -debug, with the modes Release, Debug and Release with Debug.

Release Debug Release w/ Debug
maccel-dkms 1.5 MB 2.5MB 1.5 MB
maccel-dkms-debug 178 KB 7.7MB 8.8 MB

In summary, it seems to be working just fine. Further testing might be necessary however, as debugability might be worse with an optimized build.

Copy link
Copy Markdown
Contributor

@MCPO-Spartan-117 MCPO-Spartan-117 left a comment

Choose a reason for hiding this comment

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

Haven't tested the PKG but i don't see any reason it would fail, DKMS works fine, the debug stuff and module path is up to @Gnarus-G.

@dougg0k dougg0k mentioned this pull request Dec 27, 2024
@Gnarus-G Gnarus-G self-assigned this Jan 26, 2025
Comment thread uninstall.sh Outdated
Comment thread README.md
@Gnarus-G
Copy link
Copy Markdown
Owner

Haven't tested the PKG but i don't see any reason it would fail, DKMS works fine, the debug stuff and module path is up to @Gnarus-G.

What module path are we talking about?

@Gnarus-G
Copy link
Copy Markdown
Owner

@pxlsec What's the whole point of the debug setting and where do you set it?

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

Labels

enhancement New feature or request good first issue Good for newcomers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants