8 ms·

This is fixed by the suggestion right before it: > * Reproducible builds. The sanity you save will be your own. Pro-tip: wrap your build environment with docke

This is fixed by the suggestion right before it: > * Reproducible builds. The sanity you save will be your own. Pro-tip: wrap your build environment with docker (or your favorite packager) so that your tooling and dependencies become both explicit and reproducable. The sanity you save will be your own. Upgrading compiler versions shouldn't be done out-of-band with your normal PR process.

jchw 3y ago
I agree that this helps, although I still think that in general, the default build should never do -Werror, since people may use other toolchains and it shouldn't surprise-break downstream (I'm pretty sure this is a problem Linux distros struggle with all the time..) If it does it only in your fully reproducible CI, then it should be totally fine, of course.
- eschneider 3y ago
  The scripted, packaged docker with toolchain dependencies and _is_ the build. If someone decides to use a different toolchain, the problems are on them.
  - jchw 3y ago
    Yeah that works if you are not dealing with open source. If you are dealing with open source, though, it really won't save you that much trouble, if anything it will just lead to unnecessarily hostile interactions. You're not really obligated to fix any specific issues that people report, but shrugging and saying "Your problem." is just non-productive and harms valuable downstreams like Linux distributions. Especially when a lot of new failures actually do indicate bugs and portability issues.
    - nicoburns 3y ago
      C++ is super annoying in this way. Many other languages (e.g Rust) only have one compiler and good portability out of the box which completely avoids this problem. And other ecosystems that do have multiple implementation (e.g. JavaScript) seem to have much better compatibility/interop such that it's not typically a problem you have to spend much if any time on in practice.
      - adastra22 3y ago
        I’ve had rust projects with strict clippy rules break when rustc is upgraded.
      - jxramos 3y ago
        I'm curious what sort of CPUs and OSes do those languages run on. C++ runs on all sorts of obscure real time OSes, all the standard mainstream ones as well as on embedded equipment and various CPUs, but a lot of that is possible because of the variety of compilers.
    - aseipp 3y ago
      Supporting every Linux distribution and their small differences isn't free, and Linux distributions shipping things you haven't tested directly is also a way for users to get bitten by bugs or bad interactions, which they will then report to you directly anyway so you're responsible for it. It's complicated. It's happened plenty of times where e.g. I've run into an obscure and bad bug caused by a packaging issue, or a downstream library that wasn't tested -- or there's a developer who has to get involved with a specific distro team to solve bugs their users are reporting directly to them but that they can't reproduce or pinpoint, because the distro is different from their own environment. Sometimes these point out serious issues, but other times it can be a huge squeeze to only get a little juice. For some things the tradeoffs are much less clear, open-source or not e.g. a complex multi-platform GUI application. If you're going to ship a Flatpak to Linux users for example, then the utility of allowing any random build environments is not so clear; users will be running your produced binaries anyway. These are the minority of cases, though. (No, maybe not every user wants a Flatpak, but the developers also need to make decisions that balance many needs, and not everything will be perfect.) Half of the problem, of course, is C and C++'s lack of consistent build environments/build systems/build tooling, but that's a conversation for another day. That said, I generally agree with you that if you want to be a Good Citizen in the general realm of open-source C and C++ code, you should not use -Werror by default, and you should try (to whatever reasonable extent) to allow and support dependencies your users have. And try to support sanitizers, custom CFLAGS/CXXFLAGS, allow PREFIX and DESTDIR installation options, obey the FHS, etc etc. A lot of things have consolidated in the Linux world over the years, so this isn't as bad as it used to be -- and sometimes really does find legitimate issues in your code, or even issues in other projects.
      - jchw 3y ago
        Again, you don't have to fix bugs that are reported, but treating it as invalid to use any compiler versions except for the exact ones that you use is just counterproductive. The "utility" of allowing "any random build environment" is that those random build environments are the ones that exist on your user's computers, and absent a particularly good reason why it shouldn't work (like, your compiler is too old, or literally broken,) for the most part it should, and usually, it's not even that hard to make it work. Adopting practices like defaulting -Werror -Wall on and closing bugs as WONTFIX INVALID because it's not any of the blessed toolchains gains you... not sure. I guess piece of mind from having less open issues and one less flag in your CI? But it is sure to be very annoying to users who have fairly standard setups and are trying to build your software; it's pretty standard behavior to report your build failures upstream, because again, usually it does actually signal something wrong somewhere. Developers are free to do whatever they want when releasing open source code. That doesn't mean that what they are doing is good or makes any sense. There are plenty of perfectly legal things that are utterly stupid to do, like that utterly bizarre spat between Home Assistant and NixOS.
    - saagarjha 3y ago
      It doesn't even work outside of open source. I am running a prerelease toolchain almost all the time on my computer. If the project at work turns on -Werror, I immediately turn it off and store away the change. Of course this means that I send in code fixes for things that don't reproduce on other people's machines yet, but I literally never receive pushback for this.
    - michaelmior 3y ago
      I would say it's still worth having -Werror for some "official" CI build even if it is disabled by default.
  - dotnet00 3y ago
    Open source projects that insist their docker container is the only way to go are going to be an instant reject from me. It's a total copout to just push a docker container and insist that anyone not using it is on their own. Docker is too fraught with issues for that, and as anyone can attest, there are few things more frustrating in computing than having to follow down a chain of chasing issues in things only superficially related to what you actually want to do. The least that can be done is for the project to do its best to not be dependent on specific versions, and explicitly document, in a visible place, the minimum and maximum versions known to work, along with a last changed date.
- galangalalgol 3y ago
  I would do wall wextra and werror. Again mostly for my own sanity. But I'd wait to add werror until they were all fixed so regression testing would continue as the warnings got fixed. Cpp_check and clang tidy would also eventually halt the pipeline. And *san on the tests as compiled in both debug and O3 with a couple compilers.
- pizlonator 3y ago
  I think this depends on a bunch of stuff. - Who are the consumers of the source code, i.e. who will ever check it out and build it? Sometimes, it's just one person. Sometimes, it's a team of engineers. In that case, -W -Werror is fine. - How does a warning being reported make the engineers on the team feel? If the answer is, "Hold my beer for five minutes while I commit a fix", then -W -Werror might be the right call. I've been on projects like that and some of them had nontrivial source code consumers. - How easy is it to hack the build system? Some projects have wonderfully laid out build systems. If that's the case and -W -Werror is the default, then it's not hard to go in there and change the default, if the -Werror creates problems. - Does the project have a facility (in the build system) and policy (as a matter of process) to just simply add -Wno-blah-blah as the immediate fix for any new warning that arises? I've seen that, too. (I'm using -Werror in some parts of a personal project. If you're a solo maintainer of a codebase that can be built that way, then it's worth it - IMO much lower cognitive load to never have non-error warnings. The choice of what to do when the compiler complains is a more straightforward choice.)
  - account42 3y ago
    I don't really see the point of -Werror for projects where I am the only developer because I can just fix warnings before committing but things like errring on unused variables are counterprodutctive when you are just trying something out in your local checkout. In my opinion the only place -Werror makes sense is in CI - and there you can just as well have the CI fail on warnings so you get all of them in the output and not just miss out later ones that were never produced because the build was aborted due to -Werror. CI also allows more nuanced approaches like not allowing new warnings of some types while you still deal with fixing existing ones.
- charcircuit 3y ago
  Changing other dependencies can also cause the build to break. The best thing to do is to use the dependencies the project specifies.
  - jchw 3y ago
    Technically changing literally anything, including the processor microarchitecture that the developer originally tested the code on, could easily cause a real-world breakage. That doesn't mean it should, though. Most libraries not written by Google have some kind of backwards compatibility policy. This is for good reasons. For example, if Debian updates libpng because there's a new RCE, it's ideal if they can update every package to the same new version of libpng all at once. If we go to the extreme of "exact dependencies for every package", then this would actually mean that you have to update every dependent package to a new release that has the new version of libpng, all at the same time, across all supported versions of the distribution. Not to mention, imagine the number of duplicate libraries. Many Linux distros, including Debian, have adopted a policy of only having one version of any given library across the whole repo. As far as I understand, that even includes banning statically linked copies, requiring potentially invasive patching to make sure that downstream packages use the dynamically linked system version. And trust me, if they want to do this, they *will* do this. If they can do it for Chromium, they sure as hell can do it for literally any package. There's a balance, of course. If a distro does invasive patching and it is problematic, I think most people will be reasonable about it and accept that they need to report the issue to their distribution instead. Distros generally do accept bugs for the packages that they manage, and honestly for most packages, by the time a bug gets to you, there is a pretty reasonable chance that it's actually a valid issue, so throwing away the issue simply because it came from someone running an "unofficial" build seems really counterproductive and definitely not in the spirit of open source. Reproducibility is good for many reasons. I do not feel it is a good excuse to just throw away potentially valid bug reports though. It's not that maintainers are under any obligation to actually act on bug reports, or for that matter, even accept them at all in the first place, but if you do accept bugs, I think that "this is broken in new version of Clang" is a very good and useful bug report that likely signals a problem.
    - 3y ago

jchw 3y ago

I agree that this helps, although I still think that in general, the default build should never do -Werror, since people may use other toolchains and it shouldn't surprise-break downstream (I'm pretty sure this is a problem Linux distros struggle with all the time..) If it does it only in your fully reproducible CI, then it should be totally fine, of course.

eschneider 3y ago
The scripted, packaged docker with toolchain dependencies and _is_ the build. If someone decides to use a different toolchain, the problems are on them.
- jchw 3y ago
  Yeah that works if you are not dealing with open source. If you are dealing with open source, though, it really won't save you that much trouble, if anything it will just lead to unnecessarily hostile interactions. You're not really obligated to fix any specific issues that people report, but shrugging and saying "Your problem." is just non-productive and harms valuable downstreams like Linux distributions. Especially when a lot of new failures actually do indicate bugs and portability issues.
  - nicoburns 3y ago
    C++ is super annoying in this way. Many other languages (e.g Rust) only have one compiler and good portability out of the box which completely avoids this problem. And other ecosystems that do have multiple implementation (e.g. JavaScript) seem to have much better compatibility/interop such that it's not typically a problem you have to spend much if any time on in practice.
    - adastra22 3y ago
      I’ve had rust projects with strict clippy rules break when rustc is upgraded.
    - jxramos 3y ago
      I'm curious what sort of CPUs and OSes do those languages run on. C++ runs on all sorts of obscure real time OSes, all the standard mainstream ones as well as on embedded equipment and various CPUs, but a lot of that is possible because of the variety of compilers.
  - aseipp 3y ago
    Supporting every Linux distribution and their small differences isn't free, and Linux distributions shipping things you haven't tested directly is also a way for users to get bitten by bugs or bad interactions, which they will then report to you directly anyway so you're responsible for it. It's complicated. It's happened plenty of times where e.g. I've run into an obscure and bad bug caused by a packaging issue, or a downstream library that wasn't tested -- or there's a developer who has to get involved with a specific distro team to solve bugs their users are reporting directly to them but that they can't reproduce or pinpoint, because the distro is different from their own environment. Sometimes these point out serious issues, but other times it can be a huge squeeze to only get a little juice. For some things the tradeoffs are much less clear, open-source or not e.g. a complex multi-platform GUI application. If you're going to ship a Flatpak to Linux users for example, then the utility of allowing any random build environments is not so clear; users will be running your produced binaries anyway. These are the minority of cases, though. (No, maybe not every user wants a Flatpak, but the developers also need to make decisions that balance many needs, and not everything will be perfect.) Half of the problem, of course, is C and C++'s lack of consistent build environments/build systems/build tooling, but that's a conversation for another day. That said, I generally agree with you that if you want to be a Good Citizen in the general realm of open-source C and C++ code, you should not use -Werror by default, and you should try (to whatever reasonable extent) to allow and support dependencies your users have. And try to support sanitizers, custom CFLAGS/CXXFLAGS, allow PREFIX and DESTDIR installation options, obey the FHS, etc etc. A lot of things have consolidated in the Linux world over the years, so this isn't as bad as it used to be -- and sometimes really does find legitimate issues in your code, or even issues in other projects.
    - jchw 3y ago
      Again, you don't have to fix bugs that are reported, but treating it as invalid to use any compiler versions except for the exact ones that you use is just counterproductive. The "utility" of allowing "any random build environment" is that those random build environments are the ones that exist on your user's computers, and absent a particularly good reason why it shouldn't work (like, your compiler is too old, or literally broken,) for the most part it should, and usually, it's not even that hard to make it work. Adopting practices like defaulting -Werror -Wall on and closing bugs as WONTFIX INVALID because it's not any of the blessed toolchains gains you... not sure. I guess piece of mind from having less open issues and one less flag in your CI? But it is sure to be very annoying to users who have fairly standard setups and are trying to build your software; it's pretty standard behavior to report your build failures upstream, because again, usually it does actually signal something wrong somewhere. Developers are free to do whatever they want when releasing open source code. That doesn't mean that what they are doing is good or makes any sense. There are plenty of perfectly legal things that are utterly stupid to do, like that utterly bizarre spat between Home Assistant and NixOS.
  - saagarjha 3y ago
    It doesn't even work outside of open source. I am running a prerelease toolchain almost all the time on my computer. If the project at work turns on -Werror, I immediately turn it off and store away the change. Of course this means that I send in code fixes for things that don't reproduce on other people's machines yet, but I literally never receive pushback for this.
  - michaelmior 3y ago
    I would say it's still worth having -Werror for some "official" CI build even if it is disabled by default.
- dotnet00 3y ago
  Open source projects that insist their docker container is the only way to go are going to be an instant reject from me. It's a total copout to just push a docker container and insist that anyone not using it is on their own. Docker is too fraught with issues for that, and as anyone can attest, there are few things more frustrating in computing than having to follow down a chain of chasing issues in things only superficially related to what you actually want to do. The least that can be done is for the project to do its best to not be dependent on specific versions, and explicitly document, in a visible place, the minimum and maximum versions known to work, along with a last changed date.
galangalalgol 3y ago
I would do wall wextra and werror. Again mostly for my own sanity. But I'd wait to add werror until they were all fixed so regression testing would continue as the warnings got fixed. Cpp_check and clang tidy would also eventually halt the pipeline. And *san on the tests as compiled in both debug and O3 with a couple compilers.
pizlonator 3y ago
I think this depends on a bunch of stuff. - Who are the consumers of the source code, i.e. who will ever check it out and build it? Sometimes, it's just one person. Sometimes, it's a team of engineers. In that case, -W -Werror is fine. - How does a warning being reported make the engineers on the team feel? If the answer is, "Hold my beer for five minutes while I commit a fix", then -W -Werror might be the right call. I've been on projects like that and some of them had nontrivial source code consumers. - How easy is it to hack the build system? Some projects have wonderfully laid out build systems. If that's the case and -W -Werror is the default, then it's not hard to go in there and change the default, if the -Werror creates problems. - Does the project have a facility (in the build system) and policy (as a matter of process) to just simply add -Wno-blah-blah as the immediate fix for any new warning that arises? I've seen that, too. (I'm using -Werror in some parts of a personal project. If you're a solo maintainer of a codebase that can be built that way, then it's worth it - IMO much lower cognitive load to never have non-error warnings. The choice of what to do when the compiler complains is a more straightforward choice.)
- account42 3y ago
  I don't really see the point of -Werror for projects where I am the only developer because I can just fix warnings before committing but things like errring on unused variables are counterprodutctive when you are just trying something out in your local checkout. In my opinion the only place -Werror makes sense is in CI - and there you can just as well have the CI fail on warnings so you get all of them in the output and not just miss out later ones that were never produced because the build was aborted due to -Werror. CI also allows more nuanced approaches like not allowing new warnings of some types while you still deal with fixing existing ones.
charcircuit 3y ago
Changing other dependencies can also cause the build to break. The best thing to do is to use the dependencies the project specifies.
- jchw 3y ago
  Technically changing literally anything, including the processor microarchitecture that the developer originally tested the code on, could easily cause a real-world breakage. That doesn't mean it should, though. Most libraries not written by Google have some kind of backwards compatibility policy. This is for good reasons. For example, if Debian updates libpng because there's a new RCE, it's ideal if they can update every package to the same new version of libpng all at once. If we go to the extreme of "exact dependencies for every package", then this would actually mean that you have to update every dependent package to a new release that has the new version of libpng, all at the same time, across all supported versions of the distribution. Not to mention, imagine the number of duplicate libraries. Many Linux distros, including Debian, have adopted a policy of only having one version of any given library across the whole repo. As far as I understand, that even includes banning statically linked copies, requiring potentially invasive patching to make sure that downstream packages use the dynamically linked system version. And trust me, if they want to do this, they *will* do this. If they can do it for Chromium, they sure as hell can do it for literally any package. There's a balance, of course. If a distro does invasive patching and it is problematic, I think most people will be reasonable about it and accept that they need to report the issue to their distribution instead. Distros generally do accept bugs for the packages that they manage, and honestly for most packages, by the time a bug gets to you, there is a pretty reasonable chance that it's actually a valid issue, so throwing away the issue simply because it came from someone running an "unofficial" build seems really counterproductive and definitely not in the spirit of open source. Reproducibility is good for many reasons. I do not feel it is a good excuse to just throw away potentially valid bug reports though. It's not that maintainers are under any obligation to actually act on bug reports, or for that matter, even accept them at all in the first place, but if you do accept bugs, I think that "this is broken in new version of Clang" is a very good and useful bug report that likely signals a problem.
  - 3y ago