Build all of Mjolnir with -Werror #3845
Conversation
dillona
force-pushed
the
mjolnir-werror
branch
from
0d2ab7d
to
523b5f2
Compare
|
I tested initially with Ubuntu 20.04 on both amd64 and aarch64. I see the failing macOS test now, so I will get that fixed. |
CHANGELOG.md
Outdated
| @@ -3,7 +3,8 @@ | |||
| * **Bug Fix** | |||
| * FIXED: valhalla_run_route was missing config logic.[#3824](https://github.com/valhalla/valhalla/pull/3824) | |||
| * FIXED: Handle hexlifying strings with unsigned chars [#3842](https://github.com/valhalla/valhalla/pull/3842) | |||
| * FIXED: Valhalla does not build on macOS with clang 14.0.0 [#3846](https://github.com/valhalla/valhalla/issues/3846) | |||
| * FIXED: Fix warning as error on macOS with clang 14.0.0 [#3846](https://github.com/valhalla/valhalla/issues/3846) | |||
There was a problem hiding this comment.
I don't think this is a correct change, as the macOS problem was observed on master before I made any warning as error changes
There was a problem hiding this comment.
If I'm not mistaken the issue was that warnings were already treated as error and that on the arm platform the char was unsigned so the compiler warned that the check for > 0 was meaningless and the build stopped because werror was already on. Are you saying this wasn't the cause? I remember others working around this issue before but i admit it was just my assumption as i dont have an arm mac to verify. I found the message about mac builds being broken a bit vague and wanted to put a better description of the problem in the changelog
There was a problem hiding this comment.
I believe you're describing #3842, which I was hitting on ARM Linux (Ubuntu 20.04).
For #3846, I was seeing it on x86 macOS. I believe logging.cc was already using -Werror, and the updated compiler issued a new warning for uses of sprintf, which then broke the build. I believe the only reason this hadn't show up in the x86 CI is that CI isn't using the 14.x compiler yet.
So I suppose the updated changelog text was correct; it was just different warning as error changes than the ones I have been working on.
There was a problem hiding this comment.
Oops, no worries I'll fix it up in a little bit. Apologies for the mix up!
dillona
force-pushed
the
mjolnir-werror
branch
from
b890df9
to
1d9fdc5
Compare
|
Rebased to latest master in order to get CI to run again (seemed to have some transient error earlier) |
No description provided.