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

Enable access to 'less' as a busybox applet #2189

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

ioctl2
Copy link
Contributor

@ioctl2 ioctl2 commented Oct 16, 2022

This adds less to all current boards/platforms as a BR2 package (BR2_PACKAGE_LESS)

This addition was requested on the community forums for the purpose of having it as a default pager for journalctl.

While BusyBox does offer a stripped down version of less, it did not play nice with journalctl when I compiled it in, and appeared to be broken in general. Perhaps this was an issue with my specific environment, though I tested it in a new VM after deploying a freshly builtova.

The buildroot package is automatically used by journalctl once built (the same should happen with the busybox sourced version, but again, it was not playing nice in my quick test).

*** Only tested the ova build. ***

@agners
Copy link
Member

agners commented Oct 17, 2022

We do suffer a bit from feature creep in HAOS. Also, the full journal will be available from the frontend soon (see home-assistant/supervisor#3291).

That said, missing less is an annoyance to me sometimes too 😅

How big is less?

While BusyBox does offer a stripped down version of less, it did not play nice with journalctl when I compiled it in, and appeared to be broken in general.

In what way did it not play nicely?

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 17, 2022

I did not look into the size of less because it's a core pager that should not be too large.

What I was getting with the BusyBox build of less was really weird. It seemed to only print one line of whatever you threw at it (journalctl or just a file), then a series of blank lines that filled the rest of the screen. Perhaps the defaults cause this, and other symbols needed to be defined. I will take a look at the configs related to BusyBox less.

I will see if I can do three builds (no-less, busybox-less, br2-less) and compare the final image size. This really shouldn't add too much to the image.

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 17, 2022

Side note - I thought that with this commit, buildroot would adjust the symbols for the busybox less applet to facilitate it being used as a default pager for systemd, and also enable sane defaults.

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 18, 2022

This is the output of journalctl when I build dev with the busybox less applet:
image

I think it's just a matter of enabling some additional symbols, perhaps. But this is definitely not what I was expecting for default behaviour.

Without less: 463 MB (485,509,120 bytes)
Busybox less: 463 MB (485,519,360 bytes)
Delta: 10240 bytes

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 18, 2022

These are the symbols that are automatically selected for busybox less during the build (after the manually selected first line):

CONFIG_LESS=y
CONFIG_FEATURE_LESS_MAXLINES=0
# CONFIG_FEATURE_LESS_BRACKETS is not set
CONFIG_FEATURE_LESS_FLAGS=y
CONFIG_FEATURE_LESS_TRUNCATE=y
# CONFIG_FEATURE_LESS_MARKS is not set
# CONFIG_FEATURE_LESS_REGEXP is not set
# CONFIG_FEATURE_LESS_WINCH is not set
# CONFIG_FEATURE_LESS_ASK_TERMINAL is not set
CONFIG_FEATURE_LESS_DASHCMD=y
# CONFIG_FEATURE_LESS_LINENUMS is not set
CONFIG_FEATURE_LESS_RAW=y
CONFIG_FEATURE_LESS_ENV=y

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 18, 2022

Side observation:

Not sure if this is by design, but when I switch back to a branch that does not have the less symbol enabled and run the build again, it still gets included. I had to sudo scripts/enter.sh make clean for it to build without it.

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 18, 2022

The size of the release seems to change when I clean and build fresh.

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 18, 2022

So 3 builds with the source code unchanged, separated by make clean, and these are the sizes of the resulting ova files:
Build command: sudo scripts/enter.sh make clean && sudo scripts/enter.sh make ova
462.8 MiB (485,324,800)
462.9 MiB (485,345,280)
462.8 MiB (485,314,560)

So putting aside the PR at hand, are these builds not supposed to be reproducible? Or did I miss something in my setup?

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 18, 2022

So I instead consulted graphs/package-size-stats.csv.

Package name,Package size,Package size in system (%)
busybox without less: busybox, 2909280, 0.6
busybox with less: busybox, 495312, 0.1
^^ above numbers do not make sense to me.
buildroot less: less,190608,0.0
^^ maybe you can decipher this, because all the numbers above don't seem to add up.
Actually, the size returned seems to change between builds too.
This is busybox back to dev branch and with buildroot less added (though I did not make clean between these builds):
busybox,472872,0.1

busybox less applet binary size as per graphs/file-size-stats.csv
File name,Package name,File size,Package size,File size in package (%),File size in system (%)
usr/bin/less, busybox, 14248, 495312, 2.9, 0.0

buildroot less package's binary size as per graphs/file-size-stats.csv
File name,Package name,File size,Package size,File size in package (%),File size in system (%)
usr/bin/less,less,190608,190608,100.0,0.0

I hope you can make sense of the above, as I am drawing a blank at the inconsistencies and the seemingly arbitrary output.

I'd be interested to see if someone else running a similar build environment is also getting the numbers above or similar inconsistencies.

My build environment is:

$ uname -a
Linux user-virtual-machine 5.15.0-47-generic #51-Ubuntu SMP Thu Aug 11 07:51:15 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
$ free -m
               total        used        free      shared  buff/cache   available
Mem:           11935        7775         476         224        3683        3617
Swap:            979         979           0
$ lsb_release -a
LSB Version:    core-11.1.0ubuntu4-noarch:printing-11.1.0ubuntu4-noarch:security-11.1.0ubuntu4-noarch
Distributor ID: Ubuntu
Description:    Ubuntu 22.04.1 LTS
Release:        22.04
Codename:       jammy
$ docker --version
Docker version 20.10.19, build d85ef84
$ git --version
git version 2.34.1

And I keep the source tree on a separate ext4 partition.

@agners
Copy link
Member

agners commented Oct 18, 2022

Not sure if this is by design, but when I switch back to a branch that does not have the less symbol enabled and run the build again, it still gets included. I had to sudo scripts/enter.sh make clean for it to build without it.

Yeah that is a Buildroot limitation: the output/target directory doesn't get cleaned from already deployed artifacts. Essentially, Buildroot does not keep track what files each package deploys things there, so it can't "undo" these deployments when removing the package.

output/target won't automatically get rebuilt if you blow it away, but you can get Buildroot to do it by deleting the right stamp files, see this StackOverflow thread.

So putting aside the PR at hand, are these builds not supposed to be reproducible? Or did I miss something in my setup?

Define reproducible... A while ago, getting sha256sum accurate builds was next to impossible as timestamps, randomness from order of object files at link time etc caused minor differences in the binaries (these differences are not really functional, just code stored in a different order in the binary etc.) In the last few years the reproducible build effort got big mainly for security reasons. But it needs compiler flags and careful crafted Makefiles to work a 100%.

There is BR2_REPRODUCIBLE which is a Buildroot flag which should create reproducible builds. It is not used in HAOS currently. It comes with caveats today:

This option will remove all sources of non-reproducibility
from the build process. For a given Buildroot configuration,
this allows to generate exactly identical binaries from one
build to the other, including on different machines.
                                                            
The current implementation is restricted to builds with the
same output directory. Many (absolute) paths are recorded in
intermediary files, and it is very likely that some of these
paths leak into the target rootfs. If you build with the
same O=... path, however, the result is identical.
                                                            
This is labeled as an experimental feature, as not all
packages behave properly to ensure reproducibility.

The above mentioned linking order can also affect binary size, afaik that is mostly because optimization passes might end up with different outcome depending on what order the object files got added. Probably highly specific on compiler version etc.

So the answer to your question: No, builds are not 100% reproducible currently. The size difference can also come from compression efficiency differences, but that is just a guess. 5-20KiB seems to be a bit more than I would have expected, but I guess it can add up here and there.

I did work with Yocto's reproducible build support previously. I only got aware of Buildroot's BR2_REPRODUCIBLE now that you brought it up. I think we should give that a go.

@agners
Copy link
Member

agners commented Oct 18, 2022

So I instead consulted graphs/package-size-stats.csv.

It seems to me your very first number is a bit off.

How do you apply changes? Buildroot is not that good in tracking changes to your configuration/updating output. So a previous make invocation can absolutely influence the state of our output directory, and that can cause different results.

Busybox Without less

busybox,472872,0.1

I then do:

rm -rf output/build/busybox/
# change config
make
busybox,495312,0.1

So about 22KiB.

Compiling with BR2_PACKAGE_LESS=y:

less,190608,0.0

So about 186KiB.

Would be nice if we can use Busybox less. Could be pager related (see this comment in the systemd repo).

@jens-maus
Copy link
Contributor

Just my two cents here to the matter: I would also suggest to use busybox' own less implementation even if this might be to some extend not so feature rich. I didn't follow this ticket completely, but my own experience is, that the less implementation of busybox is feature rich enough for normal use cases, especially if it comes to development or debugging purposes. In addition, the underlying operating system behind Home Assistant should be kept as slim as possible and not slowly promoted to a full fledged linux operating system with all its gimmicks and usual tools. Therefore, I would suggest to use the stripped down less implementation of busybox rather than the full fledged less that is commonly shipped with mainstream linux distros.

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 20, 2022

Just my two cents here to the matter: I would also suggest to use busybox' own less implementation even if this might be to some extend not so feature rich. I didn't follow this ticket completely, but my own experience is, that the less implementation of busybox is feature rich enough for normal use cases, especially if it comes to development or debugging purposes. In addition, the underlying operating system behind Home Assistant should be kept as slim as possible and not slowly promoted to a full fledged linux operating system with all its gimmicks and usual tools. Therefore, I would suggest to use the stripped down less implementation of busybox rather than the full fledged less that is commonly shipped with mainstream linux distros.

Thank you for your input. I can confirm that it is in the works. We will be modifying the PR to use the busybox less applet.

@ioctl2 ioctl2 changed the title Add 'less' via buildroot to all current defconfigs Enable access to 'less' as a busybox applet Oct 20, 2022
@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 20, 2022

It appears that the culprit causing busybox less to only output the first line was a config symbol which had been set to a non-default value of '0' (CONFIG_FEATURE_LESS_MAXLINES=0). This happened in an old commit fd686de. The default appears to be CONFIG_FEATURE_LESS_MAXLINES=9999999 which is what I now set it to.

I also explicitly defined the symbols that buildroot automatically enables when systemd is in use, just in case this behaviour changes down the road. There are a couple of other defaults that I enabled based on a brief look at the defaults that some other projects use.

This also enables truncate, which one can override when desired. With this change, journalctl will truncate by default, but if you pipe the output to less it will wrap by default.

I will address your other questions soon, but I think this commit is ready for a another look.

@agners
Copy link
Member

agners commented Oct 20, 2022

I also explicitly defined the symbols that buildroot automatically enables when systemd is in use, just in case this behaviour changes down the road. There are a couple of other defaults that I enabled based on a brief look at the defaults that some other projects use.

Yeah I agree with that, what I actually do usually is to re-add the generated .config, basically do this and commit:

cp output_generic_x86_64/build/busybox-1.35.0/.config buildroot-external/busybox.config

@ioctl2
Copy link
Contributor Author

ioctl2 commented Oct 20, 2022

What I've been doing is diff the .config to see if anything unexpected changed. For kconfig stuff, I will sometimes copy the .config, edit it with make xconfig, and diff. Manually changing symbols can sometimes miss things.

@jens-maus
Copy link
Contributor

@agners

Yeah I agree with that, what I actually do usually is to re-add the generated .config, basically do this and commit:

cp output_generic_x86_64/build/busybox-1.35.0/.config buildroot-external/busybox.config

You could also consider changing the top-level Makefile so that you will be able to directly call make busybox-menuconfig and make busybox-update-defconfig like this is done in my RaspberryMatic project. There I am simply calling

make busybox-update-config

after having used busybox-menuconfig to modify busybox config options. Then the update-config will sync and write back the changed config settings. See here for the top-level Makefile part in the RaspberryMatic project that is responsible for forwarding this and other buildroot targets in the buildroot build environment:

https://github.com/jens-maus/RaspberryMatic/blob/master/Makefile#L130-L140

@agners agners merged commit c68d8bf into home-assistant:dev Oct 20, 2022
@ioctl2 ioctl2 deleted the enable_less branch October 20, 2022 23:43
@agners agners added the os label Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants