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.

Sign up for GitHub

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

Jump to bottom

session: implement dbus-broker-session #321

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

session: implement dbus-broker-session #321

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d04881
build: integrate Rust library into code-base
dvdhrm Jul 11, 2023
8aec3bc
session: implement dbus-broker-session
dvdhrm Jul 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
[package]
name = "dbus-broker"
version = "0.1.0"

categories = [
"os",
]
description = "Linux D-Bus Message Broker"
edition = "2021"
homepage = "https://bus1.eu/lib/dbus-broker"
keywords = [
"broker",
"dbus",
"ipc",
"linux",
"message",
]
license = "Apache-2.0"
readme = "README.md"
repository = "https:/bus1/dbus-broker"
rust-version = "1.64"

autobins = false
autoexamples = false
autotests = false
autobenches = false

[dependencies]
clap = { version = "4.3" }
libc = { version = "0.2" }
39 changes: 39 additions & 0 deletions meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ project(
cc = meson.get_compiler('c')
conf = configuration_data()
mod_pkgconfig = import('pkgconfig')
mod_cargo = subproject('meson-cargo-1')

#
# Mandatory Dependencies
Expand Down Expand Up @@ -119,6 +120,44 @@ add_project_arguments('-DSYSTEM_CONSOLE_USERS=' + acc_sysusers, language: 'c')

conf.set('bindir', join_paths(get_option('prefix'), get_option('bindir')))

#
# Cargo Integration
#

cargo = custom_target(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be disabled/skipped at build time? If not, could you please add a meson_options.txt for it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Skip what? And why?

Copy link
Contributor

Choose a reason for hiding this comment

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

The whole dbus-broker-session, to avoid having to patch it out

Copy link
Member Author

Choose a reason for hiding this comment

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

It is part of the same configuration as the launcher. Why would you not want it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because it's Rust and uses Cargo, and that whole ecosystem is just not ready for distributions, I very much do not want to get tangled in maintaining it in Debian

Copy link
Contributor

Choose a reason for hiding this comment

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

Nothing wrong with the PR itself, it's the Cargo ecosystem, which is a mess of unstable APIs, breaking ABIs, version pinning, static linking and vendoring. It's just not fit for purpose for a Linux distribution at this time, and all attempts to shoehorn it in ends up in a gigantic amount of work to create something incredibly fragile that requires constant, massive churn and engineering effort to maintain. It's just not worth it.

There is nothing you can do, really, unless you have a magic wand and can swish it to convince the Rust people that stable APIs/ABI and shared libraries were invented for a reason :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

There is nothing you can do, really, unless you have a magic wand and can swish it to convince the Rust people that stable APIs/ABI and shared libraries were invented for a reason :-)

I get paid full-time to improve Rust and to make it more applicable to application use-cases. I will gladly spend time on trying to improve things. So the more specific you are, the easier it is to figure out what we can do.

Nothing wrong with the PR itself, it's the Cargo ecosystem, which is a mess of unstable APIs, breaking ABIs, version pinning, static linking and vendoring. It's just not fit for purpose for a Linux distribution at this time, and all attempts to shoehorn it in ends up in a gigantic amount of work to create something incredibly fragile that requires constant, massive churn and engineering effort to maintain. It's just not worth it.

I think it is the responsibility of an application developer to audit their dependencies. And I agree, a lot of the ecosystem seems to spend little time rethinking the dependency model. However, I do not necessarily see a structural problem.

For many years, systemd linked all its utility libraries statically. At some point, systemd started shipping its own private libsystemd.so to avoid linking it into each systemd binary. To my knowledge, this is still how things work. How is that different to Cargo? You can certainly achieve the same with Cargo, by linking statically or building your own private shared library.

Distributions seem to be willing to package systemd. So can you explain how using Cargo is different here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Distributions seem to be willing to package systemd. So can you explain how using Cargo is different here?

Because that was not, is not, and will not be a public library used by external projects, so there is no dependency handling hell. It's exclusively private and internal, and split out just to save (a lot of) disk space. No reverse dependency is affected when it's updated, as there aren't any.

Things uploaded to Cargo are public libraries, used independently by many unrelated projects at the same time. So in a distribution you want to ship a single copy, so that there is only one file to update when there is a security vulnerability to fix in a stable release - but this cannot happen in Rust because there's no functional and usable dynamic linking, everything is vendored and statically compiled, so in practice you have N versions of the same library for N projects using it, and they all needs to be updated and rebuilt, and all their reverse dependencies need to be updated and rebuilt, and all their reverse dependencies need to be updated and rebuilt, and...

This is not really feasible at scale of course, and that's why we have shared libraries.

In order to be maintainable in a distribution, maturity and feature level need to at least match the C/C++ ecosystem:

  • Guaranteed stable API and ABI in the standard library, like glibc
  • Shared libraries and dynamic linking as first class citizen and default for all builds, including for the standard library
  • heavily encourage API and ABI stability in public libraries

As things stand, Rust is designed to be as convenient as possible for an individual application developer, at the expense of everybody else, and for large corporations that ship a handful of applications and can afford an army of engineers to handle the constant churn.

But if you have 60.000 applications to ship all together, things just don't scale, and are not intended to, because it's a use case they don't care about - which is fair enough, they have other goals and want to pursue them instead, and they are perfectly entitled to do so.
The problem arises when one tries to shoehorn this model in a Linux distribution, as Rust is just not made for that use case. They are trying in Debian, and the results are as disastrous as expected. Which brings us to dbus-broker - it is really not a end-user app that one installs in a Flatpak or so, and can live just fine as "leaf" in the dependency tree, it's a core system component that needs to be an integral part of the distribution. Hence why I would really appreciate a config knob to disable it - it's of course fine to have optional components.

Copy link
Member Author

Choose a reason for hiding this comment

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

Distributions seem to be willing to package systemd. So can you explain how using Cargo is different here?

Because that was not, is not, and will not be a public library used by external projects, so there is no dependency handling hell. It's exclusively private and internal, and split out just to save (a lot of) disk space. No reverse dependency is affected when it's updated, as there aren't any.

I think you are underestimating the influence of libsystemd. Yes, it is private, but it has found its way into lots of software by simple copy-paste. Many projects would love to make use of it, but they have to copy the pieces they want, since systemd is unwilling to share the code-base. I mean we even modeled some APIs in dbus-broker based on libsystemd.

The Rust ecosystem decided to instead provide infrastructure to share such helpers, even though admitting that stable API was not a requirement.

I very much prefer the Rust ecosystem, where the copy-paste is avoided, and instead a clear dependence is signaled. This at least allows tracking updates and security-fixes, while the C-world just happily leaves copy-pasted code around.

Maybe the copy-paste ecosystem of C is to be blamed, rather than blessing the static-linking of Cargo. But I personally do not believe that it makes much of a difference who we blame.

Things uploaded to Cargo are public libraries, used independently by many unrelated projects at the same time. So in a distribution you want to ship a single copy, so that there is only one file to update when there is a security vulnerability to fix in a stable release - but this cannot happen in Rust because there's no functional and usable dynamic linking, everything is vendored and statically compiled, so in practice you have N versions of the same library for N projects using it, and they all needs to be updated and rebuilt, and all their reverse dependencies need to be updated and rebuilt, and all their reverse dependencies need to be updated and rebuilt, and...

This sounds like something that can easily be automated. Cargo provides all the necessary metadata, but...

This is not really feasible at scale of course, and that's why we have shared libraries.

...I do agree that it can lead to excessive rebuilds and download sizes, as well as expensive dependency calculations. This is inherent to static linking, yes.

In order to be maintainable in a distribution, maturity and feature level need to at least match the C/C++ ecosystem:

* Guaranteed stable API and ABI in the standard library, like glibc

I strongly disagree. The C world never settled on a useful platform API. Pretending glibc provides a fully featured standard library on Linux platforms is something I cannot agree with. If that was true, libsystemd would not need to be the gigantic standard library it is now. The C world survived fine with everyone statically linking their own linked-lists, their own file-system helpers, their own event loops.

Yes, there are attempts to provide stable APIs (eg., glib, qt, ...), but I do not agree that this is a consensus in the C/C++ world.

I really think the Rust ecosystem just did away with the pretend and instead settled on a shared but static standard library.

* Shared libraries and dynamic linking as first class citizen and default for all builds, including for the standard library

* heavily encourage API and ABI stability in public libraries

I think it is the responsibility of a developer to use dependencies that try to retain backwards compatibility. I do not think the Rust ecosystem encourages unstable APIs, and I think the Rust standard library is a perfect example that stable API is certainly attainable in Rust.

Stable ABI does not matter for static linking (at least not for this discussion, I think), so I will ignore it for now. There is an ongoing effort in Rust to support a stable c-rust ABI, btw.

Which brings us to dbus-broker - it is really not a end-user app that one installs in a Flatpak or so, and can live just fine as "leaf" in the dependency tree, it's a core system component that needs to be an integral part of the distribution. Hence why I would really appreciate a config knob to disable it - it's of course fine to have optional components.

Lets get specific: Lets imagine dbus-broker shipping its own Rust crates and using Cargo to build them and publish it on crates.io. These crates will have no dependencies other than the Rust standard library (well, actually core and alloc, not even std). To me this is very similar to what we do with c-util right now, or what systemd does with libsystem.

I do not see how this changes anything from a distribution perspective. Why would the distribution decide to package the Cargo crates separately? It is not like you currently package the c-util dependencies we have separately, do you?

And yeah, if we start to pull in lots and lots of crates from all around the ecosystem, problems will likely pop up. But lets be honest, this would be on us to fix, not on the Rust ecosystem. I think linking all this stuff dynamically would not make it any better. I don't like this habit of pulling in things left and right, but don't blame it on the tools, blame it on the people that do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Distributions seem to be willing to package systemd. So can you explain how using Cargo is different here?

Because that was not, is not, and will not be a public library used by external projects, so there is no dependency handling hell. It's exclusively private and internal, and split out just to save (a lot of) disk space. No reverse dependency is affected when it's updated, as there aren't any.

I think you are underestimating the influence of libsystemd. Yes, it is private, but it has found its way into lots of software by simple copy-paste. Many projects would love to make use of it, but they have to copy the pieces they want, since systemd is unwilling to share the code-base. I mean we even modeled some APIs in dbus-broker based on libsystemd.

The Rust ecosystem decided to instead provide infrastructure to share such helpers, even though admitting that stable API was not a requirement.

I very much prefer the Rust ecosystem, where the copy-paste is avoided, and instead a clear dependence is signaled. This at least allows tracking updates and security-fixes, while the C-world just happily leaves copy-pasted code around.

Maybe the copy-paste ecosystem of C is to be blamed, rather than blessing the static-linking of Cargo. But I personally do not believe that it makes much of a difference who we blame.

This is exactly what I mean when I say that Rust is designed to be as convenient as possible for a single application developer, with no regards whatsoever for anybody else. If somebody wants to copy and paste private code they can do that, it's open source after all, but it's their problem and they get to keep the pieces, and it is a problem, not something to encourage. I very much doubt anybody else is really using internal systemd code outside of the public libsystemd library, I know for sure dbus-broker isn't - otherwise it couldn't be licensed as Apache2 and would be GPL2, for starters.

Things uploaded to Cargo are public libraries, used independently by many unrelated projects at the same time. So in a distribution you want to ship a single copy, so that there is only one file to update when there is a security vulnerability to fix in a stable release - but this cannot happen in Rust because there's no functional and usable dynamic linking, everything is vendored and statically compiled, so in practice you have N versions of the same library for N projects using it, and they all needs to be updated and rebuilt, and all their reverse dependencies need to be updated and rebuilt, and all their reverse dependencies need to be updated and rebuilt, and...

This sounds like something that can easily be automated. Cargo provides all the necessary metadata, but...

This is not really feasible at scale of course, and that's why we have shared libraries.

...I do agree that it can lead to excessive rebuilds and download sizes, as well as expensive dependency calculations. This is inherent to static linking, yes.

It is not automated, and "just automate it" is a fig leaf. It's hugely expensive, in terms of hardware and engineering resources, and all those resources are completely wasted trying to chase after a badly designed ecosystem that reintroduces long-solved problems for no good reasons, instead of being used for something actually useful. It's beyond obvious that the Rust owners do not care one bit about the Linux distributions model, so why should we waste our limited time and resources to accommodate them and fight an impossible, uphill battle? There are so many things to do, and so little time to do them already.

In order to be maintainable in a distribution, maturity and feature level need to at least match the C/C++ ecosystem:

* Guaranteed stable API and ABI in the standard library, like glibc

I strongly disagree. The C world never settled on a useful platform API. Pretending glibc provides a fully featured standard library on Linux platforms is something I cannot agree with. If that was true, libsystemd would not need to be the gigantic standard library it is now. The C world survived fine with everyone statically linking their own linked-lists, their own file-system helpers, their own event loops.

Yes, there are attempts to provide stable APIs (eg., glib, qt, ...), but I do not agree that this is a consensus in the C/C++ world.

I really think the Rust ecosystem just did away with the pretend and instead settled on a shared but static standard library.

One can take a program compiled against glibc, libsystemd or glib from 10 years ago, and run it without issues against today's version of those libraries. That's what having a stable ABI means. Whether they provide enough functionality or not it's completely orthogonal and unrelated - if anybody thinks glibc is missing some APIs, they can just go add them if they wish. They will be subject to the same ABI stability promises.
In Rust, one can't even use the same version twice, as the standard library is hashed in some way.

* Shared libraries and dynamic linking as first class citizen and default for all builds, including for the standard library

* heavily encourage API and ABI stability in public libraries

I think it is the responsibility of a developer to use dependencies that try to retain backwards compatibility. I do not think the Rust ecosystem encourages unstable APIs, and I think the Rust standard library is a perfect example that stable API is certainly attainable in Rust.

Stable ABI does not matter for static linking (at least not for this discussion, I think), so I will ignore it for now. There is an ongoing effort in Rust to support a stable c-rust ABI, btw.

Developers can't do anything if not even the standard library is stable. It would be pointless, so why bother? The ecosystem actively encourage to do the opposite - just vendor, pin and static link. So that's what developers do, it should not be a surprise at all. The responsibility is with those making these overall design decisions, as that's what pushes in a certain direction instead of another.

Which brings us to dbus-broker - it is really not a end-user app that one installs in a Flatpak or so, and can live just fine as "leaf" in the dependency tree, it's a core system component that needs to be an integral part of the distribution. Hence why I would really appreciate a config knob to disable it - it's of course fine to have optional components.

Lets get specific: Lets imagine dbus-broker shipping its own Rust crates and using Cargo to build them and publish it on crates.io. These crates will have no dependencies other than the Rust standard library (well, actually core and alloc, not even std). To me this is very similar to what we do with c-util right now, or what systemd does with libsystem.

I do not see how this changes anything from a distribution perspective. Why would the distribution decide to package the Cargo crates separately? It is not like you currently package the c-util dependencies we have separately, do you?

And yeah, if we start to pull in lots and lots of crates from all around the ecosystem, problems will likely pop up. But lets be honest, this would be on us to fix, not on the Rust ecosystem. I think linking all this stuff dynamically would not make it any better. I don't like this habit of pulling in things left and right, but don't blame it on the tools, blame it on the people that do it.

libsystemd-shared is part of the same source package, it's not separate, so it's the same codebase, the same repository, the same release tarball. If something is part of a different cargo project, then it's a separate project, and needs to be handled separately as such.

And in this PR you are already adding a dependency on an external lirbary, "clap", which itself depends on dozens of other libraries, so it's already out of the question.

If there were zero traces of cargo, and only and exclusively the compiler and its standard library were used, then it could maybe be feasible, even if painful. But as soon as cargo shows up, it's not workable at all, and I will need to either disable it if possible, or patch it out.

'cargo',
kwargs: mod_cargo.get_variable('build_dict') + {
'env': mod_cargo.get_variable('build_env') + {
'DBRK_VERSION': meson.project_version(),
},
'input': meson.project_source_root() / 'Cargo.toml',
'output': [
'libdbus_broker.a',
],
},
)
cargo_libdbus_broker_a = cargo[0]

dep_rlib = declare_dependency(
link_with: cargo_libdbus_broker_a,
version: meson.project_version(),
)

meson.add_dist_script(
mod_cargo.get_variable('dist_cmd') + [
meson.project_source_root() / 'Cargo.toml',
],
)

run_target(
'vendor',
kwargs: mod_cargo.get_variable('vendor_dict') + {
'command': mod_cargo.get_variable('vendor_cmd') + [
meson.project_source_root() / 'Cargo.toml',
],
},
)

#
# Subdirs
#
Expand Down
8 changes: 8 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
//! D-Bus Broker Support Library
//!
//! This rust library provides all the implementation details of the entire
//! dbus-broker code-base. It is compiled into a single archive and then linked
//! into each target, if needed. The linker is expected to strip all unused
//! parts of the library.

pub mod session;
17 changes: 17 additions & 0 deletions src/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,23 @@ if use_launcher
)
endif

#
# target: dbus-broker-session
#

if use_launcher
exe_dbus_broker_session = executable(
'dbus-broker-session',
[
'session/main.c',
],
dependencies: [
dep_rlib,
],
install: true,
)
endif

#
# target: test-*
#
Expand Down
11 changes: 11 additions & 0 deletions src/session/main.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/*
* D-Bus Session Initiator Main Entry
*/

#include <inttypes.h>

extern int32_t dbrk_session_main(int32_t argc, uint8_t **argv);
Copy link

Choose a reason for hiding this comment

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

This isn't correct. int32_t is not guaranteed to be an alias of int, nor is uint8_t necessarily an alias of unsigned char.

  • int is only required to be 16 bits wide, so the return value could be truncated on some weird MCU-like system. (To be fair, shell return values are truncated to unsigned 8 bits anyway, so it wouldn't make any difference.)
  • uint8_t is an optional type and might not be defined at all. (Not a big deal, as it would just fail to compile in the first place.)
  • char is only required to be at least 8 bits wide, not exactly. In theory you could have a system with an 11-bit char or something, in which case a char and uint8_t could have different pointer representations.

I realize these are mostly theoretical concerns that won't matter on any supported platorm. You're also not actually redefining main() itself. Nevertheless I just think it's simply best to do bindings by the book. The increasingly popular use of exact-width types combined with incorrect assumptions about primitive types can turn out to be messy.

Anyway, I'm not a Rust guy myself, but based on a quick google search, I think you should probably use libc::c_int and libc::c_char here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This function refers to:

#[export_name = "dbrk_session_main"]
pub extern "C" fn main(
    argc: i32,
    argv: *const *const u8,
) -> i32 {

I think it matches the Rust function just fine. Please see the Rust-nomicon for details on FFI guarantees of Rust data-types, if unclear.

There is no int, or char, involved, nor any C datatypes that would ask for libc::* types. Am I missing something?


int main(int argc, char **argv) {
return dbrk_session_main(argc, (uint8_t **)argv);
}
Loading
Loading