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

Disable warning-as-error #286

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Disable warning-as-error #286

wants to merge 1 commit into from

Conversation

KyleGospo
Copy link

@KyleGospo KyleGospo commented Jul 23, 2024

Breaks compilation on newer versions of GCC. Likely better to leave this up to the build system.

Fixes #282

Breaks compilation on newer versions of GCC. Likely better to leave this up to the build system.
@kakra
Copy link
Contributor

kakra commented Jan 20, 2025

It's a good practice if error on warning would be default on for a lot more projects in the world to improve code quality - and if your distro build system does not work for it, either fix it as a maintainer via patch, or explicitly tell your build system to not treat warnings as errors.

Removing that constraint upstream and asking downstream for quality control does not work well.

That said, Gentoo packaging removes this with sed -i 's/ -Werror//' makeflags because Gentoo policy says that the build system should control this.

I'd vote against this patch.

@lilydjwg
Copy link

I'm OK with warning-as-error as default, but against making it hard to disable (last time it took me hours to find a -Werror in a .pc file).

if error on warning would be default on for a lot more projects in the world to improve code quality.

But this will also make a lot of users fail to compile old projects with newer compilers / libraries that are perfectly compatible otherwise.

either fix it as a maintainer via patch

There is an assumption that the project maintainer has access to a latest environment. For dedicated developers this won't be an issue but for smaller personal projects it may take too much effort to set up one.

or explicitly tell your build system to not treat warnings as errors.

This is good as long as it works as expected. E.g. meson has an option for -Werror and disabling it should remove every -Werror instance from the build system.

@kakra
Copy link
Contributor

kakra commented Jan 20, 2025

There is an assumption that the project maintainer has access to a latest environment

Actually I meant the package maintainer to submit a patch upstream.

last time it took me hours to find a -Werror in a .pc file

This is a fair point, and projects should have a simple and obvious way to configure their build flags. I think, bees is not too hard here, although it could be documented.

But I agree that projects should fix their build system if they hide such configuration deeply somewhere in the build system, possibly scattered over a lot of different files, not configurable at a central place.

Tho, none of those aspects change my personal opinion about using stricter build flags.

But this will also make a lot of users fail to compile old projects with newer compilers / libraries that are perfectly compatible otherwise.

I'm not sure if this is a valid assumption. Warnings are there for a good reason, and an updated compiler may only later warn about an issue with undefined behavior that you - as the develop - haven't even been aware of. This does not tell anything about whether the code was perfectly fine before. Handling old unmaintained code is an example of "deal with it, or drop it". Also, exactly in that case, you should probably compile with the older version of the compiler, because the new compiler may introduce new behavior resulting in random new behavior of the legacy code. And exactly that is where warnings come in handy again - and you should really compile with warnings as errors to spot the weak points of old code. There's a great responsibility being a package maintainer for a distribution: You are distributing binary code to users who trust you.

And especially bees handles all your valuable data - it should be really trustworthy.

That being said, we should probably fix those warnings with newer gcc.

Which compiler version is it that fails for you? Mentioning "newer" is a little vague, especially if someone comes across this after a long time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build fails on Fedora 40
3 participants