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

Fix build on big-endian platforms #58

Merged
merged 1 commit into from Mar 1, 2019

Conversation

Projects
3 participants
@pkubaj
Copy link
Contributor

pkubaj commented Feb 23, 2019

There's no number variable, change it to value.

Fix build on big-endian platforms
There's no number variable, change it to value.
@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Feb 23, 2019

Codecov Report

Merging #58 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #58   +/-   ##
=======================================
  Coverage   98.05%   98.05%           
=======================================
  Files          79       79           
  Lines        5103     5103           
=======================================
  Hits         5004     5004           
  Misses         99       99
Impacted Files Coverage Δ
src/Corrade/Utility/Endianness.h 100% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c235a0b...7a84146. Read the comment docs.

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Feb 23, 2019

Thank you! Sorry for such a silly mistake, big-endian platforms are hard to come by.

Since you have one, could I ask you a favor? :) Can you build with tests as well (enable BUILD_TESTS in CMake), run them (using ctest --output-on-failure in the build dir) and report what failed? I bet there will be a few more bugs. Thanks a million! 👍

@pkubaj

This comment has been minimized.

Copy link
Contributor Author

pkubaj commented Feb 23, 2019

Sure.

The following tests FAILED:
         13 - ContainersStridedArrayViewTest (Failed)
         43 - UtilityDirectoryTest (Failed)
         47 - UtilitySha1Test (Failed)
         49 - UtilitySystemTest (Failed)
         53 - UtilityResourceTest (Failed)

The platform I use is Talos II from Raptor ( https://www.raptorcs.com/content/TL2B01/intro.html ), but if you want something cheaper, they offer preorders for Blackbird ( https://www.raptorcs.com/content/BK1B01/intro.html ) or you could just get PowerMac G5. Ubiquiti routers can also run big-endian and they are even cheaper (although also much slower).

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Feb 23, 2019

The --output-on-failure flag I mentioned should print also the output of the failing tests, that's kinda essential to knowing what's wrong and what should be fixed ;)

@pkubaj

This comment has been minimized.

Copy link
Contributor Author

pkubaj commented Feb 23, 2019

Here's the complete log:
https://pastebin.com/GkSgVGyd

@mosra mosra added this to the 2019.0b milestone Feb 23, 2019

@mosra mosra added this to TODO in Utility via automation Feb 23, 2019

@mosra mosra self-assigned this Feb 23, 2019

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Feb 23, 2019

Thanks a lot! Some of these are rather trivial, more about the test expectations being wrong, but some (such as the SHA-1 calculation or Resource) look actually serious.

Would you mind if I bother you again later (next week, let's say) to test proposed fixes? :)

@pkubaj

This comment has been minimized.

Copy link
Contributor Author

pkubaj commented Feb 23, 2019

No problem, I can test your fixes.

@mosra mosra merged commit 7a84146 into mosra:master Mar 1, 2019

5 checks passed

LGTM analysis: C/C++ No new or fixed alerts
Details
codecov/patch Coverage not affected when comparing c235a0b...7a84146
Details
codecov/project 98.05% remains the same compared to c235a0b
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Utility automation moved this from TODO to Done Mar 1, 2019

@mosra mosra referenced this pull request Mar 1, 2019

Merged

Fixes for big-endian platforms #60

@mosra

This comment has been minimized.

Copy link
Owner

mosra commented Mar 1, 2019

Merged, thanks a lot! I opened #60 with the proposed fixes for big-endian test failures, would be great if you could have a look :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.