scylladb / seastar

High performance server-side application framework
http://seastar.io
Apache License 2.0
8.39k stars 1.55k forks source link

Fix dpdk support when using custom prefix #2542

Open p12tic opened 4 days ago

p12tic commented 4 days ago

Currently Seastar manages building of DPDK internally and includes the built DPDK library to within Seastar library. Unfortunately when Seastar is installed to a custom prefix, it starts to require an installation of DPDK to that prefix. This does not make sense, as Seastar shouldn't require anything related to DPDK after installation.

This PR addresses the problem. The root cause is that the fact that Seastar uses DPDK leaks to installed CMake targets. First, Seastar is fixed to not search for installed DPDK once Seastar has been installed itself. Second, the dependencies have been adjusted so that the dependency on DPDK library does not transfer to users of Seastar library.

This allows Seastar with DPDK enabled being installed in custom prefix.

p12tic commented 4 days ago

The first commit fixes a problem that may be interesting. Some variables have same names across different cmake files. This is problematic because the Seastar project recommends to not set initial value of cmake variables and rely on the default being empty. At least I got this feedback in some of my older PRs. This assumption does not hold if some other file sets the variable to non-empty value beforehand.

Perhaps it makes sense to invert the policy. Setting variables to empty results in additional code, which is small, stable inefficiency. On the other hand, accidentally being hit by cmake variables being overridden in non-obvious ways may waste hours or days of time or even more, if the failure manifests in non-obvious way in different part of codebase.

p12tic commented 14 hours ago

@tchaikov Thanks for review. I think I've addressed your comments.