-
Notifications
You must be signed in to change notification settings - Fork 443
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
PDAL 1.7.1 fails to build on various architectures (-Werror=shift-count-overflow, -Werror=cast-align, -Werror=cast-qual) #1915
Comments
Probably true, but we thought we'd try it. |
May I suggest instead to selectively promote certain warnings to errors, the ones you know you don't want in the source tree. Every new GCC release will break the PDAL build due to new warnings it introduces. To deal with this issue I've patched out |
Or we could just back out |
Removing Doing log analysis on the warnings may be an option to get rid of certain classes of issues for example. |
I think you should always build with -Werror. If you don't think the warnings are important to fix, you can suppress them by turning them off at the command-line or individually with pragmas. Most of the time you should fix them. If we're targeting certain compilers/architectures, we should build for them before we release and hopefully test for them. Having some experience with targeting various systems/architectures, I can say that it can be problematic to say "yep, it compiles" and call it good. Of course, it may work just fine, but then again... Maybe we're not in a position to be providing PDAL packages for architectures we have no access to. Are there VMs we can spin up? I should add that people are welcome to build PDAL in any way they choose -- this is open source. But I'm not sure where the line gets drawn WRT our responsibility. |
You can emulate some of the architectures with QEMU, that's about as generally accessible those get. It better than nothing, but QEMU introduces its own bugs, so not something you can rely on 100%.
You should only enable For now I can live with having to patch out |
Well, we're not the Debian project here, and maintaining a build matrix of Travis + AppVeyor is plenty. We've never actually run or tested PDAL on any of those environments you're showing compilation failures in, we've never compiled PDAL on those environments, and we've never advertised that PDAL even works on those environments. It's also not our project's responsibility to ensure PDAL works on platforms we don't have access to develop upon. Caveat emptor.
So you wish us to leave |
No, as I said before:
Regarding other architectures, only amd64 and i386 are important everything else is nice to have. Although since PDAL built there before it should continue to do so, or it will block testing migration and hence inclusion in the next stable release. That's the reason why I have to care about PDAL building on those architectures, although there are unlikely to be any users of PDAL on those architectures. armhf for Android and arm64 for newer ARM hardware may have or get actual users of PDAL, those are the other two (along with amd64 and i386) which you should strive to support upstream. |
Fwiw, on OpenBSD/amd64, trying to build PDAL 1.7.1 with clang 6.0.0,
Locally backporting fb002d5 of course helps. |
On Mac (with PDAL 1.7.1 from Homebrew), I added
to my project's |
In fact, I'm not quite sure why PDAL exports From the CMake docs:
I don't know CMake very well, but probably you should put that list of warnings into |
The complaint about INTERFACE_COMPILE_OPTIONS is separate (I think). Opened #1922 to address this. |
The |
PDAL 1.71. fails to build on various architectures: buildlogs
i386
arm64
mips
Enabling
-Werror
unconditionally is probably not a good idea.The text was updated successfully, but these errors were encountered: