Skip to content

Conversation

@keszybz
Copy link
Member

@keszybz keszybz commented Oct 9, 2020

Fixes #17278.

@poettering
Copy link
Member

is this ifdeffery still worth it? aren't the deps on libshared.so limited enough these days to not require this? at least on my machine libsystemd-shared only has a footprint of 3.4M, so is there really much benefit left?

Really, I'd rather have fewer hacks like that, not more. ifdeffery that leaks into main programs just sucks. Not a fan.

@mbiebl
Copy link
Contributor

mbiebl commented Oct 9, 2020

v246

716K	build-v246/systemd-sysusers.standalone
800K	build-v246/systemd-tmpfiles.standalone

build-v246/systemd-sysusers.standalone
  NEEDED               librt.so.1
  NEEDED               libselinux.so.1
  NEEDED               libpthread.so.0
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2
build-v246/systemd-tmpfiles.standalone
  NEEDED               libacl.so.1
  NEEDED               libcap.so.2
  NEEDED               librt.so.1
  NEEDED               libselinux.so.1
  NEEDED               libpthread.so.0
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2

master

1,3M	build-master/systemd-sysusers.standalone
1,2M	build-master/systemd-tmpfiles.standalone

build-master/systemd-sysusers.standalone
  NEEDED               libblkid.so.1
  NEEDED               libidn2.so.0
  NEEDED               libmount.so.1
  NEEDED               libcrypto.so.1.1
  NEEDED               libp11-kit.so.0
  NEEDED               librt.so.1
  NEEDED               libselinux.so.1
  NEEDED               libm.so.6
  NEEDED               libdl.so.2
  NEEDED               libpthread.so.0
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2
build-master/systemd-tmpfiles.standalone
  NEEDED               libacl.so.1
  NEEDED               libblkid.so.1
  NEEDED               libcap.so.2
  NEEDED               libmount.so.1
  NEEDED               librt.so.1
  NEEDED               libselinux.so.1
  NEEDED               libdl.so.2
  NEEDED               libpthread.so.0
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2

PR17297

952K	build-PR17297/systemd-sysusers.standalone
824K	build-PR17297/systemd-tmpfiles.standalone

build-PR17297/systemd-sysusers.standalone
  NEEDED               libidn2.so.0
  NEEDED               libcrypto.so.1.1
  NEEDED               libp11-kit.so.0
  NEEDED               librt.so.1
  NEEDED               libselinux.so.1
  NEEDED               libm.so.6
  NEEDED               libpthread.so.0
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2
build-PR17297/systemd-tmpfiles.standalone
  NEEDED               libacl.so.1
  NEEDED               libcap.so.2
  NEEDED               librt.so.1
  NEEDED               libselinux.so.1
  NEEDED               libpthread.so.0
  NEEDED               libc.so.6
  NEEDED               ld-linux-x86-64.so.2

Much better, thanks @keszybz
The libblkid and libmount deps are gone, systemd-syusers.standalone still has a libidn2, libcrypto and libp11-kit dep, which is meh :-/

@poettering
Copy link
Member

Much better, thanks @keszybz
The libblkid and libmount deps are gone, systemd-syusers.standalone still has a libidn2, libcrypto and libp11-kit dep, which is meh

libidn2 should be gone with git master, too.

@mbiebl
Copy link
Contributor

mbiebl commented Oct 19, 2020

libidn2 should be gone with git master, too.

Ok, that's good. @keszybz any idea why the libcrypto/p11-kit deps leak into those binaries?

@poettering
Copy link
Member

i definitely have the intention to make the p11-kit dep also a dlopen() one

@poettering
Copy link
Member

hum, could we maybe not merge stuff like this before the discussions are done? there has not been any code review on this one, has there?

i.e. i for one would much prefer if this ifdeffery would not happen in the main programs but in the helper calls. ifdeffery in main programs sprinkled all over the place and just sucks. And it's going to break next time anyone touches this stuff.

@mbiebl
Copy link
Contributor

mbiebl commented Oct 21, 2020

hm, there wasn't any discussion label or so and your first remark is more of the general nature.

@mbiebl
Copy link
Contributor

mbiebl commented Oct 21, 2020

i.e. i for one would much prefer if this ifdeffery would not happen in the main programs but in the helper calls. ifdeffery in main programs sprinkled all over the place and just sucks. And it's going to break next time anyone touches this stuff.

I did have a look at the changes and they looked fine to me. The changes are not too invasive (I don't think "sprinkled all over the place" is quite accurate) and restricted to the affected binaries.
That said, I apologize if this is not what you had in mind. If there is a nicer approach, I'm all for it.

@mbiebl
Copy link
Contributor

mbiebl commented Oct 21, 2020

On a more general note: I appreciate the effort regarding dlopen to reduce dependencies, but this also has a downside (not sure if this also affects RPM based distros): We no longer get any automatic dependencies on the affected libraries. Say one of those libraries bumps the SONAME, the functionality in systemd simply stops working. I'm a bit torn.
@keszybz how do you see this as Fedora maintainer?

@poettering
Copy link
Member

We can certainly add some meta info file into the tree somewhere that is kept in sync (i.e is auto generated) with the precise .so names we use in dlopen() from which libs/binaries. You could use that to automatically synthesize Recommends style deps, and notice so name changes that way.

Would require some plumbing on your side though to parse that meta info file and propagate it into the debian packaging metainfo in some form. but that should not be too hard? we could synthesize files we can %include from RPM spec files from that maybe, and debian something that works for the debian control files? dunno?

(implementationwise I'd probably add a macro to declare such weak so deps in code that ends up in a comment section in the ELF file, from where it could be extracted via readelf. That way the meta-info would be kept up-to-date very easily and would be attached directly to the binary files)

we can't be the first project using dlopen() for such weak deps. It might be worth to track down what is done in other RPMs/DEBs for this case. Maybe there's something used somewhere that is better than manual dep updating

@mbiebl
Copy link
Contributor

mbiebl commented Oct 21, 2020

I'm not aware of any prior art in that regard (at least on the Debian side). We have a mechanism called Built-Using [1] in Debian, but this doesn't really match our requirements here (it's more suitable for static linking). I'm really curious how other distros handle this.

[1] https://www.debian.org/doc/debian-policy/ch-relationships.html#s-built-using

@mbiebl
Copy link
Contributor

mbiebl commented Oct 21, 2020

Fwiw, I completely agree that those (weak) deps should ideally be generated automatically. Otherwise they are bound to be out-of-date.

@poettering
Copy link
Member

I drafted #17416 now, which implements something like this. Would that work for you? @keszybz ?

(maybe continue this specific discussion on #17416)

@keszybz keszybz deleted the tmpfiles-sysusers-disable-standalone-image branch October 22, 2020 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

significant growth in size and dependencies for standalone binaries

4 participants