As such, I propose a generic DEB_BUILD_OPTIONS=nowerror modelled after
the original observation, but meant to also match other checkers such as shellcheck. The general idea should be that a warning should that can be non-fatal should be non-fatal if possible.
From a process point of view, I think this mail serves as a possible standardization point. Packages can add support for this and theresponsibility for providing patches for this generally resides with
As such, I propose a generic DEB_BUILD_OPTIONS=nowerror modelled after the original observation, but meant to also match other checkers such as shellcheck. The general idea should be that a warning should that can be non-fatal should be non-fatal if possible.
The name is too specific and can be misread:
- It is specific to -Werror, but other similar systems exist.
- It can be easily read an "now error" (i.e., a warning
should "now (be an) error").
Also I think it was recommended to *not* use -Werror by default as it
is too fragile. Maybe one should have a "developer mode" flag instead
that allows using -Werror?
Hi,
I observe a pattern repeated at least twice and probably more often in packages.
* A package adds -Werror to the build. When a new toolchain version is
uploaded, it triggers a new warning and that makes the package FTBFS.
* A package runs shellcheck during build. When a new shellcheck is
uploaded, it triggers a new warning and makes the package FTBFS.
shellcheck:
* grml-debootstrap
* josm-installer
* kdump-tools
* python-sshoot
* sshcommand
* uwsgi
On Fri, Feb 24, 2023 at 2:26 PM Helmut Grohne <helmut@subdivi.de> wrote:
Hi,
I observe a pattern repeated at least twice and probably more often in packages.
* A package adds -Werror to the build. When a new toolchain version is
uploaded, it triggers a new warning and that makes the package FTBFS.
* A package runs shellcheck during build. When a new shellcheck is
uploaded, it triggers a new warning and makes the package FTBFS.
â–ˆ
IMO, these are just linters. And shouldn't not run after the source is released.
â–ˆ
I'd like to propose the option name `nolint`.
I question the conflation of a hypothetical DEB_BUILD_OPTIONS=nowerror with other linters like shellcheck (or other tools for other programming languages).
I question the conflation of a hypothetical DEB_BUILD_OPTIONS=nowerror with other linters like shellcheck (or other tools for other programming languages).
I'm surprised you are conflating shellcheck with the question about -Werror. I
don't yet understand why you would like to skip shellcheck by setting DEB_BUILD_OPTIONS=nowerror and not by setting DEB_BUILD_OPTIONS=nocheck?
Yes, -Werror is also a kind of check but whether -Werror should be not be passed when DEB_BUILD_OPTIONS=nocheck is given, is a slightly different question I think.
Quoting Shengjing Zhu (2023-02-24 09:14:46)
IMO, these are just linters. And shouldn't not run after the source is released.
I'd like to propose the option name `nolint`.
I think that there is value to run linters at build time in a default build because (as shown by Helmut's initial mail) new version of linters may show different problems compared to the old version and those only get fixed if the
build breaks. So I think it's useful to have a package FTBFS if a new version of a linter introduces a failure.
But of course there is also a use-case of disabling linters if one is not doing
a default build. So I think the more interesting questions are:
Should there be a new DEB_BUILD_OPTIONS=nowerror that disables -Werror?
Should DEB_BUILD_OPTIONS=nowerror also disable other linters?
Should other linters like shellcheck be disabled with DEB_BUILD_OPTIONS=nocheck?
Should -Werror be disabled with DEB_BUILD_OPTIONS=nocheck?
On Fri, Feb 24, 2023 at 10:58:37AM +0100, Johannes Schauer Marin Rodrigues wrote:
Should other linters like shellcheck be disabled with DEB_BUILD_OPTIONS=nocheck?
I argue for "no" (see above).
It was also my understanding that we recommend not using -Werror duringAlso I think it was recommended to *not* use -Werror by default as it
is too fragile. Maybe one should have a "developer mode" flag instead
that allows using -Werror?
Well, if we were avoiding -Werror by default, we wouldn't have this discussion. It certainly isn't consensus.
In a typical build system like Autotools, CMake or Meson, it's going to
be much, much easier for the answer to be yes, because the obvious way
to make linters easy to run is to implement them as a (slightly
specialized) test.
The way I've generally set up lint checks in my recent projects is to
make them a test that usually always passes (with non-fatal warnings
when a problem is detected, like "not ok # TODO" in TAP syntax), and
then have a non-default way to turn those warnings into a test failure,
which I use in upstream CI (but usually not in Debian packaging).
Simon McVittie <smcv@debian.org> writes:
In a typical build system like Autotools, CMake or Meson, it's going to
be much, much easier for the answer to be yes, because the obvious way
to make linters easy to run is to implement them as a (slightly specialized) test.
I agree for separate linters, but I'm not sure this is true for -Werror.
...
* A package adds -Werror to the build. When a new toolchain version is
uploaded, it triggers a new warning and that makes the package FTBFS.
...
When building affected packages with more recent toolchains, such build failures are annoying. In a bootstrap setting, they hide later problems.
For that reason, I would like to have a standard way to opt out of such failures. I understand that some of the warnings may be pointing at real
bugs and that ignoring them certainly is a compromise. I also understand
that packages may fail to build for other reasons with new toolchains
(e.g. missing #includes). However, -Werror has proven to be quite
repetitive and seems worthwhile to address to me.
As such, I propose a generic DEB_BUILD_OPTIONS=nowerror modelled after
the original observation,
...
So let me know if you think this is a bad idea.
...
Examples:
* glibc adds -Werror
...
Helmut
What you describe is an RC bug as soon as the more recent toolchain
becomes default, and the correct solution is to not build with -Werror.
DEB_BUILD_OPTIONS=nowerror would imply that building with -Werror
by default would be OK, but there are already too many FTBFS due
to -Werror.
DEB_BUILD_OPTIONS=werror as standard name for an opt-in option for CI
builds would be a better solution.
...
Examples:
* glibc adds -Werror
...
glibc does not use the default gcc, which avoids most of the problems
you are worried about (but is not a general solution).
On Sun, Feb 26, 2023 at 07:15:45PM +0200, Adrian Bunk wrote:
What you describe is an RC bug as soon as the more recent toolchain
becomes default, and the correct solution is to not build with -Werror.
DEB_BUILD_OPTIONS=nowerror would imply that building with -Werror
by default would be OK, but there are already too many FTBFS due
to -Werror.
I would happily agree with all of this, but I do not see consensus on
either.
...
The problem here specifically arises, because we do not have consensus
on -Werror being a bad idea in release builds.
...
Helmut
Helmut> The problem here specifically arises, because we do not have
Helmut> consensus on -Werror being a bad idea in release builds.
I agree with your reading of consensus and for that reason support registering an option to say do not use -Werror.
Steve> If this is for people doing out-of-archive builds who don't"Steve" == Steve Langasek <vorlon@debian.org> writes:
Steve> If this is for people doing out-of-archive builds who don't"Steve" == Steve Langasek <vorlon@debian.org> writes:
Steve> want to deal with failures from -Werror, I can see how having
Steve> a single environment toggle is useful to them; but it doesn't
Steve> seem useful to *Debian* when such out-of-archive rebuilds
Steve> don't correspond to the official builds. E.g. if they're
Steve> test builds for new toolchains, knowing that the package
Steve> *could* build, with options Debian doesn't actually use, is
Steve> of limited use. (If you build twice, once with once without
Steve> and file bugs on packages where the results differ, I guess
Steve> that's one use, but involves a lot of manual labor.)
In the general case I agree.
But we have the specific case of cross-bootstrapping.
They want to be able to do builds to bootstrap and they want to have an option they can pass into the build to ask debian/rules not to include -Werror.
I suspect Helmut believes he can get patches accepted in the packages
where this is most important to honor the option.
Given his track record, I bet he's right.
So, we have a team in Debian wanting a interface sufficiently
standardized for them to do their work.
I think we have confidence that once we agree on a interface they can
produce appropriate consumers of the interface as well as
implementations of the interface.
I think we should have a high bar for standing in the way of this kind
of work.
I can see that for bootstrapping a new architecture, it will sometimes
be
useful to use a toolchain newer than the one that is currently default
in
Debian, and as a result it is useful to also be able to bypass new
stricter
-Werror behavior from gcc upstream that breaks compatibility with
existing
code.
It isn't clear to me from the discussion history that this is the
actual use
case at issue. But supposing it is, that's one use case; and it's a
use
case that can be addressed without having to make any per-package
changes
and without having to make any changes to dpkg-buildflags interfaces by exporting
DEB_CFLAGS_APPEND=-Wno-error
DEB_CXXFLAGS_APPEND=-Wno-error
as part of the bootstrap build environment, for all packages.
Of course, dpkg-buildflags also exports flags for other languages than
C and
C++ (and should do), so if you want to have full package coverage you
would
want your set of _APPEND variables to match the set of per-language
flags
that dpkg-buildflags already handles. Having to export 7 variables
instead
of 1 is annoying. But it also doesn't require reaching consensus on a
new
interface in dpkg. And I remain unconvinced that the particular
proposed
interface is the right way around for Debian at large.
It isn't clear to me from the discussion history that this is the actual use case at issue. But supposing it is, that's one use case; and it's a use case that can be addressed without having to make any per-package changes and without having to make any changes to dpkg-buildflags interfaces by exporting
DEB_CFLAGS_APPEND=-Wno-error
DEB_CXXFLAGS_APPEND=-Wno-error
as part of the bootstrap build environment, for all packages.
Now, if all packages would just use the flags from dpkg-buildflags as is,
or as the final part of CFLAGS/CXXFLAGS, that would be nice. However, IME, that is not always the case and some maintainers append -Werror in particular.
Following this discussion, I fear we might not reach consensus. But my ideal (strong) suggestion to package maintainers would be:
1) Don't use -Werror (or equivalent for your packages language) during
normal
builds (i.e. on buildd)
2) Do use -Werror via some mechanism (DEB_CFLAGS_APPEND)? during CI builds
3) Actually utilize CI builds to detect any breakages early.
This is conceptually interesting to me. In practice, I don't see us using this in Ubuntu. We have per-architecture differences from Debian (ppc64el building with -O3 by default; riscv64 being a release architecture where it isn't in Debian) that make it interesting to pick up on per-architecture build failures caught by -Werror and not without. But it's not practical to do CI -Werror builds; when we do out-of-archive rebuilds for all architectures, it's a significant committment of resources and each rebuild takes about a month to complete (on the slowest archs). And to be able to effectively analyze build results to identify Werror-related failures with high signal would require two parallel builds, one with and without the
flag, built against the same baseline.
On 28.02.23 20:34, Steve Langasek wrote:
But it's not practical to do CI -Werror builds; when we do
out-of-archive rebuilds for all architectures, it's a significant committment of resources and each rebuild takes about a month to
complete (on the slowest archs). And to be able to effectively
analyze build results to identify Werror-related failures with high
signal would require two parallel builds, one with and without the
flag, built against the same baseline.
That you are so resource constrained here surprises me a little. I can see that for Debian, but I'm surprised that Ubuntu is affected as well. Especially as you'd think that this could also be done within virtualization - the evaluation here is mostly around running the compiler and checking its errors, not so much about running tests accurately on real hardware.
Well, I'm not seeking to stand in the way of the work, so much as trying to make sure this is the most useful work to be doing to meet the actual use cases.
I can see that for bootstrapping a new architecture, it will sometimes be useful to use a toolchain newer than the one that is currently default in Debian, and as a result it is useful to also be able to bypass new stricter -Werror behavior from gcc upstream that breaks compatibility with existing code.
It isn't clear to me from the discussion history that this is the actual use case at issue. But supposing it is, that's one use case; and it's a use
case that can be addressed without having to make any per-package changes
and without having to make any changes to dpkg-buildflags interfaces by exporting
DEB_CFLAGS_APPEND=-Wno-error
DEB_CXXFLAGS_APPEND=-Wno-error
as part of the bootstrap build environment, for all packages.
| Sysop: | Keyop |
|---|---|
| Location: | Huddersfield, West Yorkshire, UK |
| Users: | 339 |
| Nodes: | 16 (2 / 14) |
| Uptime: | 00:19:12 |
| Calls: | 7,466 |
| Calls today: | 2 |
| Files: | 12,691 |
| Messages: | 5,627,864 |