[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: DEB_BUILD_OPTIONS=nowerror



On Fri, Feb 24, 2023 at 10:58:37AM +0100, Johannes Schauer Marin Rodrigues wrote:
> I question the conflation of a hypothetical DEB_BUILD_OPTIONS=nowerror with
> other linters like shellcheck (or other tools for other programming languages).

I didn't quite reason about that aspect and now see how it is not
obvious.

> 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?

I see -Werror and shellcheck as something fundamentally different to
tests, but maybe that view is not universal.

 * Tests exist to verify that implemented functionality works as
   intended. Tests can fail when the environment changes, but when they
   fail, typically a practical use case also breaks. With -Werror,
   shellcheck, and black, we often have harmless warnings, false positives
   or bugs that are difficult to trigger. So linter violations tend to not
   impact the utility of a software as badly.
 * In a cross build, we cannot run tests, but we can run linters. That
   is a small reason to keep these concepts separate.

Proposed distinction: Tests highlight actual problems by executing the
implementation and linters highlight possible problems by analyzing the
implementation without actually running it.

> 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.

Yeah and for the reasons above, I wouldn't like -Werror to be disabled
by nocheck.

> 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 think this quite nails it. Thank you for raising the term "linter".
And yeah, the common wisdom is to run them on CI, but not during package
builds except for some packages that enable them in package builds as
well.

> > I'd like to propose the option name `nolint`.

This fully addresses the concerns raised by Ansgar I think. I like it.
Thank you.

> 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.

Of course there is value in running them! But there also is a cost.
Quite clearly, we do not have distribution-wide consensus on that
trade-off. And the difference is subtle: Those who think that a default
build shouldn't run linters tend to also think that CI should run
linters. So it's not about disabling linters, but about keeping packages
buildable and not introducing latent build failures.

> 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?

Yeah, that's the question I raised effectively. (+ painting it
correctly)

> Should DEB_BUILD_OPTIONS=nowerror also disable other linters?

I still argue for "yes" and "nolint" makes more sense of that.

> Should other linters like shellcheck be disabled with
> DEB_BUILD_OPTIONS=nocheck?

I argue for "no" (see above).

> Should -Werror be disabled with DEB_BUILD_OPTIONS=nocheck?

I argue for "no" (see above).

I gather that things are not as obvious as I thought and that asking
d-devel was a good thing before sending patches. :)

Helmut


Reply to: