-
Notifications
You must be signed in to change notification settings - Fork 187
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
Run CI tests on Mac too and create Mac binaries on releases #178
Conversation
b52216f
to
14700e2
Compare
14700e2
to
050b150
Compare
I asked a friend to download We didn't have enough time to install boost to see if that would resolve it but I'm wondering if the dynamic dependency against boost is expected? I thought we would produce an independent binary that has everything it needs compiled into it or is this dependency expected? |
@satyamakgec If I remember correctly I think you are using a Mac, no? If so, would you mind trying to run the |
@cburgdorf Yeah I do have, I approved this as it seems right to me but I haven't test it out. But I can test it locally if you want. |
I think there's a problem with how we handle the libsolc dependency. It ends up missing |
Thanks for checking, that's yet another error...interesting. |
If I run
If I download the produced
So, clearly there's a linking problem and I'm now wondering how to statically link the |
Ok, I've spend half the day on this but didn't get anywhere. I believe this is the place that causes these dependencies to become dynamically linked.
I trued changing these to
I tried to resolve that by passing flags to the compiler and changing the search path in the script but no luck. I'm giving up for today but it seems clear that we won't have a January release without resolving this first. Any bright ideas are welcome 😁 |
To keep track, here are a few things that I tried regarding |
I will take a look in a while. 😉 |
@cburgdorf right set of instruction will be
I used these steps in a docker container and the build works fine at least for ubuntu |
@cburgdorf @g-r-a-n-t I think there is no problem with the solc linking, There are some steps that were missing I think and those are essential to dynamically link the libraries with the build. |
@satyamakgec Can you put up a branch with the changes that you think should be working. I want to let Github Actions cut a release and download it locally to see if it works on my machine. In general, I think static linking would be better because even if we get dynamic linking to work its less convenient if the user has to install boost or other dependencies themselves. Solidity itself also creates statically linked binaries. We can see no boost dependency here:
|
Adding a couple more library paths seems to have satisfied both builds. It now links to the boost libs statically. @cburgdorf, you should be able to run the solc-rust build now with the latest commit on I downloaded the linux binary and tested it on my laptop, it works just find. Previously, attempting to compile a Fe contract resulted in linking error. This is what
@satyamakgec Would mind trying this on macOS? I'm not sure if the error you saw is resolved. |
I am still getting the same error @g-r-a-n-t from the new release. |
Hmm.. I do the same steps in github actions CI then it again spitting the same error. https://github.com/satyamakgec/fe/releases/tag/v0.1.0-test-1 In general, what I do is to create a defined docker image with all the dependencies and then use that build and that binary can be used everywhere. But I think here static link makes more sense as you said @cburgdorf. |
Hooray! I can confirm that the linux build from https://github.com/g-r-a-n-t/fe/releases/tag/v_test_3 works on my machine.
Everything is statically linked into the binary 🎉 Great find with the different search paths! For the Mac issue, I think this might be an incompatibility between different mac versions...at least that's what a quick Google search suggests regarding that error. But I think we can choose between different Mac versions on Github Actions to resolve that. Btw, I'm not sure if the |
FYI I am ahead in macOS version 10.13.6 |
Ok, so I tried cburgdorf/solc-rust@505758b and it turns out the
|
This page suggests 10.15 is the minimal version that Github Actions supports. But it's unclear if that is the actual issue. |
Oh but at least we have one confirmation from @njgheorghita that the mac binary is working for him. I will report back when I know his version number. I think I'm going to merge this now. It may not work for everyone yet but given that current master doesn't work at all (not even the linux builds) it is at least an improvement to the status quo. |
I preserved the remaining problems in this new issue #181 |
Thanks everyone for helping out with this. |
The Mac that was able to run the binary (from @njgheorghita) was on 10.14.6 |
What was wrong?
We currently do not test on Mac and don't produce Mac binaries on releases.
How was it fixed?
To-Do
Add entry to the release notes (may forgo for trivial changes)
Clean up commit history