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

uplink-sys download uplink-c libraries rather than compiling them #19

Open
ifraixedes opened this issue Jul 1, 2022 · 9 comments
Open
Labels
crate:uplink-sys Specific issues / PRs for uplink-sys crate scope:build Issues / PRs realted with the build process status:under-discussion Issues / PRs on hold for a future decision type:enhancement New feature or request

Comments

@ifraixedes
Copy link
Collaborator

uplink-sys has a build script that compiles the uplink-c for generating the binary libraries files (e.g. .so, .dll, etc.).
uplink-c requires Go compiling.

Requiring Go causes friction for using the crate because it has to be installed

  • It isn't uncommon that people in the Rust ecosystem has Go installed, so Rust developers will be reluctant to use it.
  • docs.rs doesn't work and it seems that's because of Go not being installed in the container that it uses for building the crate and its documentation. See https://docs.rs/crate/uplink-sys/0.5.1/builds/580167

Although this is related to uplink-sys and Rust developers will likely use the uplink crate, this problem is transient. Because uplink depends on uplink-sys, when uplink is used, it requires building uplink-sys so those 2 issues listed above impact uplink crate in the same way (see doc.rs for uplink https://docs.rs/crate/uplink/0.1.1/builds/585706).

For resolving this issue, first, Storj has to publish the uplink-c binary libraries for the different architectures and operating systems, that uplink-c supports. Then we can change the uplink-sys crate build script:

  1. Download them and build the crate.
  2. Or has all of them packaged in the crate for using them to build the crate in the target machine. This option is better because it doesn't require network access which is beneficial in certain secured environments or to be able to build the crate without an internet connection.
@ifraixedes ifraixedes added the type:enhancement New feature or request label Jul 1, 2022
@kmozurkewich
Copy link
Member

kmozurkewich commented Jul 1, 2022

With our other bindings we made is explicitly required that the dev builds the uplink-c, usually via build scripts in the bindings; this means having the toolchain for the uplink-c build is a pre-req. Rust devs will likely have Go installed and if not, they can quickly install it. Having the Storj team manage yet-another artifact is unnecessary.

@ifraixedes
Copy link
Collaborator Author

With our other bindings we made is explicitly required that the dev builds the uplink-c, usually via build scripts in the bindings; this means having the toolchain for the uplink-c build is a pre-req. Rust devs will likely have Go installed and if not, they can quickly install it.

In my opinion, it creates too much fiction.
What do you make you think that Rust devs will likely have Go installed?

What about docs.rs? do you know if there is a way that could compile uplink-sys?

Having the Storj team manage yet-another artifact is unnecessary.

I think that could be added to the the uplink-c release process and upload the binaries into Github as we upload binaries for Storj repo release.

@kmozurkewich
Copy link
Member

What do you make you think that Rust devs will likely have Go installed?

Simply because they are developers. The build instructions should work, however, there will be edge-cases that a dev with a unique environment would be expected to navigate.

I think that could be added to the the uplink-c release process and upload the binaries into Github as we upload binaries for Storj repo release.

Agreed - would not be much effort, however, ensuring compatibility and support with each platform developers expect the binaries to work on could create additional overhead.

@ifraixedes
Copy link
Collaborator Author

Simply because they are developers. The build instructions should work, however, there will be edge-cases that a dev with a unique environment would be expected to navigate.

I see more of an issue when uplink-sys is a transient dependency. Then it isn't that clear that Go is required.

Agreed - would not be much effort, however, ensuring compatibility and support with each platform developers expect the binaries to work on could create additional overhead.

True, however, we could require Go as a fallback solution, if for that specific arch and/or OS the crate doesn't have the binary libraries then compile uplink-c.

I'll have a look at a possible solution to docs.rs without requiring this.

@ifraixedes
Copy link
Collaborator Author

I've spent some time with the docs.rs docs and it looks to me that there is no way to use a container with Go installed because it uses some predefined images.

I also tried to do some hacks for skipping calling Go, in terms of not failing despite not building the docs for uplink-sys to see if I could reach a point where we could skip them but allowing to build uplink crate docs. On local, they didn't work, but after I thought that it was a bad idea because uplink would never build if uplink-sys isn't built and without building the docs generation would always fail.

At this point, I don't know how we can make docs.rs work without bundling the binary libraries on it. My next try is to trick the build script to use a bundle uplink-c library for the target that docs.rs uses and only used when the build runs by docs.rs.

@ifraixedes
Copy link
Collaborator Author

ifraixedes commented Jul 4, 2022

I tried to conditionally build the crate including the Uplink-c binary libraries but the build is still failing in the docs.rs because it needs libclang 😞

I don't have more ideas about what more to do for building the documentation of the crates in docs.rs.

It's worth commenting that even if we add the library binaries and build the crate from the docs.rs won't work either because of the dependency with libclang.

@ifraixedes
Copy link
Collaborator Author

ifraixedes commented Jul 5, 2022

I created #21 for the specific issue with docs.rs not building the docs for the crates and leave this issue only for the discussing and working on not requiring Go for building uplink-sys crate

@ifraixedes ifraixedes added scope:build Issues / PRs realted with the build process crate:uplink-sys Specific issues / PRs for uplink-sys crate labels Jul 5, 2022
@ifraixedes ifraixedes added the status:under-discussion Issues / PRs on hold for a future decision label Oct 25, 2022
@ifraixedes
Copy link
Collaborator Author

I was thinking that perhaps we could have a hybrid approach.

When Go is installed and the operative system is Linux or OSX, compile the uplink-c as it's currently done. For the rest of the cases use the lib objects included in the repository.

For doing it, we should include the lib objects in the repository; at the beginning only for the supported platforms, Linux and OSX , and use them when Go isn't installed. After we could include other operative systems and perhaps architectures, which would require updating the build.rs script to execute differently for each one that requires different commands, for example, Windows.

@ifraixedes
Copy link
Collaborator Author

ifraixedes commented Oct 27, 2023

I realized that since uplink-c v1.8.0, there are the binary libs for the 3 major operative systems published in Github.

I'm thinking of creating a proof-of-concept of the hybrid approach mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate:uplink-sys Specific issues / PRs for uplink-sys crate scope:build Issues / PRs realted with the build process status:under-discussion Issues / PRs on hold for a future decision type:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants