-
Notifications
You must be signed in to change notification settings - Fork 41
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
address warnings in resource/resource_gen.cpp etc, then revert workaround #300
Comments
Thanks. |
Another warning that is popping up in that code on a system with gcc 4.9.2
|
Gotta! Will fix. |
I might be interested in this. |
This will be great if this can be looked at for July freeze. Adding "bug" label. |
@garlick: I dropped your C++ flag changes and built it on Ubuntu with the following g++ version but I don't reproduce those errors.
The boost graph packages used are:
So v1.62 is a bit newer than your test. Maybe they addressed some of these compilation errors... How did you test this with g++ 5.4.0? |
I tried this with There probably is some bug within boost graph library in 1.53 that has been fixed later. Let me try centor7 image which should still pull in 1.53 |
I built the entire GCC-5.4 stack from the source on my centos7 container to test this out. But with my side-installed GCC
I am getting:
This seems to be a C++ API mismatch between GCC 5.4 and the yaml shared library. |
To sum up, I have spent enough time to look into this and I don't see a good way to observe this (mis-)behavior and it looks like the current code base works okay under all of our CI testing environments and dependencies. I will undo @garlick's Makefile changes in resources/Makefile.am (8a4b016) since we don't get fuller compiler checks with them. In fact, I have found a few instances of use of unused variables after undoing those flags. I will do a PR on that soon. |
PR #697 addresses this. |
I'm trying to compile the latest flux-sched master with spack on Cori using gcc 7.5.0. I get the error included below. Presumably this is a boost error? I am wondering if the removal of
|
That problem is still there... sorry. I have a work around and will post it here. |
Adding 24a9c53 back in should fix the issue. It is an issue with a version of Boost Graph Library installed. So I don't know what to do with it other than relaxing the compiler options. |
@SteVwonder: I think this should still be closed, do you agree? I find reverting those compiler flags allows us to identify compiler-detectable problems (unused variables) and we shouldn't leave it that way just because there are some issues in some versions of BGL that only manifest themselves under a subset of GCC versions. |
Can we use a workaround like this suggestion to avoid throwing warnings/errors on boost headers while still emitting warnings on our code? Maybe @trws knows of a better workaround? |
That's actually what I would have suggested. There are other ways to get to the same place, but it's wholely appropriate for a usually system-provided dependency like boost to be under |
As discussed in #298, g++ 5.4.0 issues some new warnings that must be disabled in order to be non-fatal due to
-Werror
. In order to support this compiler, several-Wno-*
options were added to resources/Makefile.am (8a4b016). However, one that is needed,-Wno-maybe-uninitialized
, could not be added because it is not understood by clang 3.8 in travis, and an unknown warning option is fatal.The workaround (6856990) was to add
-Wno-error
to resources/Makefile.am, but that should only be a temporary solution until the root cause of the warnings is found.Here is the warning output:
The text was updated successfully, but these errors were encountered: