seastar icon indicating copy to clipboard operation
seastar copied to clipboard

install-dependencies.sh: add more packages for static linkage

Open tchaikov opened this issue 2 years ago • 19 comments

when statically link against Seastar, its dependencies pull in more dependencies. but install-dependencies.sh cannot fulfill these needs. but in the README.md, we are using an example to statically link against Seastar, so let's add the missing bits to install-dependencies.sh .

tchaikov avatar Dec 11 '23 11:12 tchaikov

Do we encourage static linking?

avikivity avatar Dec 11 '23 11:12 avikivity

Do we encourage static linking?

not explicitly. see https://github.com/scylladb/seastar#using-seastar-from-its-build-directory-without-installation .

and search for static.

tchaikov avatar Dec 11 '23 11:12 tchaikov

(I wonder what we statically build against systemd-devel )

mykaul avatar Dec 11 '23 11:12 mykaul

(I wonder what we statically build against systemd-devel )

please see the commit message of https://github.com/scylladb/seastar/pull/1997/commits/ef878ccf3d763608aa7406ce0c0facb25a5b43c6

tchaikov avatar Dec 11 '23 11:12 tchaikov

will update the commit message and cover letter to be more specific.

Regards Kefu Chai

Le lun. 11 déc. 2023 à 19:54, Avi Kivity @.***> a écrit :

Do we encourage static linking?

— Reply to this email directly, view it on GitHub https://github.com/scylladb/seastar/pull/1997#issuecomment-1849925391, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAONPZQRHCG47OH5D5GDKLYI3YA3AVCNFSM6AAAAABAPUEVVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBZHEZDKMZZGE . You are receiving this because you authored the thread.Message ID: @.***>

tchaikov avatar Dec 11 '23 12:12 tchaikov

(I wonder what we statically build against systemd-devel )

please see the commit message of ef878cc

Still can't figure it out. Unlikely to be https://github.com/scylladb/seastar/blob/72fe971fe8d8484a210ff487b68a7831b4d0e2f9/scripts/perftune.py#L1415

(but I'm nitpicking, feel free to ignore - was more curious about it)

mykaul avatar Dec 11 '23 12:12 mykaul

(I wonder what we statically build against systemd-devel )

please see the commit message of ef878cc

Still can't figure it out. Unlikely to be

https://github.com/scylladb/seastar/blob/72fe971fe8d8484a210ff487b68a7831b4d0e2f9/scripts/perftune.py#L1415

(but I'm nitpicking, feel free to ignore - was more curious about it)

lemme elaborate on it a little bit:

$ pkg-config --libs --cflags --static hwloc
-lhwloc -lm -ludev -lpthread

we use pkg-config to retrieve the compile and link flags. and we link against hwloc for retrieving the hardware locality information of the system. see https://github.com/scylladb/seastar/blob/72fe971fe8d8484a210ff487b68a7831b4d0e2f9/include/seastar/core/resource.hh#L37 .

if we want to link to libseastar.a, we would need to use the output of pkg-config --libs --cflags --static hwloc as the link flags. so hwloc wants to link against libudev.a or libudev.so. these files are typically packaged by development package, libudev.so is a symlink to something like libudev.so.1, as as the so libraries are always versioned. so to ensure the the linker can have access to libudev.so, we have to install it using the package including it.

tchaikov avatar Dec 11 '23 13:12 tchaikov

will update the commit message and cover letter to be more specific. Regards Kefu Chai Le lun. 11 déc. 2023 à 19:54, Avi Kivity @.> a écrit : Do we encourage static linking? — Reply to this email directly, view it on GitHub <#1997 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAONPZQRHCG47OH5D5GDKLYI3YA3AVCNFSM6AAAAABAPUEVVOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBZHEZDKMZZGE . You are receiving this because you authored the thread.Message ID: @.>

v2:

  • replace "we encourage encourage developers to statically link" with "in README.md, ..."

tchaikov avatar Dec 11 '23 13:12 tchaikov

if this change is approved. we might want to redo the image along with the change of #1999

tchaikov avatar Dec 13 '23 03:12 tchaikov

@scylladb/seastar-maint hello maintainers, could you help review this change?

tchaikov avatar May 16 '24 08:05 tchaikov

In https://github.com/scylladb/seastar/pull/2244 @denesb seems to suggest that libudev-devel is needed not just for static linking but to build Seastar. If that's the case we should add it without suggesting it is for static linking.

But regarding static linking, I am not convinced by your rationale: "install-dependencies.sh" should be what is required to build Seastar, including running its official test suite - but not to make completely general use of it in any concievable way. If nothing in the build process does static linking, we don't need packages that are only used for static linking. The user would install those packages on their own if they want to do static linking, just like they would install "vi" if they want to edit some C++ code using that editor.

nyh avatar May 16 '24 08:05 nyh

If nothing in the build process does static linking

could you define "the build process"? our README.md actually implies a build process with following steps:

  1. sudo ./install-dependencies.sh
  2. ./configure.py --mode=release
  3. ninja -C build/release

but without the packages installed by this pull request. the above process fails. as unlike the "debug" build, the "release" build actually builds seastar as static libraries.

tchaikov avatar May 16 '24 09:05 tchaikov

our README.md actually implies a build process with following steps:

1. `sudo ./install-dependencies.sh`

2. `./configure.py --mode=release`

3. `ninja -C build/release`

but without the packages installed by this pull request. the above process fails. as unlike the "debug" build, the "release" build actually builds seastar as static libraries.

So you should have opened with that :-) Yes, of course, ninja -C build/release should work. If that needs static libraries, then so be it (I don't know why, but if it's a fact, we need to accept it...).

But... How does our CI always manage to build the release mode if we didn't have this dependency?

nyh avatar May 16 '24 09:05 nyh

sorry, i should have been less misleading. actually there should have been a step 4. where user uses the built seastar to build his/her application. when building it, the building system in turn consumes the .pc file, which encodes the missing dependency. this is elaborated in the commit message already.

tchaikov avatar May 16 '24 09:05 tchaikov

sorry, i should have been less misleading. actually there should have been a step 4. where user uses the built seastar to build his/her application. when building it, the building system in turn consumes the .pc file, which encodes the missing dependency. this is elaborated in the commit message already.

Ok, so this is different from what you said. It sounds like you're now saying that "ninja -C build/release" does work fully without these packages, but then later, when you try to use it using the ".pc", things will be missing. I am not sure that this is Seastar's concern, any more than Seastar doesn't require you to install "vi" or "gdb" just because as an application developer you are likely to need those too.

Beyond the "waste" of requiring more packages for users that don't need them, what bothers me most about this patch is that I don't understand its story of static libraries at all. Why are all sorts of "-devel" packages needed, when none of those actually installs static libraries (.a). You even mention libudev.so in your discussion of static linking, which doesn't make sense. What am I missing?

nyh avatar May 16 '24 09:05 nyh

sorry, i should have been less misleading. actually there should have been a step 4. where user uses the built seastar to build his/her application. when building it, the building system in turn consumes the .pc file, which encodes the missing dependency. this is elaborated in the commit message already.

Ok, so this is different from what you said. It sounds like you're now saying that "ninja -C build/release" does work fully without these packages, but then later, when you try to use it using the ".pc", things will be missing. I am not sure that this is Seastar's concern, any more than Seastar doesn't require you to install "vi" or "gdb" just because as an application developer you are likely to need those too.

if the consuming of .pc is not seastar's concern, i think i will just close this PR.

Beyond the "waste" of requiring more packages for users that don't need them, what bothers me most about this patch is that I don't understand its story of static libraries at all. Why are all sorts of "-devel" packages needed, when none of those actually installs static libraries (.a).

i explained this in https://github.com/scylladb/seastar/pull/1997/commits/3cd63c867cbe93251eb9c61acaf14ff733647b8a

You even mention libudev.so in your discussion of static linking, which doesn't make sense. What am I missing?

yes, for the sake of completeness and correctness, i mentioned both the shared library and static library. it still makes sense, after re-reading my comment at https://github.com/scylladb/seastar/pull/1997#issuecomment-1850053477 .

tchaikov avatar May 16 '24 09:05 tchaikov

Ok, so this is different from what you said. It sounds like you're now saying that "ninja -C build/release" does work fully without these packages, but then later, when you try to use it using the ".pc", things will be missing. I am not sure that this is Seastar's concern, any more than Seastar doesn't require you to install "vi" or "gdb" just because as an application developer you are likely to need those too.

if the consuming of .pc is not seastar's concern, i think i will just close this PR.

But I think @denesb suggested in his PR that some of these packages are needed not because of static linking.

Beyond the "waste" of requiring more packages for users that don't need them, what bothers me most about this patch is that I don't understand its story of static libraries at all. Why are all sorts of "-devel" packages needed, when none of those actually installs static libraries (.a).

i explained this in 3cd63c8

I still don't understand. You say "if hwloc is statically linked. it would bring into libudev", and then talk about where libudev.so comes from. But where does libudev.a come from? Why don't we need it? I looked now, and I can't find libudev.a anywhere on Fedora - how come this is not a problem, and how does the systemd-devel package (which doesn't contain it!) help static linking?

Can you please give a complete example of a problem that this PR solves, maybe I'll finally understand?

nyh avatar May 16 '24 10:05 nyh

But I think @denesb suggested in his PR that some of these packages are needed not because of static linking.

Turns out -ludev is only needed to link statically against libseastar.a, not for building it.

denesb avatar May 17 '24 07:05 denesb

Ok, so this is different from what you said. It sounds like you're now saying that "ninja -C build/release" does work fully without these packages, but then later, when you try to use it using the ".pc", things will be missing. I am not sure that this is Seastar's concern, any more than Seastar doesn't require you to install "vi" or "gdb" just because as an application developer you are likely to need those too.

if the consuming of .pc is not seastar's concern, i think i will just close this PR.

But I think @denesb suggested in his PR that some of these packages are needed not because of static linking.

Beyond the "waste" of requiring more packages for users that don't need them, what bothers me most about this patch is that I don't understand its story of static libraries at all. Why are all sorts of "-devel" packages needed, when none of those actually installs static libraries (.a).

again, it's explained in the commit message of https://github.com/scylladb/seastar/pull/1997/commits/3cd63c867cbe93251eb9c61acaf14ff733647b8a, i am quoting part of it so that we can continue the discussion in a more efficient way:

Because, in general, distro packacage guidelines encourage maintainers to package shared library if possible, and reuse the shared libraries redistributed by the distro whenever appropriate.

i explained this in 3cd63c8

I still don't understand. You say "if hwloc is statically linked. it would bring into libudev", and then talk about where libudev.so comes from. But where does libudev.a come from? Why don't we need it? I looked now, and I can't find libudev.a anywhere on Fedora - how come this is not a problem, and how does the systemd-devel package (which doesn't contain it!) help static linking?

the reason why i mentioned libudev.a was for the sake of completeness. not because it is provided by the pre-built packages. we don't need libudev.a. what we need is libudev.a or libudev.so. but since libudev.so is packaged, we can and should use it. please read https://github.com/scylladb/seastar/pull/1997#issuecomment-1850053477 for more details.

Can you please give a complete example of a problem that this PR solves, maybe I'll finally understand?

sure. i am basically quoting https://github.com/scylladb/seastar?tab=readme-ov-file#using-an-installed-seastar:

  1. sudo ./install-dependencies.sh

  2. ./configure.py --mode=release --prefix=/usr/local

  3. ninja -C build/release install

  4. g++ my_app.cc $(pkg-config --libs --cflags --static seastar) -o my_app

    developer wants to use the built seastar, via its .pc file. but the .pc file references the output of pkg-config --libs --cflags --static hwloc as seastar's link flags, like

    $ pkg-config --libs --cflags --static hwloc
    -lhwloc -lm -ludev -lpthread
    

    but neither libudev.so nor libudev.a is available in a base system

  5. fortunately, libudev.so is packaged by libudev-dev, so we can install it to fulfill this needs.

tchaikov avatar May 17 '24 10:05 tchaikov