ci: change the build to be verbose by default #31619

Previously CI would use a non-verbose build (where exact compilation and linking commands are not visible) and only if it fails, would rerun the build in verbose mode.

Change this to use verbose in the initial build as well for two reasons:

1. It is useful to be able to see the exact compilation and linking commands even for successful builds, to e.g. confirm some particular flags are (not) used as expected.
2. In failed builds, if the failure is during linking, the repeated verbose build may not show a single compilation command because all files are already compiled, so compilation flags will remain hidden.

<!-- *** Please remove the following help text before submitting: *** Pull requests without a rationale and clear improvement may be closed immediately. GUI-related pull requests should be opened against https://github.com/bitcoin-core/gui first. See CONTRIBUTING.md -->

<!-- Please provide clear motivation for your patch and explain how it improves Bitcoin Core user experience or Bitcoin Core developer experience significantly: * Any test improvements or new tests that improve coverage are always welcome. * All other changes should have accompanying unit tests (see `src/test/`) or functional tests (see `test/`). Contributors should note which tests cover modified code. If no tests exist for a region of modified code, new tests should accompany the change. * Bug fixes are most welcome when they come with steps to reproduce or an explanation of the potential issue as well as reasoning for the way the bug was fixed. * Features are welcome, but might be rejected due to design or scope issues. If a feature is based on a lot of dependencies, contributors should first consider building the system outside of Bitcoin Core, if possible. * Refactoring changes are only accepted if they are required for a feature or bug fix or otherwise improve developer experience significantly. For example, most "code style" refactoring changes require a thorough explanation why they are useful, what downsides they have and why they *significantly* improve developer experience or avoid serious programming bugs. Note that code style is often a subjective matter. Unless they are explicitly mentioned to be preferred in the [developer notes](/doc/developer-notes.md), stylistic code changes are usually rejected. -->

<!-- Bitcoin Core has a thorough review process and even the most trivial change needs to pass a lot of eyes and requires non-zero or even substantial time effort to review. There is a huge lack of active reviewers on the project, so patches often sit for a long time. -->

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31619.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

I actually needed this while working on https://github.com/bitcoin/bitcoin/pull/31424

How is this different from just looking at the C++ compiler flags .................... output?

How is this different from just looking at the C++ compiler flags .................... output?

Not weighing in on the approach here, but the flags output is fuzzy at best (see the disclaimer below them). Some flags only apply to certain types of objects (simd for ex), certain libs (leveldb), certain build types (shared/static), etc.

Yes, it is a bit fuzzy, but printing the flags to the CI log doesn't solve the problems listed in the motivation either, because to confirm that a flag is used for all compilations in a successful compilation, one will have to go through all lines. Without a script, I don't see anyone doing that manually. In fact, this change makes the log so bloated that the scrollback will normally truncate it, so it is more likely that the change is going to hide issues instead of surface them.

How is this different from just looking at the C++ compiler flags .................... output?

The exact compilation command contains more information than just the flags. I guess this is why the second build (after a failure) is verbose. There is also this:

NOTE: The summary above may not exactly match the final applied build flags
      if any additional CMAKE_* or environment variables have been modified.
      To see the exact flags applied, build with the --verbose option.

Yes, I understand they are not identical. As pointed out earlier, I don't see how this solves this class of issues: #31619 (comment). It seems more like this is a one-off patch that was used once for debugging. I just don't see how it could have helped in that case (as opposed to using already existing info in logs). It would help to add exact steps to reproduce your use-case, otherwise, this all seems a bit speculative.

I just got this, which looks odd (note the $<$<AND:$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>> inside):

C++ compiler flags .................... -O0 -ftrapv -g3 -std=c++20 -fPIC -fdebug-prefix-map=.../src=. -fmacro-prefix-map=.../src=. -Werror $<$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>:SHELL:-Xcompiler -pthread> $<$<AND:$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>>,$<NOT:$<COMPILE_LANGUAGE:Swift>>>:-pthread> -Wall -Wextra -Wgnu -Wformat -Wformat-security -Wvla -Wshadow-field -Wthread-safety -Wloop-analysis -Wredundant-decls -Wunused-member-function -Wdate-time -Wconditional-uninitialized -Woverloaded-virtual -Wsuggest-override -Wimplicit-fallthrough -Wunreachable-code -Wdocumentation -Wself-assign -Wundef -Wno-unused-parameter -Wstack-protector -fstack-protector-all -fcf-protection=full -fstack-clash-protection

It would help to add exact steps to reproduce or a link, otherwise, this all seems a bit speculative.

An alternative would be to compile just one object verbosely to print the compile flags, which would then also print the link flags verbosely.

However, all of this is hard to judge without exact details.

So NACK. This is an extremely trivial ci fixup that should be trivial to test by providing a commit id (or diff) and the ci FILE_ENV to reproduce. The absence of such does not spark confidence that this is the right fix a given problem, or that a problem exists in the first place.

On top of that, there are other issues:

• This will bloat the log so much that the scollback overflows, so this will break the UX of all common use-cases of the CI log.
• This will also hide the configure flags and summary in the truncated scrollback (and everything that happens before the configure in the CI), so it is more likely to hide issues than surface them.

Generally agree with maflcko.

I just got this, which looks odd (note the $<$<AND:$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>> inside):

See #31482.

I just got this, which looks odd (note the < &lt;AND:$<NOT:$<COMPILE_LANG_AND_ID:CUDA,NVIDIA>> inside):

See #31482.

There is not a single FreeBSD CI task, so I fail to see how a patch to the CI will help with a FreeBSD issue that happens outside the CI.