search

lucab / libsystemd-rs

A pure-Rust client library to work with systemd
https://crates.io/crates/libsystemd
Other
106 stars 19 forks source link

logging: fix build in armv7 #143

Closed daniestevez closed 10 months ago

daniestevez commented 1 year ago

In armv7-unknown-linux-gnueabihf (ARM 32-bit), FileStat::inode is an u32 instead of an u64, so we need to convert it to u64.

daniestevez commented 1 year ago

Force-pushed to fix a clippy lint.

swsnr commented 1 year ago

Can't you use inode_t and dev_t in the definition of the journal steam struct? That'd avoid the conversation entirely…

daniestevez commented 1 year ago

Can't you use inode_t and dev_t in the definition of the journal steam struct? That'd avoid the conversation entirely…

inode is also read from std::fs::Metadata::st_ino, which seems to return a u64 even in ARMv7, so this would cause problems.

swsnr commented 1 year ago

I guess you're referring to the from_metadata function of JournalStream? I think we can remove that; I believe it's not used anymore since #142, and I guess I just forgot to get rid of it.

daniestevez commented 1 year ago

I have force-pushed a new version where I use libc::dev_t and libc::ino_t as types for the fields of JournalStream and remove the JournalStream::from_metadata function as suggested.

lucab commented 1 year ago

This needs a rebase before merging. Also, the removed API will require a semver bump, so let's keep this hanging a bit longer before merging so that we can tag a 0.6.1 release before this.

swsnr commented 1 year ago

@lucab Don't we need a semver bump already for the MSRV bump in #142? Shouldn't we just rollout 0.7.0?

daniestevez commented 1 year ago

~I've just rebased and force-pushed.~ ~Hold on a second. I did something wrong.~

Rebase done correctly now.

daniestevez commented 1 year ago

What is the status of this PR? Is it waiting for a semver bump?

wchargin commented 1 year ago

Hi! Just want to +1 this patch. It fixes ARMv6, too; I'm depending on it to cross-build to arm-unknown-linux-gnueabihf for my Pi Zero W.

My Cargo.toml has

[patch.crates-io]
libsystemd = { git = "https://github.com/lucab/libsystemd-rs", rev = "refs/pull/143/merge", version = "0.6.0" }

to override the transitive dep from systemd-journal-logger, but it would be great if this could be properly released so I don't have to depend on that fragile ref.

swsnr commented 11 months ago

@lucab This is hanging around for a few months now, can I go ahead, merge it, do an all over dependencies update and release 0.7.0?

lucab commented 11 months ago

@swsnr yes, thanks!