-
Notifications
You must be signed in to change notification settings - Fork 12
Bazelify the GMP Precision Arithmetic Library #2
Comments
Issue Status: 1. Open 2. Started 3. Submitted 4. Done This issue now has a funding of 1000.0 DAI (1000.0 USD @ $1.0/DAI) attached to it as part of the Ethereum Foundation fund.
|
Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. These users each claimed they can complete the work by 3 weeks, 1 day ago. 1) pepeinvest has applied to start work (Funders only: approve worker | reject worker). Creative Bussines developers And finance LTd Learn more on the Gitcoin Issue Details page. 2) pacamara has applied to start work (Funders only: approve worker | reject worker). Hi! I'd like to start work on this. Action plan: 1. Install Bazel. 2. Write appropriate rules for lib GMP. 3. Test on Ubuntu, OSX, and possibly Windows (if required). Learn more on the Gitcoin Issue Details page. 3) robin-thomas has been approved to start work.
Learn more on the Gitcoin Issue Details page. |
Hey @jakerockland you're approved but please let us know ASAP if you don't think you'll be able to do it :) |
@jakerockland If you want we can possibly work on together on this & maybe share the bounty? |
@rauljordan @ceresstation To make sure I'm understanding the issue correctly, is the end deliverable for this something along the lines of a new repository containing the source for GMP, with a Bazel build file that allows the GMP libraries to be built with Bazel instead of Make? |
@samparsky I'm open to collaborating if there is a sensible way to split things up, what did you have in mind? |
@jakerockland Yes. This should be built entirely using Bazel as the build tool. It should build on:
|
@prestonvanloon Thank you for the clarification. Given the high priority of this and what else I have in the pipeline, after taking some time to scope this out last night and realize how much of a behemoth/custom configuration scripts there are with GMP and confirming now that I understood properly, I think that it may be better to let someone else take on this bounty @ceresstation. I've spend a good amount of time today trying to get things to work with this approach, to no avail bazel-contrib/rules_go#1233. Have you seen this issue @prestonvanloon? |
Fair @jakerockland, @samparsky are you still up for taking it on? Do you feel confident you're able to do it? |
@ceresstation Started work on it, will let you know my progress by tomorrow |
@jakerockland @ceresstation I have been able to get it to build successfully using Bazel on MacOSX. Currently testing for ubuntu using a docker image. |
That's awesome, thanks for the update @samparsky! |
@prestonvanloon I have been able to get it to build on using Bazel on Ubuntu & MacOSX. But I am facing an issue I am able to get the build to work properly on MacOSX with Bazel sandbox mode off only & on Linux it works with sandbox mode on only. I think it has something to do with the way the underlying filesystem works in these different OSes. You can check out the details here The only difference between the two folders is |
@samparsky I took a look at your repository. It looks like your implementation is a genrule calling configure and make. The goal of this task is to use bazel to build the library, not make. Additionally, you shouldn’t require users to install any system dependencies. They should be able to have a working bazel installation and run I realize this issue might have been misleading because I attempted to use the genrule approach as a bandaid fix for building go-bls without gmp installed on the system. Sorry about the confusion. |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done @samparsky due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!
Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days |
@prestonvanloon I tried building it manually couldn't get it to work. Also emailed the GMPlib mailing list to get any pointers couldn't get any positive response |
@prestonvanloon If it's not an issue, I would like to work on it. As I understand, you need to download the GMP library, and use bazel to build it. |
@robin-thomas sounds good to me. As long as bazel is the tool to actually build it. (Not using bazel as a wrapper for make or similar) |
@ceresstation can you help transfer this issue to @robin-thomas please? |
@prestonvanloon @rauljordan Could you take a look at: https://github.com/robin-thomas/bazelify-gmp? Its still WIP, but to give you some idea. |
Looking great @robin-thomas! |
@prestonvanloon Thanks! I'll continue working on it. Still loads more to be done. @ceresstation Can you kindly transfer this issue to me? Thanks. |
@prestonvanloon, @rauljordan. Wanted to give you an update. Many of the *.h, *.c files are generated on the fly by executing the configure script, many of which are not even known during the analysis stage of Bazel. Bazel requires these names before the build even starts! Related to how Bazel constructs its dependency graph. Anyway, the only solution here is to zip all those files into one file with a fixed name that can be passed to Bazel. But then again, there is no native support to pass that zip file as the input to cc_* rules for compilation. The only way around it is using Tree Artifacts, and it's a bit of hack to get it working and compiled into a library. The configure script also generates loads of *.asm files based on the host architecture. It cannot be passed to Bazel's cc_rules, as Bazel already assumes *.asm files to be assembly code generated by the compiler. bazelbuild/bazel#4856. But that's not the case here, and we need those files to be processed. The only way to do that is to use m4 to process these files into .s files which can then be given to Bazel's cc- rules (its placed in the same zip file above). Another issue is how GMP compiles many of these generated *.c files. They require macros to be passed to the compiler, which is different for every single file. Else the file cannot be compiled. This may not have been much of an issue, but with Tree Artifacts, this seems a bit hard to solve. And I'm currently trying to solve this problem. |
Thanks for the update @robin-thomas! Can select help with choosing the correct files based on the architecture? |
@prestonvanloon No. That select is a bit different. It's like a dictionary lookup based on a key, where the key can be architecture or some other parameters, but the value stored in the dictionary should be pre-defined. That's not the case here since we don't know the contents of the dictionary beforehand. Anyway, I think I might have found another way to tackle it. So trying that. |
Seems like I found a small bug in Bazel's code: bazelbuild/bazel#6963 Maybe I'll wait for someone from their dev team to reply there. |
Sounds good, that certainly looks like a bug to me. |
Hey @robin-thomas tried it out on a fresh windows configuration and received this:
However I'm not convinced this is an issue with the project but perhaps with how I set everything up - perhaps there was some other system dep I was missing |
@rauljordan Were you able to build Maybe checkout to commit |
@prestonvanloon Can I assume its working on Linux and Mac? |
Hi @robin-thomas, confirmed it works on Linux and Mac. Waiting on a windows confirmation as I no longer have access to a windows machine to attempt. Perhaps @IvanTheGreatDev can help with this? |
@rauljordan That's great! |
Hey @robin-thomas tried on another windows machine and received the same error. Checked out to that commit and then received this Java error
Sorry this is taking so long to debug - I know also @JerryFireman has windows and can try this out. |
@rauljordan That's weird. That commit was before I even started working on gmp! So you had never built |
@JerryFireman @IvanTheGreatDev Have either of you built |
@rauljordan Regarding the first error, I had a look into cgo code.
Windows static libraries have an extension *.lib (maybe Bazel produces *.a libraries on Windows, but I don't know. Can someone check this out?). If that's the case, then |
@rauljordan Also, can you share the details about the Windows setup? What's the compiler used? If its MSVC, then |
Hey Robin, it is indeed MSVC. That must be the issue in that case. @houjieth agreed to try it oit on windows this evening as well.
…On Thu, Jan 3 2019 at 5:43 PM, < ***@***.*** > wrote:
@rauljordan ( https://github.com/rauljordan ) Also, can you share the
details about the Windows setup? What's the compiler used? If its MSVC,
then cgo doesn't have support for it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub (
#2 (comment) ) ,
or mute the thread (
https://github.com/notifications/unsubscribe-auth/AFUIPUUTUZFf9O6-V7KjbpxYO6FVcWO2ks5u_pWtgaJpZM4Ym4L_
).
|
@rauljordan Yup. That should be the issue then. So you never tested on Windows before? @houjieth Try MSYS + gcc. Or maybe Git Bash (not sure whether a compiler is built into it) |
@houjieth Any updates? |
I'm using native bazel on windows (no cygwin, no mingw, no msys, no other simulated unix environment such as WSL). Here's my output
I'm going to try MinGW + MSYS next. |
@houjieth From the log, it seems to be using MSVC. If that's the case, then the error is expected. |
Tried on MSYS2, got the same error. How can I enforce it to use gcc instead of MSVC? I'm really not familiar with MinGW and MSYS2... |
Are you able to run the below command?
@houjieth Is there somewhere we can chat? Also run the below command, and say whats the output:
|
Now I'm using MSYS2 now without MSVC.
If add
|
Thanks for the logs @houjieth! @prestonvanloon It seems like |
Hey @robin-thomas, m4 is here on sourceforge http://gnuwin32.sourceforge.net/packages/m4.htm, and perhaps we can use bazel platform selection to download the right distribution? It seems to be downloading the linux version with the current configuation. @prestonvanloon suggested this. |
@rauljordan @prestonvanloon I had fixed all the m4 issues, and other issues that came later. It now builds GMP library fine. But |
@houjieth can you give this another try? |
Builds on windows! |
The above works on Hyper, but not on MSYS. However, this is good enough for us as a lot of work has already been put in. Allowing for support all across the board including MSYS can be a separate PR or bounty. |
@ceresstation we are happy with the bounty - I can edit the README post merge to include details about building on Windows. |
Fantastic to hear @rauljordan, thanks for all of your help @robin-thomas! |
Issue Status: 1. Open 2. Started 3. Submitted 4. Done The funding of 1000.0 DAI (1000.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @robin-thomas.
|
Just to clarify, is the single
Or is it also required to bring in the m4 rules?
m4 is a dependency of gmp, but I'm not sure if the bazelify-gmp is self-sufficient to use alone. Thanks. |
@luxe Yes, I think you would need to install the transitive dependencies. |
Hi all,
A critical component of this library is the GNU Multiple Precision Arithmetic Library written in C used for most underlying cryptographic calculations that are part of BLS. A major problem with this is that most architectures do not ship with GMP installed globally, and therefore any user wishing to use go-bls in their project will need to pollute their global namespace with libgmp.
We have opted for using the Bazel build system created by Google for dependency management and packaging of this repository for use in other projects. In particular, go-bls will be a critical component of the Prysm client created by Prysmatic Labs for Ethereum 2.0. Prysm uses the Bazel build tool extensively and ideally we'd want to import go-bls via Bazel as well. The problem with this is that GMP is not included as a Bazel dependency of go-bls, and therefore the build will fail due not finding the library.
A solution to this would be to convert lib GMP into a Bazel project following the guidelines for C/C++ projects in the Bazel documentation here. This is fairly extensive effort as GMP is a very big library containing multiple pieces. Tackling this issue would allow Prysm and any other Bazel-enabled project to import go-bls without requiring users to have GMP installed on their machine.
We are requesting to bounty this project out for motivated individuals to make this happen. This is a high priority for us at Prysmatic Labs.
Starting Point
This issue created by @prestonvanloon is a great starting point for someone tackling this: unable to gen_rule as part of http_archive.
Preston began by using http_archive to pull the libgmp official repo and begin creating a bazel BUILD file for the system directly.
The text was updated successfully, but these errors were encountered: