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

CI broken on new GNU version #949

Open
j9liu opened this issue Sep 26, 2024 · 6 comments
Open

CI broken on new GNU version #949

j9liu opened this issue Sep 26, 2024 · 6 comments
Labels
bug Something isn't working

Comments

@j9liu
Copy link
Contributor

j9liu commented Sep 26, 2024

Looks like CI is broken for ubuntu-latest/gcc/RelWithDebInfo on multiple branches. Have not tried it on main but I wouldn't be surprised if it failed. However, it looks like the Github runner has not changed versions; it's something to do with the build process.

I compared a working job with a failed job on the same branch and noticed this difference:

Successful:

-- The CXX compiler identification is GNU 11.4.0
-- The C compiler identification is GNU 11.4.0

Failed:

-- The CXX compiler identification is GNU 13.2.0
-- The C compiler identification is GNU 13.2.0

This new version results in this failure:

190: -------------------------------------------------------------------------------
190: JsonValue::getSafeNumber() throws if narrowing conversion error would occur
190:   2^64 - 1 cannot be converted back to a double
190: -------------------------------------------------------------------------------
190: /home/runner/work/cesium-native/cesium-native/CesiumGltf/test/TestJsonValue.cpp:44
190: ...............................................................................
190: 
190: /home/runner/work/cesium-native/cesium-native/CesiumGltf/test/TestJsonValue.cpp:46: FAILED:
190:   REQUIRE_THROWS( value.getSafeNumber<double>() )
190: because no exception was thrown where one was expected:
190: 
190: ===============================================================================
190: test cases: 1 | 1 failed
190: assertions: 4 | 3 passed | 1 failed
@j9liu j9liu added the bug Something isn't working label Sep 26, 2024
@j9liu j9liu assigned j9liu and unassigned j9liu Sep 26, 2024
@j9liu j9liu added this to the October 2024 Release milestone Sep 26, 2024
@lilleyse
Copy link
Contributor

I just ran into this issue locally with GCC 13.1.0.

@j9liu
Copy link
Contributor Author

j9liu commented Sep 26, 2024

Turns out the actual culprit is an OS update -- the runner is now using ubuntu-24 instead of ubuntu-22.

I wonder if the narrowing conversion error is related to the test failures we encountered earlier with mac: #867

@lilleyse
Copy link
Contributor

Locally I got the test to pass by changing it to 2^64 - 2. Weird because both 2^64 - 1 and 2^64 - 2 are not representable as double. 🤷

TEST_CASE("JsonValue::getSafeNumber() throws if narrowing conversion error "
"would occur") {
SECTION("2^64 - 1 cannot be converted back to a double") {
auto value = JsonValue(std::numeric_limits<std::uint64_t>::max());
REQUIRE_THROWS(value.getSafeNumber<double>());
}

@j9liu
Copy link
Contributor Author

j9liu commented Sep 26, 2024

Interesting... 🤔 @lilleyse could you see what getSafeNumber is returning when the test doesn't throw? (for 2^64 - 1).

In the past I've gotten weird errors with uint64_t that resulted in some weird undefined behavior in some cases. It almost souns like an overflow is happening under-the-hood that makes the value wrap around to 0 or a smaller number... but since the value is being returned by std::numeric_limits I feel like that shouldn't be right.

@lilleyse
Copy link
Contributor

Even stranger, it passes in debug mode but not release.

getSafeNumber returns 18446744073709551616.0 which ought to have thrown.

@kring kring removed this from the October 2024 Release milestone Oct 1, 2024
@kring
Copy link
Member

kring commented Oct 1, 2024

Moved this out of yesterday's release because we worked around it by temporarily downgrading to Ubuntu 22.04 on CI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants