Improve build system#262
Conversation
Signed-off-by: Jakub Klinkovský <1289205+lahwaacz@users.noreply.github.com>
…file Signed-off-by: Jakub Klinkovský <1289205+lahwaacz@users.noreply.github.com>
…ng their own flags For example, Arch Linux wants debug information in the binaries. Signed-off-by: Jakub Klinkovský <1289205+lahwaacz@users.noreply.github.com>
For example, Arch Linux strips debug information after build during the package creation and producing separate debug info files during the build interferes with the Arch tooling. Signed-off-by: Jakub Klinkovský <1289205+lahwaacz@users.noreply.github.com>
…SECCOMP "yes" means linking to system libtirpc, "no" means building libtirpc from source. Signed-off-by: Jakub Klinkovský <1289205+lahwaacz@users.noreply.github.com>
|
@elezar Can you look at this please? 🙏 |
|
@lahwaacz sorry for the delay. I will have a look. |
elezar
left a comment
elezar
left a comment
There was a problem hiding this comment.
Thanks for the changes.
I think they look good. The one thing missing is ensuring that the defaults for all major platforms are the same given the WITH_TIRPC changes.
|
@elezar Any progress with merging this? It's been almost half a year... |
|
@elezar It has been a year, can you merge this please? |
|
@cdesiniotis , please review this. |
|
@henry118 Please review this. |
| $(MAKE) -f $(MAKE_DIR)/elftoolchain.mk DESTDIR=$(DEPS_DIR) install | ||
| endif | ||
| ifeq ($(WITH_TIRPC), yes) | ||
| ifeq ($(WITH_TIRPC), no) |
There was a problem hiding this comment.
This flips the existing behavior.
Currently we will build the library from source when this flag is yes, but after this change it behaves the opposite.
This might break some platforms that package this repo.
There was a problem hiding this comment.
As I wrote before:
Well, the meaning of WITH_TIRPC was not flipped:
- before: "yes" means building libtirpc from source, "no" means don't use libtirpc at all (I think)
(@klueska explained that "Previouslynomeant -- use the nativesunrpclibrary that is bundled in the OS (instead of tirpc).") - after: "yes" means linking to system libtirpc, "no" means building libtirpc from source
Which other approach enabling linking to system libtirpc would you propose?
There was a problem hiding this comment.
imho we should keep the existing semantics.
- yes - build from source
- no - use system library
My concern is that many downstream users might already depend on the current behavior and silently flipping the meaning would cause unexpected breaks.
There was a problem hiding this comment.
WITH_TIRPC=no does not use a system libtirpc - it uses something else for RPC.
AFAICT, there is currently no way to build with system libtirpc without patching or passing custom compiler flags. IMHO this is a good enough reason for a breaking change.
There was a problem hiding this comment.
That's the SunRPC implementation that used to be shipped as part of glibc. Like Ubuntu 18.04 and older, they often still had the glibc SunRPC headers. But modern distros would require libtirpc-dev to compile code using RPC.
To avoid changing the meaning of WITH_TIRPC, perhaps we can introduce a 3 state selector instead:
TIRPC_MODE ?= sunrpc
# sunrpc - use the platform-provided SunRPC implementation
# bundled - build and statically link the bundled libtirpc
# system - use the system libtirpc discovered through pkg-configThen preserve the existing flag as a compatibility shim:
ifdef WITH_TIRPC
ifeq ($(WITH_TIRPC),yes)
TIRPC_MODE := bundled
else
TIRPC_MODE := sunrpc
endif
endifThis keeps existing build scripts working as they do today:
make WITH_TIRPC=yes # still means bundled libtirpc
make WITH_TIRPC=no # still means SunRPCwhile allowing the new behavior explicitly:
make TIRPC_MODE=system7 participants
These changes make building with system flags and libs more straightforward or actually possible. I had to patch the build system a lot while making a package for Arch Linux.