On Tue, Jan 12, 2021 at 04:50:50AM +0900, Masahiro Yamada wrote: > On Tue, Jan 12, 2021 at 4:34 AM Nathan Chancellor > <natechancellor@xxxxxxxxx> wrote: > > > > On Tue, Jan 12, 2021 at 04:19:01AM +0900, Masahiro Yamada wrote: > > > On Tue, Jan 12, 2021 at 3:06 AM Nathan Chancellor > > > <natechancellor@xxxxxxxxx> wrote: > > > > > > > > After commit da5fb18225b4 ("bpf: Support pre-2.25-binutils objcopy for > > > > vmlinux BTF"), having CONFIG_DEBUG_INFO_BTF enabled but lacking a valid > > > > copy of pahole results in a kernel that will fully compile but fail to > > > > link. The user then has to either install pahole or disable > > > > CONFIG_DEBUG_INFO_BTF and rebuild the kernel but only after their build > > > > has failed, which could have been a significant amount of time depending > > > > on the hardware. > > > > > > > > Avoid a poor user experience and require pahole to be installed with an > > > > appropriate version to select and use CONFIG_DEBUG_INFO_BTF, which is > > > > standard for options that require a specific tools version. > > > > > > > > Suggested-by: Sedat Dilek <sedat.dilek@xxxxxxxxx> > > > > Signed-off-by: Nathan Chancellor <natechancellor@xxxxxxxxx> > > > > > > > > > > > > I am not sure if this is the right direction. > > > > > > > > > I used to believe moving any tool test to the Kconfig > > > was the right thing to do. > > > > > > For example, I tried to move the libelf test to Kconfig, > > > and make STACK_VALIDATION depend on it. > > > > > > https://patchwork.kernel.org/project/linux-kbuild/patch/1531186516-15764-1-git-send-email-yamada.masahiro@xxxxxxxxxxxxx/ > > > > > > It was rejected. > > > > > > > > > In my understanding, it is good to test target toolchains > > > in Kconfig (e.g. cc-option, ld-option, etc). > > > > > > As for host tools, in contrast, it is better to _intentionally_ > > > break the build in order to let users know that something needed is missing. > > > Then, they will install necessary tools or libraries. > > > It is just a one-time setup, in most cases, > > > just running 'apt install' or 'dnf install'. > > > > > > > > > > > > Recently, a similar thing happened to GCC_PLUGINS > > > https://patchwork.kernel.org/project/linux-kbuild/patch/20201203125700.161354-1-masahiroy@xxxxxxxxxx/#23855673 > > > > > > > > > > > > > > > Following this pattern, if a new pahole is not installed, > > > it might be better to break the build instead of hiding > > > the CONFIG option. > > > > > > In my case, it is just a matter of 'apt install pahole'. > > > On some distributions, the bundled pahole is not new enough, > > > and people may end up with building pahole from the source code. > > > > This is fair enough. However, I think that parts of this patch could > > still be salvaged into something that fits this by making it so that if > > pahole is not installed (CONFIG_PAHOLE_VERSION=0) or too old, the build > > errors at the beginning, rather at the end. I am not sure where the best > > place to put that check would be though. > > Me neither. > > > Collecting tool checks to the beginning would be user-friendly. > However, scattering the related code to multiple places is not > nice from the developer point of view. > > How big is it a problem if the build fails > at the very last stage? > > You can install pahole, then resume "make". > > Kbuild skips unneeded building, then you will > be able to come back to the last build stage shortly. There will often be times where I am testing multiple configurations in a row serially and the longer that a build takes to fail, the longer it takes for me to get a "real" result. That is my motivation behind this change. If people are happy with the current state of things, I will just stick with universally disabling CONFIG_DEBUG_INFO_BTF in my test framework. Cheers, Nathan