-
Notifications
You must be signed in to change notification settings - Fork 90
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 simplifications #140
Fix simplifications #140
Conversation
on macOS:
|
on Windows 10
|
b3904ad
to
ce79181
Compare
Fixed the windows issues and rebased. |
Hm, found this similar problem from a FreeBSD user: https://bugs.llvm.org/show_bug.cgi?id=26156 |
Looks good to me so far. Waiting for @jmjatlanta's input. |
On macOS:
Results from Windows: |
More fixes here. Still unsure how to handle the __int128 problem on Mac. I'd like to preserve it generally for the gitian-osx build. How to distinguish between gitian and native? |
For Windows, I cloned pmconrad/bitshares-core and checked out 1584_simplification. I then changed the origin of libraries/fc to pmconrad/fc, and checked out 1584_more_simplifications. The results are here: https://gist.github.com/jmjatlanta/1aeeabe9ca71e6923daeb1ee87aea68a |
May be unrelated, but it says __int128 and mac: EOSIO/eos#250 |
Thanks. It points at the root of the problem. Will try a workaround. |
Please try again |
May be a dirty environment, but here is the first error:
|
That should be |
Windows: It compiled
|
Thanks! |
I found a StackOverflow piece that fixed the cmake/mac/pthread issue. But now I am investigating the item below (It's back, or it never left):
Note: The __int128 issue here is in core, whereas earlier it was fc. Update: after refreshing my codebase, the __int128 issue in fc was back, so evidently my cmake addition didn't fix the isue. I will try to revert the pr #121 changes. Update 2: I was able to get everything to compile once. Something is unstable, it may be my environment again. I will clone again, make the change to CMakeLists.txt, and see what happens. |
Replaced lexical_cast on uint128 with fc::variant in core - bitshares/bitshares-core#1789 |
The following is what was added to CMakeLists.txt in core. It was added before the find_boost call:
I highly doubt this was caused by this PR, but will test on Mac with [core develop] and [fc master] to verify. I will then create a PR in core for this fix. |
With the unusable state of the development branch on mac, it is difficult to test a PR of my CMakeLists.txt fix without your branch. You could include it as part of this PR, or I can wait until this is merged in and do it as a separate PR. Regardless, I'll perform a review and test on all platforms again, but with my fix when testing with macOS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. One small cmake warning.
@@ -7,7 +7,7 @@ PROJECT( fc ) | |||
set( CMAKE_CXX_STANDARD 14 ) | |||
SET( CMAKE_CXX_STANDARD_REQUIRED ON ) | |||
|
|||
if( "${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU" ) | |||
if( "${CMAKE_CXX_COMPILER_ID}" MATCHES "^(Apple)?Clang|GNU$" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CMake Warning (dev) in CMakeLists.txt:
A logical block opening on the line
/home/jmjatlanta/Development/cpp/bitshares-fc-1584/CMakeLists.txt:10 (if)
closes on the line
/home/jmjatlanta/Development/cpp/bitshares-fc-1584/CMakeLists.txt:14 (endif)
with mis-matching arguments.
Perhaps C-n-P the "if" into the "else" and "endif". Tested with cmake 3.15.1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, also in core
Attempt to fix bitshares/bitshares-core#1816