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.

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

WIP: Native Debian packaging #715

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

Conversation

ottok
Copy link
Contributor

@ottok ottok commented Jan 14, 2024

WORK-IN-PROGRESS: Do not merge

Hi!

I wanted to contribute to btop the native Debian packaging so that the project CI can build .deb packages on every commit and for example publish in a PPA, or just offer .deb package directly in the release artifacts. Additionallyo my goal is to make the absolute state-of-the-art build with all optimization flags, hardening flags, and have the build use the latest version of fmtlib in Debian unstable instead of stale embedded copy from include/fmt. Being state-of-the-art includes that the packaging must pass 100% all tests in Salsa-CI and Lintian.
The code base is small, so being perfect should be a reasonable amount of work.

I've already done most of the work, and I can split parts of this PR into separate submissions (e.g. the man page could be an independent PR), but there are a couple of things I am stuck on:

  • Link with system fmtlib: Instead of the bundled include/fmt I'd like the build to use files from /usr/include/fmt. Can you advice how I should extend the Makefile to use /usr/include/fmt if it exist?

  • Verbose build: I am trying to make the build as verbose as possible so that every single compiler/linker invocation prints out in the build log. However, no matter what I experiment with VERBOSE=true and QUIET=false the resulting log isn't verbose enough for blhc. Attached build log: btop_1.x.y_amd64.build.txt. Am I getting the most verbose mode now or not? Could the Makefile perhaps be extended with some inline comments explaining the purpose of QUIET and VERBOSE? Also I don't understand why there is also a CMakeLists.txt for CMake builds. When is CMake better, why was it created? The inline comment simply states CMake configuration for btop, nothing else.

  • Hardening flags: Related to above I am trying to debug what hardening flags are passed, but there are so many CFLAGS and LDFLAGS being overridden in the Makefile that I lost track. Is the Makefile intentionally written so that it will override most of the preset environment flags?
    Current state of hardening-check shows:

hardening-check --verbose bin/btop 
bin/btop:
 Position Independent Executable: yes
 Stack protected: yes
 Fortify Source functions: yes (some protected functions found)
	unprotected: memset
	unprotected: memcpy
	unprotected: wmemcpy
	unprotected: vsnprintf
	unprotected: memmove
	unprotected: gethostname
	unprotected: wmemset
	unprotected: read
	protected: memcpy
	protected: wmemcpy
	protected: fprintf
	protected: vsnprintf
 Read-only relocations: yes
 Immediate binding: yes
 Stack clash protection: yes
 Control flow integrity: no, not found!
  • Test suite: Does btop have any tests that could run automatically after the build, or be triggered manually with say make test (none such target exists currently)? I can easily but btop in a build array that builds and runs it on 10 architectures, but I would need to have some test command to verify after the build that it actually runs.

@evelikov
Copy link

Greetings, random ideas from a passer-by:

  • Use shebang Makefile explicitly a Makefile

The file seems to be GNUmake, where it's usually called gmake on non-linux platforms. Plus very few makefiles have one, so it might be better to omit it all together

  • Fix misc spelling

Nice fixes - should probably open another PR to land them sooner than later.

  • man page commits

Ditto - very neat stuff. A separate PR might be in order?

Thanks in advance for the man/typos o/

@imwints
Copy link
Contributor

imwints commented Jan 15, 2024

Completely agree with the comment above 👍🏽

Regarding the man pages: there's already an open pull request. It is stale for a while now, maybe we can merge the efforts.
Although I would think about the necessity of a man page altogether. btop is not a library nor does it export any functions to the outside. There is little to no need for documentation like that. 99% of the users will simply call btop without any flags. The few options which are available are easily listed with btop -h and are more or less only needed for debugging or scripts. I guess the man page would be a nice-to-have but also overkill for a project of this size.

Optimization

Additionally my goal is to make the absolute state-of-the-art build with all optimization flags ...

Which optimization flags are you talking about? btop is built with -O2 (or even -O3 with CMake release) and LTO by default.

Hardening

... hardening flags ...

Which flags are you talking about? btop is built with _FORTIFY_SOURCE _GLIBCXX_ASSERTIONS _LIBCPP_ENABLE_ASSERTIONS and -fstack-clash-protection -fcf-protection -fstack-protector
by default. This is more than most other application provide. Compilers default to better hardening options in newer versions anyway.

FMT

... and have the build use the latest version of fmtlib in Debian unstable instead of stale embedded copy from include/fmt.

Link with system fmtlib: Instead of the bundled include/fmt I'd like the build to use files from /usr/include/fmt. Can you advice how I should extend the Makefile to use /usr/include/fmt if it exist?

This was discussed before.
There is no need to use any system library since btop doesn't link against libfmt. It's a header-only build dependency.

Debian

Pipeline

Being state-of-the-art includes that the packaging must pass 100% all tests in Salsa-CI and Lintian.

I don't see a reason why btop would need to conform to any external pipeline or CI.
Again, this will tie this project to a distribution which may prevent moving forward with newer C++ features etc.. Especially with a something as outdated as Debian.

Verbose build

I am trying to make the build as verbose as possible so that every single compiler/linker invocation prints out in the build log. However, no matter what I experiment with VERBOSE=true and QUIET=false the resulting log isn't verbose enough for blhc.

This is all described in the README.

Yes, make VERBOSE=true gets you what you want. If you want some cleaner output you could also use CMake: cmake -B build -G Ninja && ninja -C build -v (-v for verbose output)

This will get you something like this:

ninja: entering directory 'build'
[1/10] /usr/lib/llvm/18/bin/clang++ -DGPU_SUPPORT -D_FILE_OFFSET_BITS=64 -D_GLIBCXX_ASSERTIONS -D_LIBCPP_ENABLE_ASSERTIONS=1 -isystem ~/src/btop.git/main/include -g -std=c++20 -flto=thin -fcolor-diagnostics -Wall -Wextra -Wpedantic -ftree-vectorize -fstack-clash-protection -fstack-protector -fcf-protection -MD -MT CMakeFiles/btop.dir/src/linux/btop_collect.cpp.o -MF CMakeFiles/btop.dir/src/linux/btop_collect.cpp.o.d -o CMakeFiles/btop.dir/src/linux/btop_collect.cpp.o -c ~/src/btop.git/main/src/linux/btop_collect.cpp
[2/10] /usr/lib/llvm/18/bin/clang++ -DGPU_SUPPORT -D_FILE_OFFSET_BITS=64 -D_GLIBCXX_ASSERTIONS -D_LIBCPP_ENABLE_ASSERTIONS=1 -isystem ~/src/btop.git/main/include -g -std=c++20 -flto=thin -fcolor-diagnostics -Wall -Wextra -Wpedantic -ftree-vectorize -fstack-clash-protection -fstack-protector -fcf-protection -MD -MT CMakeFiles/btop.dir/src/btop_tools.cpp.o -MF CMakeFiles/btop.dir/src/btop_tools.cpp.o.d -o CMakeFiles/btop.dir/src/btop_tools.cpp.o -c ~/src/btop.git/main/src/btop_tools.cpp
[3/10] /usr/lib/llvm/18/bin/clang++ -DGPU_SUPPORT -D_FILE_OFFSET_BITS=64 -D_GLIBCXX_ASSERTIONS -D_LIBCPP_ENABLE_ASSERTIONS=1 -isystem ~/src/btop.git/main/include -g -std=c++20 -flto=thin -fcolor-diagnostics -Wall -Wextra -Wpedantic -ftree-vectorize -fstack-clash-protection -fstack-protector -fcf-protection -MD -MT CMakeFiles/btop.dir/src/btop_theme.cpp.o -MF CMakeFiles/btop.dir/src/btop_theme.cpp.o.d -o CMakeFiles/btop.dir/src/btop_theme.cpp.o -c ~/src/btop.git/main/src/btop_theme.cpp
...

If Debian wants a hardened package they can adjust their build flags. This is described in the README

There is no reason for btop to provide more by default. All those come with a performance penalty which is in some cases undesirable and since not all Distributions want that behavior we should keep this minimal and let packagers adjust as needed.

Am I getting the most verbose mode now or not? Could the Makefile perhaps be extended with some inline comments explaining the purpose of QUIET and VERBOSE?

This is all described in the README.

CMake

Also I don't understand why there is also a CMakeLists.txt for CMake builds. When is CMake better, why was it created? The inline comment simply states CMake configuration for btop, nothing else.

There are #28 #324 #330 #362. All requests for CMake as a build system. It is not uncommon for a project to provide multiple ways to build the package, mostly a combination of meson and CMake.

The Makefile currently does a lot what a Makefile generator otherwise would do. This was historically autotools and now modern generators like CMake or meson do this as well. They are meant for configuring a build file and provide faster rebuilds with ninja.

The top level comment in the CMakeLists.txt is just for orientation. It is an alternative way to build btop, mostly because it's more flexible in what is does (generating makefile) and also easy to integrate in packaging.

Hardening flags (v2)

Related to above I am trying to debug what hardening flags are passed, but there are so many CFLAGS and LDFLAGS being overridden in the Makefile that I lost track. Is the Makefile intentionally written so that it will override most of the preset environment flags?

As mentioned above.

No, it's not intended to override the environment. Again, see the README for details.
make info will spit out all compilation flags.

Testing

Test suite: Does btop have any tests that could run automatically after the build, or be triggered manually with say make test (none such target exists currently)? I can easily but btop in a build array that builds and runs it on 10 architectures, but I would need to have some test command to verify after the build that it actually runs.

No, btop currently does not provide tests.


I don't know why you added lowdown in every CI file. The cmake-* builds do not produce artifacts, they are only meant to very the build step.

And there is already a debian package.

Please open a new PR for the spell corrections 👍🏽

This was referenced Jan 18, 2024
@ottok
Copy link
Contributor Author

ottok commented Jan 18, 2024

Thanks for taking the time to write a long answer addressing all the points. Yes, I have read the README, the Makefile, the CMakeLists.txt and skimmed through most of the source code. The reason I still had questions about the how the build is intended to work and what is the role of CMake is that the documentation only describes partially how the system works, and lacks explanations on why things are as they are, thus making it hard to reason about how changes should be designed and prompting questions about the background and purpose of things.

I added lowdown to the CMake dependencies to get CI job Linux CMake / cmake_build_on_linux to pass. I also added it to the musl jobs but they won't pass as the musl Docker images are 2+ years old and run Alpine v3.14 which is too old to include lowdown, which is Alpine starting from v3.15.

Spelling and man page are now split into separate PRs #723 and #724. I will return to this PR soon along with a motivation why having Debian packaging sources at upstream comes only with integration and quality benefits and there are no downsides.

@imwints
Copy link
Contributor

imwints commented Jan 18, 2024

Okay I took that a little too offensive. It's just an alternative to make to build btop. I personally find CMake better tool for the job since it is designed for configuring a Makefile, where in the raw Makefile we jump through a lot of hoops currently and adding new functionality is mostly more involved (IMO).

I added lowdown to the CMake dependencies to get CI job Linux CMake / cmake_build_on_linux to pass.

I don't quite understand? The build passed before? Since the CMake jobs are only to verify that the actual build works and not to generate artifacts with e.g. man pages included this won't be necessary.

Regarding the musl images this needs some work probably but I don't know of any alternative currently. But apk allows you to pull from a newer repository, so you could try to install it with apk add --no-cache --<mirror-flag>=<alpine-v3.15-repo-url> lowdown. But this might blow up CI times if this needs to update a lot of dependencies.

According to the Linux Standard Base (LSB), README files should be
installed in the /usr/share/doc directory. By convention however the
README files should not go there 'bare', but in a subdirectory for each
program.

Install to $(PREFIX)/share/doc/btop/README.md to comply with this.

To ensure the 'themes' is copied as a full directory and not individual
files, create the /usr/share/btop in a separate step as the doc
installation step no longer does it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants