search

xorbit / LiFePO4wered-Pi

Access library, command line tool and daemon for the LiFePO4wered/Pi module
GNU General Public License v2.0
132 stars 31 forks source link

WIP: Switch from Python build system to Makefile #38

Closed Jookia closed 4 years ago

Jookia commented 4 years ago

This is a quick change I did earlier today to make building easier, especially with cross compiling and distribution packaging.

Bugs: This doesn't run INSTALL.sh to enable the daemon or edit /boot.

Jookia commented 4 years ago

Okay, this is feature complete I believe. The user visible changes are lack of Python dependency and having to run 'make all' instead of './build.py'. Also installing to /usr/local by default which is generally what you do on Unix. If people want to avoid using libsystemd-dev they'd have to add USE_SYSTEMD=0 to the makefile line, I haven't noted this yet.

The developer visible changes are much easier packaging and cross compiling. For a concrete example, buildroot cross-compiles this and installs it with no custom scripts as 'make' and 'make install' do the right thing now even when given a cross-compiler and destination directory for installation.

I do need to fix this though: Aug 14 01:59:38 npi systemd[1]: Configuration file /usr/local/lib/systemd/system/lifepo4wered-daemon.service is marked executable

I know this is a big change and I'm not expecting this to be merged soon or if ever, but input would be good.

xorbit commented 4 years ago

Very nice work, thanks. I do intend to merge this, these have been changes I've wanted but was not able to do myself since I don't know enough about Linux internals to be honest. Most of my work is hardware, deeply embedded, small micros, etc.

One question. I noticed in INSTALL.sh you just do a modprobe instead of writing to the /etc/modules file, is this because you have replaced this with the modules-load.d/lifepo4wered.conf file in the Makefile?

Jookia commented 4 years ago

Oh cool! Yep, that's why. Though modules-load.d is a systemd-ism, so I might need to change it to modprobe or something like that. I'll have to see what non-systemd distros do to handle this without overwriting user files.

If this passes your initial review I'll do a squash and clean up of history in to more logical commits.

On Thu, Aug 13, 2020 at 09:43:44PM -0700, Patrick Van Oosterwijck wrote:

Very nice work, thanks. I do intend to merge this, these have been changes I've wanted but was not able to do myself since I don't know enough about Linux internals to be honest. Most of my work is hardware, deeply embedded, small micros, etc.

One question. I noticed in INSTALL.sh you just do a modprobe instead of writing to the /etc/modules file, is this because you have replaced this with the modules-load.d/lifepo4wered.conf file in the Makefile?

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/xorbit/LiFePO4wered-Pi/pull/38#issuecomment-673880174

xorbit commented 4 years ago

Since the code still supports SysV init as well as systemd, I'd like the loading of the kernel module to also still work in SysV style installs. Maybe keep the append to /etc/modules in that case?

Jookia commented 4 years ago

Yeah having the INSTALL.sh append to /etc/modules might be good if sysvinit is detected.

Though I just did some research and it seems like /etc/modules-load.d is NOT a systemd thing and works on other distros: https://docs.voidlinux.org/config/kernel.html https://wiki.gentoo.org/wiki/Kernel_Modules#Automatic_loading https://man.voidlinux.org/modules-load.8

On Thu, Aug 13, 2020 at 09:54:59PM -0700, Patrick Van Oosterwijck wrote:

Since the code still supports SysV init as well as systemd, I'd like the loading of the kernel module to also still work in SysV style installs. Maybe keep the append to /etc/modules in that case?

xorbit commented 4 years ago

Ok, putting it in /etc/modules-load.d is fine then.

Would you be willing to quickly review the latest in the xorbit:py3-systemd-pkg branch? I added stuff to the Makefile that was still in INSTALL.sh, and since you seem to know what you're doing with Makefiles I'd like a quick check.

I added code to restart the service after installing the files, and I'm using raspi-config to make sure UART and I2C are enabled, instead of messing with the /boot/config.txt myself.

Jookia commented 4 years ago

I like the changes but I believe your changes will break in case case where you're packaging the files for the system and not actually trying to install it- such as for a Linux package or for Buildroot (my use case) where the host isn't necessary going to run the daemon. It's a weird convention, but make 'install' shouldn't do much aside from copying and fixing up files.

Having another target like 'make user-install' or something might be good for this where it depends on the 'install' and 'enable-bus' target but also has separate setup-init target.

If you want I can do a PR to test and fix that up for you.